From 2708c8c367a37fce87a655850908b10d766abb1c Mon Sep 17 00:00:00 2001 From: i10416 Date: Fri, 1 Mar 2024 01:41:24 +0900 Subject: [PATCH 1/7] fix(#19806): wrong tasty of scala module class reference This commit makes the following diff to TASTy for i17255 files. The TASTy before this commit relied on the compiler (aka all TASTy clients) intrinsically knowing how to resolve Module$ when the definition is actually Module[ModuleClass]. ```sh scalac tests/run/i17255/J.java tests/run/i17255/Module.scala -Yprint-tasty -Yjava-tasty ``` ```diff 90: EMPTYCLAUSE 91: TERMREF 17 [Module] 93: SHAREDtype 12 95: ELIDED 96: SHAREDtype 91 98: STATIC 99: DEFDEF(12) 18 [module] 102: EMPTYCLAUSE - 103: SELECTtpt 19 [Module$] + 103: SELECTtpt 19 [Module[ModuleClass]] 105: SHAREDtype 3 107: ELIDED 108: TYPEREF 17 [Module] 110: SHAREDtype 3 112: STATIC ``` --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 1f0adc9f1421..6d537ea62c7c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -688,7 +688,11 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer report.error(StableIdentPattern(tree, pt), tree.srcPos) def typedSelect(tree0: untpd.Select, pt: Type, qual: Tree)(using Context): Tree = - val selName = tree0.name + val selName = + if ctx.isJava && tree0.name.isTypeName && tree0.name.endsWith(StdNames.str.MODULE_SUFFIX) then + tree0.name.stripModuleClassSuffix.moduleClassName + else + tree0.name val tree = cpy.Select(tree0)(qual, selName) val superAccess = qual.isInstanceOf[Super] val rawType = selectionType(tree, qual) From 373e8e9788a9f11c0214cd6817966025e3a93f74 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Mon, 11 Mar 2024 14:53:02 +0100 Subject: [PATCH 2/7] fix name of Ident and Select trees for module class from java --- .../dotty/tools/dotc/core/ContextOps.scala | 5 +-- .../src/dotty/tools/dotc/typer/Typer.scala | 38 +++++++++++-------- tests/run/i17255/J.java | 37 ++++++++++++------ tests/run/i17255/Module.scala | 10 +++-- 4 files changed, 55 insertions(+), 35 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/ContextOps.scala b/compiler/src/dotty/tools/dotc/core/ContextOps.scala index 427cfa47c7c1..57c369a08de6 100644 --- a/compiler/src/dotty/tools/dotc/core/ContextOps.scala +++ b/compiler/src/dotty/tools/dotc/core/ContextOps.scala @@ -64,10 +64,7 @@ object ContextOps: val directSearch = def asModule = if name.isTypeName && name.endsWith(StdNames.str.MODULE_SUFFIX) then - pre.findMember(name.stripModuleClassSuffix.moduleClassName, pre, required, excluded) match - case NoDenotation => NoDenotation - case symDenot: SymDenotation => - symDenot.companionModule.denot + pre.findMember(name.stripModuleClassSuffix.moduleClassName, pre, required, excluded) else NoDenotation pre.findMember(name, pre, required, excluded) match case NoDenotation => asModule diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 6d537ea62c7c..5b176a7797ee 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -631,7 +631,13 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer case checkedType: NamedType if !prefixIsElidable(checkedType) => ref(checkedType).withSpan(tree.span) case _ => - tree.withType(checkedType) + def isScalaModuleRef = checkedType match + case moduleRef: TypeRef if moduleRef.symbol.is(ModuleClass, butNot = JavaDefined) => true + case _ => false + if ctx.isJava && isScalaModuleRef then + cpy.Ident(tree)(tree.name.unmangleClassName).withType(checkedType) + else + tree.withType(checkedType) val tree2 = toNotNullTermRef(tree1, pt) checkLegalValue(tree2, pt) tree2 @@ -688,11 +694,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer report.error(StableIdentPattern(tree, pt), tree.srcPos) def typedSelect(tree0: untpd.Select, pt: Type, qual: Tree)(using Context): Tree = - val selName = - if ctx.isJava && tree0.name.isTypeName && tree0.name.endsWith(StdNames.str.MODULE_SUFFIX) then - tree0.name.stripModuleClassSuffix.moduleClassName - else - tree0.name + val selName = tree0.name val tree = cpy.Select(tree0)(qual, selName) val superAccess = qual.isInstanceOf[Super] val rawType = selectionType(tree, qual) @@ -769,26 +771,30 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer def typeSelectOnTerm(using Context): Tree = val qual = typedExpr(tree.qualifier, shallowSelectionProto(tree.name, pt, this, tree.nameSpan)) - typedSelect(tree, pt, qual).withSpan(tree.span).computeNullable() - - def javaSelectOnType(qual: Tree)(using Context) = - // semantic name conversion for `O$` in java code - if !qual.symbol.is(JavaDefined) then - val tree2 = untpd.cpy.Select(tree)(qual, tree.name.unmangleClassName) - assignType(tree2, qual) + if ctx.isJava then + javaSelection(qual) else - assignType(cpy.Select(tree)(qual, tree.name), qual) + typedSelect(tree, pt, qual).withSpan(tree.span).computeNullable() + + def javaSelection(qual: Tree)(using Context) = + val tree1 = assignType(cpy.Select(tree)(qual, tree.name), qual) + tree1.tpe match + case moduleRef: TypeRef if moduleRef.symbol.is(ModuleClass, butNot = JavaDefined) => + // handle unmangling of module names (Foo$ -> Foo[ModuleClass]) + cpy.Select(tree)(qual, tree.name.unmangleClassName).withType(moduleRef) + case _ => + tree1 def tryJavaSelectOnType(using Context): Tree = tree.qualifier match { case sel @ Select(qual, name) => val qual1 = untpd.cpy.Select(sel)(qual, name.toTypeName) val qual2 = typedType(qual1, WildcardType) - javaSelectOnType(qual2) + javaSelection(qual2) case id @ Ident(name) => val qual1 = untpd.cpy.Ident(id)(name.toTypeName) val qual2 = typedType(qual1, WildcardType) - javaSelectOnType(qual2) + javaSelection(qual2) case _ => errorTree(tree, em"cannot convert to type selection") // will never be printed due to fallback diff --git a/tests/run/i17255/J.java b/tests/run/i17255/J.java index 3c6d64d75ab9..3706e53cbbc3 100644 --- a/tests/run/i17255/J.java +++ b/tests/run/i17255/J.java @@ -1,16 +1,29 @@ package p; public class J { - public static J j = new J(); - public static p.J f() { - return p.J.j; - } - public static Module$ module2() { - return p.Module$.MODULE$; - } - public static p.Module$ module() { - return p.Module$.MODULE$; - } + public static J j = new J(); - public String toString() { return "J"; } -} \ No newline at end of file + public static p.J f() { + return p.J.j; + } + + public static Module$ module2() { + return p.Module$.MODULE$; + } + + public static p.Module$ module() { + return p.Module$.MODULE$; + } + + public static Module.InnerModule$ innermodule2() { + return p.Module.InnerModule$.MODULE$; + } + + public static p.Module.InnerModule$ innermodule() { + return p.Module.InnerModule$.MODULE$; + } + + public String toString() { + return "J"; + } +} diff --git a/tests/run/i17255/Module.scala b/tests/run/i17255/Module.scala index e681264093da..936e78e8e4ce 100644 --- a/tests/run/i17255/Module.scala +++ b/tests/run/i17255/Module.scala @@ -1,8 +1,10 @@ -// scalajs: --skip - package p { object Module { override def toString = "Module" + + object InnerModule { + override def toString = "InnerModule" + } } } @@ -10,4 +12,6 @@ object Test extends App { assert(p.J.f().toString == "J") assert(p.J.module().toString == "Module") assert(p.J.module2().toString == "Module") -} \ No newline at end of file + assert(p.J.innermodule().toString == "InnerModule") + assert(p.J.innermodule2().toString == "InnerModule") +} From 89201623875bc363cf7bec2a745649df1d9c8168 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Mon, 11 Mar 2024 16:57:31 +0100 Subject: [PATCH 3/7] Test -print-tasty output of Java sources --- .../tools/dotc/config/ScalaSettings.scala | 1 + .../dotc/core/tasty/TastyAnsiiPrinter.scala | 5 +- .../tools/dotc/core/tasty/TastyPrinter.scala | 26 ++- .../tools/dotc/printing/RefinedPrinter.scala | 5 +- .../dotty/tools/dotc/transform/Pickler.scala | 37 +++- tests/pos/i19806/J.tastycheck | 161 ++++++++++++++++++ tests/pos/i19806/J_SCALA_ONLY.java | 22 +++ tests/pos/i19806/Module.scala | 9 + tests/run/i17255/Module.scala | 1 + 9 files changed, 256 insertions(+), 11 deletions(-) create mode 100644 tests/pos/i19806/J.tastycheck create mode 100644 tests/pos/i19806/J_SCALA_ONLY.java create mode 100644 tests/pos/i19806/Module.scala diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 3dafedd8e2e0..0cb508ce1272 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -388,6 +388,7 @@ private sealed trait YSettings: val YshowPrintErrors: Setting[Boolean] = BooleanSetting("-Yshow-print-errors", "Don't suppress exceptions thrown during tree printing.") val YprintTasty: Setting[Boolean] = BooleanSetting("-Yprint-tasty", "Prints the generated TASTY to stdout.") val YtestPickler: Setting[Boolean] = BooleanSetting("-Ytest-pickler", "Self-test for pickling functionality; should be used with -Ystop-after:pickler.") + val YtestPicklerCheck: Setting[Boolean] = BooleanSetting("-Ytest-pickler-check", "Self-test for pickling -print-tasty output; should be used with -Ytest-pickler.") val YcheckReentrant: Setting[Boolean] = BooleanSetting("-Ycheck-reentrant", "Check that compiled program does not contain vars that can be accessed from a global root.") val YdropComments: Setting[Boolean] = BooleanSetting("-Ydrop-docs", "Drop documentation when scanning source files.", aliases = List("-Ydrop-comments")) val YcookComments: Setting[Boolean] = BooleanSetting("-Ycook-docs", "Cook the documentation (type check `@usecase`, etc.)", aliases = List("-Ycook-comments")) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TastyAnsiiPrinter.scala b/compiler/src/dotty/tools/dotc/core/tasty/TastyAnsiiPrinter.scala index d8d72a4e651e..a3d8cedacb4a 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TastyAnsiiPrinter.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TastyAnsiiPrinter.scala @@ -2,7 +2,10 @@ package dotty.tools.dotc package core package tasty -class TastyAnsiiPrinter(bytes: Array[Byte]) extends TastyPrinter(bytes) { +class TastyAnsiiPrinter(bytes: Array[Byte], testPickler: Boolean) extends TastyPrinter(bytes, testPickler) { + + def this(bytes: Array[Byte]) = this(bytes, testPickler = false) + override protected def nameStr(str: String): String = Console.MAGENTA + str + Console.RESET override protected def treeStr(str: String): String = Console.YELLOW + str + Console.RESET override protected def lengthStr(str: String): String = Console.CYAN + str + Console.RESET diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala b/compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala index a74607dbc9d5..a4818e8d6fd5 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala @@ -20,9 +20,12 @@ import dotty.tools.tasty.TastyBuffer.Addr object TastyPrinter: def showContents(bytes: Array[Byte], noColor: Boolean): String = + showContents(bytes, noColor, testPickler = false) + + def showContents(bytes: Array[Byte], noColor: Boolean, testPickler: Boolean = false): String = val printer = - if noColor then new TastyPrinter(bytes) - else new TastyAnsiiPrinter(bytes) + if noColor then new TastyPrinter(bytes, testPickler) + else new TastyAnsiiPrinter(bytes, testPickler) printer.showContents() def main(args: Array[String]): Unit = { @@ -62,7 +65,9 @@ object TastyPrinter: println(line) } -class TastyPrinter(bytes: Array[Byte]) { +class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { + + def this(bytes: Array[Byte]) = this(bytes, testPickler = false) class TastyPrinterUnpickler extends TastyUnpickler(bytes) { var namesStart: Addr = uninitialized @@ -84,9 +89,16 @@ class TastyPrinter(bytes: Array[Byte]) { private def printHeader(sb: StringBuilder): Unit = val header = unpickler.header sb.append("Header:\n") - sb.append(s" version: ${header.majorVersion}.${header.minorVersion}.${header.experimentalVersion}\n") - sb.append(" tooling: ").append(header.toolingVersion).append("\n") - sb.append(" UUID: ").append(header.uuid).append("\n") + if testPickler then + // these fields are not stable when the TASTy/compiler versions change, so not useful for testing + sb.append(" version: \n") + sb.append(" tooling: \n") + sb.append(" UUID: \n") + else + sb.append(s" version: ${header.majorVersion}.${header.minorVersion}.${header.experimentalVersion}\n") + sb.append(" tooling: ").append(header.toolingVersion).append("\n") + sb.append(" UUID: ").append(header.uuid).append("\n") + end if sb.append("\n") private def printNames(sb: StringBuilder): Unit = @@ -230,6 +242,8 @@ class TastyPrinter(bytes: Array[Byte]) { import reader.* sb.append(s"\n\nAttributes (${reader.endAddr.index - reader.startAddr.index} bytes, starting from $base):\n") while !isAtEnd do + // TODO: Should we elide attributes under testPickler? (i.e. + // if we add new attributes many check files will need to be updated) val tag = readByte() sb.append(" ").append(attributeTagToString(tag)) if isBooleanAttrTag(tag) then () diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 893b34f48396..93e280f8a13c 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -1010,7 +1010,10 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { var modsText = modText(constr.mods, constr.symbol, "", isType = false) if (!modsText.isEmpty) modsText = " " ~ modsText if (constr.mods.hasAnnotations && !constr.mods.hasFlags) modsText = modsText ~~ " this" - withEnclosingDef(constr) { addParamssText(tparamsTxt ~~ modsText, constr.trailingParamss) } + val ctorParamss = + // for fake `(x$1: Unit): Foo` constructor, don't print the param (span is not reconstructed correctly) + if constr.symbol.isAllOf(JavaParsers.fakeFlags) then Nil else constr.trailingParamss + withEnclosingDef(constr) { addParamssText(tparamsTxt ~~ modsText, ctorParamss) } } val parentsText = Text(impl.parents.map(constrText), if (ofNew) keywordStr(" with ") else ", ") val derivedText = Text(impl.derived.map(toText(_)), ", ") diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index 27b5d53e25dc..0ce3be9c1c4e 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -69,6 +69,7 @@ class Pickler extends Phase { // Maps that keep a record if -Ytest-pickler is set. private val beforePickling = new mutable.HashMap[ClassSymbol, String] + private val printedTasty = new mutable.HashMap[ClassSymbol, String] private val pickledBytes = new mutable.HashMap[ClassSymbol, (CompilationUnit, Array[Byte])] /** Drop any elements of this list that are linked module classes of other elements in the list */ @@ -184,7 +185,10 @@ class Pickler extends Phase { else val pickled = computePickled() reportPositionWarnings() - if ctx.settings.YtestPickler.value then pickledBytes(cls) = (unit, pickled) + if ctx.settings.YtestPickler.value then + pickledBytes(cls) = (unit, pickled) + if ctx.settings.YtestPicklerCheck.value then + printedTasty(cls) = TastyPrinter.showContents(pickled, noColor = true, testPickler = true) () => pickled unit.pickled += (cls -> demandPickled) @@ -251,15 +255,22 @@ class Pickler extends Phase { private def testUnpickler(using Context): Unit = pickling.println(i"testing unpickler at run ${ctx.runId}") ctx.initialize() + val resolveCheck = ctx.settings.YtestPicklerCheck.value val unpicklers = for ((cls, (unit, bytes)) <- pickledBytes) yield { val unpickler = new DottyUnpickler(unit.source.file, bytes) unpickler.enter(roots = Set.empty) - cls -> (unit, unpickler) + val optCheck = + if resolveCheck then + val resolved = unit.source.file.resolveSibling(s"${cls.name.mangledString}.tastycheck") + if resolved == null then None + else Some(resolved) + else None + cls -> (unit, unpickler, optCheck) } pickling.println("************* entered toplevel ***********") val rootCtx = ctx - for ((cls, (unit, unpickler)) <- unpicklers) do + for ((cls, (unit, unpickler, optCheck)) <- unpicklers) do val testJava = unit.typedAsJava if testJava then if unpickler.unpickler.nameAtRef.contents.exists(_ == nme.FromJavaObject) then @@ -268,6 +279,15 @@ class Pickler extends Phase { val freshUnit = CompilationUnit(rootCtx.compilationUnit.source) freshUnit.needsCaptureChecking = unit.needsCaptureChecking freshUnit.knowsPureFuns = unit.knowsPureFuns + optCheck match + case Some(check) => + import java.nio.charset.StandardCharsets.UTF_8 + val checkContents = String(check.toByteArray, UTF_8) + inContext(rootCtx.fresh.setCompilationUnit(freshUnit)): + testSamePrinted(printedTasty(cls), checkContents, cls, check) + case None => + () + inContext(printerContext(testJava)(using rootCtx.fresh.setCompilationUnit(freshUnit))): testSame(i"$unpickled%\n%", beforePickling(cls), cls) @@ -283,4 +303,15 @@ class Pickler extends Phase { | | diff before-pickling.txt after-pickling.txt""") end testSame + + private def testSamePrinted(printed: String, checkContents: String, cls: ClassSymbol, check: AbstractFile)(using Context) = + import java.nio.charset.StandardCharsets.UTF_8 + def normal(s: String) = new String(s.getBytes(UTF_8), UTF_8) + val unequal = printed.length() != checkContents.length() || normal(printed) != normal(checkContents) + if unequal then + output("after-printing.txt", printed) + report.error(em"""TASTy printer difference for $cls in ${cls.source}, for details: + | + | diff ${check.toString} after-printing.txt""") + end testSamePrinted } diff --git a/tests/pos/i19806/J.tastycheck b/tests/pos/i19806/J.tastycheck new file mode 100644 index 000000000000..900530e80965 --- /dev/null +++ b/tests/pos/i19806/J.tastycheck @@ -0,0 +1,161 @@ +Header: + version: + tooling: + UUID: + +Names (217 bytes, starting from 80): + 0: ASTs + 1: p + 2: J + 3: J[ModuleClass] + 4: Object + 5: java + 6: lang + 7: java[Qualified . lang] + 8: _ + 9: + 10: Unit + 11: scala + 12: module2 + 13: Module + 14: Module[ModuleClass] + 15: module + 16: innermodule2 + 17: InnerModule + 18: InnerModule[ModuleClass] + 19: innermodule + 20: T + 21: Nothing + 22: Positions + 23: tests/pos/i19806/J_SCALA_ONLY.java + 24: Comments + 25: Attributes + + +Trees (145 bytes, starting from 300): + 0: PACKAGE(142) + 3: TERMREFpkg 1 [p] + 5: VALDEF(11) 2 [J] + 8: IDENTtpt 3 [J[ModuleClass]] + 10: TYPEREFsymbol 18 + 12: TERMREFpkg 1 [p] + 14: ELIDED + 15: SHAREDtype 10 + 17: OBJECT + 18: TYPEDEF(86) 3 [J[ModuleClass]] + 21: TEMPLATE(82) + 23: TYPEREF 4 [Object] + 25: TERMREFpkg 7 [java[Qualified . lang]] + 27: SELFDEF 8 [_] + 29: SINGLETONtpt + 30: TERMREFsymbol 5 + 32: SHAREDtype 12 + 34: DEFDEF(7) 9 [] + 37: EMPTYCLAUSE + 38: TYPEREF 10 [Unit] + 40: TERMREFpkg 11 [scala] + 42: STABLE + 43: DEFDEF(12) 12 [module2] + 46: EMPTYCLAUSE + 47: IDENTtpt 14 [Module[ModuleClass]] + 49: TYPEREF 14 [Module[ModuleClass]] + 51: SHAREDtype 12 + 53: ELIDED + 54: SHAREDtype 49 + 56: STATIC + 57: DEFDEF(12) 15 [module] + 60: EMPTYCLAUSE + 61: SELECTtpt 14 [Module[ModuleClass]] + 63: SHAREDtype 3 + 65: ELIDED + 66: TYPEREF 14 [Module[ModuleClass]] + 68: SHAREDtype 3 + 70: STATIC + 71: DEFDEF(14) 16 [innermodule2] + 74: EMPTYCLAUSE + 75: SELECTtpt 18 [InnerModule[ModuleClass]] + 77: TERMREF 13 [Module] + 79: SHAREDtype 12 + 81: ELIDED + 82: TYPEREF 18 [InnerModule[ModuleClass]] + 84: SHAREDtype 77 + 86: STATIC + 87: DEFDEF(16) 19 [innermodule] + 90: EMPTYCLAUSE + 91: SELECTtpt 18 [InnerModule[ModuleClass]] + 93: SELECT 13 [Module] + 95: SHAREDtype 3 + 97: ELIDED + 98: TYPEREF 18 [InnerModule[ModuleClass]] + 100: TERMREF 13 [Module] + 102: SHAREDtype 3 + 104: STATIC + 105: OBJECT + 106: TYPEDEF(37) 2 [J] + 109: TEMPLATE(34) + 111: TYPEPARAM(11) 20 [T] + 114: TYPEBOUNDS(6) + 116: TYPEREF 21 [Nothing] + 118: SHAREDtype 40 + 120: SHAREDtype 23 + 122: PRIVATE + 123: LOCAL + 124: SHAREDtype 120 + 126: SPLITCLAUSE + 127: DEFDEF(16) 9 [] + 130: TYPEPARAM(7) 20 [T] + 133: TYPEBOUNDStpt(4) + 135: SHAREDtype 116 + 137: SHAREDtype 120 + 139: EMPTYCLAUSE + 140: SHAREDtype 38 + 142: ELIDED + 143: SHAREDtype 38 + 145: + +Positions (145 bytes, starting from 448): + lines: 23 + line sizes: + 10, 0, 19, 0, 15, 0, 35, 29, 3, 0, 36, 29, 3, 0, 52, 41, 3, 0, 53, 41 + 3, 1, 0 + positions: + 0: 0 .. 394 + 5: 12 .. 12 + 8: 12 .. 12 + 18: 12 .. 394 + 21: 52 .. 392 + 23: 25 .. 25 + 30: 52 .. 52 + 34: 52 .. 52 + 38: 52 .. 52 + 43: 52 .. 119 + 47: 66 .. 73 + 57: 123 .. 191 + 61: 137 .. 146 + 63: 137 .. 138 + 71: 195 .. 291 + 75: 209 .. 228 + 77: 209 .. 215 + 87: 295 .. 392 + 91: 309 .. 330 + 93: 309 .. 317 + 95: 309 .. 310 + 106: 12 .. 394 + 109: 27 .. 48 + 111: 27 .. 28 + 114: 27 .. 27 + 124: 28 .. 28 + 127: 35 .. 48 + 130: 27 .. 28 + 135: 27 .. 27 + 137: 27 .. 27 + 140: 46 .. 46 + + source paths: + 0: 23 [tests/pos/i19806/J_SCALA_ONLY.java] + + +Attributes (4 bytes, starting from 597): + JAVAattr + OUTLINEattr + SOURCEFILEattr 23 [tests/pos/i19806/J_SCALA_ONLY.java] diff --git a/tests/pos/i19806/J_SCALA_ONLY.java b/tests/pos/i19806/J_SCALA_ONLY.java new file mode 100644 index 000000000000..abbc6c0e9d57 --- /dev/null +++ b/tests/pos/i19806/J_SCALA_ONLY.java @@ -0,0 +1,22 @@ +package p; + +public class J { + + public J() {} + + public static Module$ module2() { + return p.Module$.MODULE$; + } + + public static p.Module$ module() { + return p.Module$.MODULE$; + } + + public static Module.InnerModule$ innermodule2() { + return p.Module.InnerModule$.MODULE$; + } + + public static p.Module.InnerModule$ innermodule() { + return p.Module.InnerModule$.MODULE$; + } +} diff --git a/tests/pos/i19806/Module.scala b/tests/pos/i19806/Module.scala new file mode 100644 index 000000000000..d0142fc24682 --- /dev/null +++ b/tests/pos/i19806/Module.scala @@ -0,0 +1,9 @@ +//> using options -Yjava-tasty -Ytest-pickler-check + +package p + +object Module: + object InnerModule + +class Outer: + object InnerModule diff --git a/tests/run/i17255/Module.scala b/tests/run/i17255/Module.scala index 936e78e8e4ce..9b7153edbfd1 100644 --- a/tests/run/i17255/Module.scala +++ b/tests/run/i17255/Module.scala @@ -1,3 +1,4 @@ +// scalajs: --skip package p { object Module { override def toString = "Module" From 090b6473e68d8d0e89d0a659803db25fcb2ea026 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Wed, 13 Mar 2024 11:36:57 +0100 Subject: [PATCH 4/7] elide source file names --- .../tools/dotc/core/tasty/TastyPrinter.scala | 76 ++++++++++++++----- tests/pos/i19806/J.tastycheck | 6 +- 2 files changed, 60 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala b/compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala index a4818e8d6fd5..e63ac4737a09 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala @@ -13,9 +13,11 @@ import dotty.tools.tasty.TastyFormat.{ASTsSection, PositionsSection, CommentsSec import java.nio.file.{Files, Paths} import dotty.tools.io.{JarArchive, Path} import dotty.tools.tasty.TastyFormat.header +import scala.collection.immutable.BitSet import scala.compiletime.uninitialized import dotty.tools.tasty.TastyBuffer.Addr +import dotty.tools.dotc.core.Names.TermName object TastyPrinter: @@ -82,10 +84,6 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { private val unpickler: TastyPrinterUnpickler = new TastyPrinterUnpickler import unpickler.{nameAtRef, unpickle} - private def nameToString(name: Name): String = name.debugString - - private def nameRefToString(ref: NameRef): String = nameToString(nameAtRef(ref)) - private def printHeader(sb: StringBuilder): Unit = val header = unpickler.header sb.append("Header:\n") @@ -101,27 +99,34 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { end if sb.append("\n") - private def printNames(sb: StringBuilder): Unit = + private def printNames(sb: StringBuilder)(using refs: NameRefs): Unit = sb.append(s"Names (${unpickler.namesEnd.index - unpickler.namesStart.index} bytes, starting from ${unpickler.namesStart.index}):\n") for ((name, idx) <- nameAtRef.contents.zipWithIndex) { val index = nameStr("%6d".format(idx)) - sb.append(index).append(": ").append(nameToString(name)).append("\n") + sb.append(index).append(": ").append(refs.nameRefToString(NameRef(idx))).append("\n") } def showContents(): String = { val sb: StringBuilder = new StringBuilder + given NameRefs = unpickle0(new SourceFileUnpickler)(using NameRefs.empty).getOrElse(NameRefs.empty) printHeader(sb) printNames(sb) - unpickle(new TreeSectionUnpickler(sb)) - unpickle(new PositionSectionUnpickler(sb)) - unpickle(new CommentSectionUnpickler(sb)) - unpickle(new AttributesSectionUnpickler(sb)) + unpickle0(new TreeSectionUnpickler(sb)) + unpickle0(new PositionSectionUnpickler(sb)) + unpickle0(new CommentSectionUnpickler(sb)) + unpickle0(new AttributesSectionUnpickler(sb)) sb.result } - class TreeSectionUnpickler(sb: StringBuilder) extends SectionUnpickler[Unit](ASTsSection) { + def unpickle0[R](sec: PrinterSectionUnpickler[R])(using NameRefs): Option[R] = + unpickle(new SectionUnpickler[R](sec.name) { + def unpickle(reader: TastyReader, nameAtRef: NameTable): R = + sec.unpickle0(reader.subReader(reader.startAddr, reader.endAddr)) // fork so we can visit multiple times + }) + + class TreeSectionUnpickler(sb: StringBuilder) extends PrinterSectionUnpickler[Unit](ASTsSection) { import dotty.tools.tasty.TastyFormat.* - def unpickle(reader: TastyReader, tastyName: NameTable): Unit = { + def unpickle0(reader: TastyReader)(using refs: NameRefs): Unit = { import reader.* var indent = 0 def newLine() = { @@ -131,7 +136,7 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { def printNat() = sb.append(treeStr(" " + readNat())) def printName() = { val idx = readNat() - sb.append(nameStr(" " + idx + " [" + nameRefToString(NameRef(idx)) + "]")) + sb.append(nameStr(" " + idx + " [" + refs.nameRefToString(NameRef(idx)) + "]")) } def printTree(): Unit = { newLine() @@ -190,8 +195,8 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { } } - class PositionSectionUnpickler(sb: StringBuilder) extends SectionUnpickler[Unit](PositionsSection) { - def unpickle(reader: TastyReader, tastyName: NameTable): Unit = { + class PositionSectionUnpickler(sb: StringBuilder) extends PrinterSectionUnpickler[Unit](PositionsSection) { + def unpickle0(reader: TastyReader)(using tastyName: NameRefs): Unit = { import reader.* val posUnpickler = new PositionUnpickler(reader, tastyName) sb.append(s"\n\nPositions (${reader.endAddr.index - reader.startAddr.index} bytes, starting from $base):\n") @@ -222,8 +227,8 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { } } - class CommentSectionUnpickler(sb: StringBuilder) extends SectionUnpickler[Unit](CommentsSection) { - def unpickle(reader: TastyReader, tastyName: NameTable): Unit = { + class CommentSectionUnpickler(sb: StringBuilder) extends PrinterSectionUnpickler[Unit](CommentsSection) { + def unpickle0(reader: TastyReader)(using NameRefs): Unit = { import reader.* val comments = new CommentUnpickler(reader).comments if !comments.isEmpty then @@ -236,9 +241,9 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { } } - class AttributesSectionUnpickler(sb: StringBuilder) extends SectionUnpickler[Unit](AttributesSection) { + class AttributesSectionUnpickler(sb: StringBuilder) extends PrinterSectionUnpickler[Unit](AttributesSection) { import dotty.tools.tasty.TastyFormat.* - def unpickle(reader: TastyReader, tastyName: NameTable): Unit = { + def unpickle0(reader: TastyReader)(using nameAtRef: NameRefs): Unit = { import reader.* sb.append(s"\n\nAttributes (${reader.endAddr.index - reader.startAddr.index} bytes, starting from $base):\n") while !isAtEnd do @@ -256,6 +261,39 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { } } + class NameRefs(sourceFileRefs: Set[NameRef]) extends (NameRef => TermName): + private val isSourceFile = sourceFileRefs.map(_.index).to(BitSet) + + def nameRefToString(ref: NameRef): String = this(ref).debugString + + def apply(ref: NameRef): TermName = + if isSourceFile(ref.index) then NameRefs.elidedSourceFile + else nameAtRef(ref) + + object NameRefs: + import dotty.tools.dotc.core.Names.termName + + private val elidedSourceFile = termName("") + val empty = NameRefs(Set.empty) + + + class SourceFileUnpickler extends PrinterSectionUnpickler[NameRefs](PositionsSection) { + def unpickle0(reader: TastyReader)(using nameAtRef: NameRefs): NameRefs = { + if !testPickler then return NameRefs.empty + val buf = Set.newBuilder[NameRef] + val posUnpickler = new PositionUnpickler(reader, nameAtRef) + val sources = posUnpickler.sourceNameRefs + for ((_, nameRef) <- sources.iterator) { + buf += nameRef + } + NameRefs(buf.result) + } + } + + abstract class PrinterSectionUnpickler[T](val name: String) { + def unpickle0(reader: TastyReader)(using refs: NameRefs): T + } + protected def nameStr(str: String): String = str protected def treeStr(str: String): String = str protected def lengthStr(str: String): String = str diff --git a/tests/pos/i19806/J.tastycheck b/tests/pos/i19806/J.tastycheck index 900530e80965..7fe7970c5099 100644 --- a/tests/pos/i19806/J.tastycheck +++ b/tests/pos/i19806/J.tastycheck @@ -27,7 +27,7 @@ Names (217 bytes, starting from 80): 20: T 21: Nothing 22: Positions - 23: tests/pos/i19806/J_SCALA_ONLY.java + 23: 24: Comments 25: Attributes @@ -152,10 +152,10 @@ Positions (145 bytes, starting from 448): 140: 46 .. 46 source paths: - 0: 23 [tests/pos/i19806/J_SCALA_ONLY.java] + 0: 23 [] Attributes (4 bytes, starting from 597): JAVAattr OUTLINEattr - SOURCEFILEattr 23 [tests/pos/i19806/J_SCALA_ONLY.java] + SOURCEFILEattr 23 [] From 923f63f04d0c57ffbdfa26cd5198ffb5ba998858 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Wed, 13 Mar 2024 12:05:37 +0100 Subject: [PATCH 5/7] check lines not whole file --- .../dotty/tools/dotc/transform/Pickler.scala | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index 0ce3be9c1c4e..e467635f25bc 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -304,14 +304,33 @@ class Pickler extends Phase { | diff before-pickling.txt after-pickling.txt""") end testSame - private def testSamePrinted(printed: String, checkContents: String, cls: ClassSymbol, check: AbstractFile)(using Context) = - import java.nio.charset.StandardCharsets.UTF_8 - def normal(s: String) = new String(s.getBytes(UTF_8), UTF_8) - val unequal = printed.length() != checkContents.length() || normal(printed) != normal(checkContents) - if unequal then + private def testSamePrinted(printed: String, checkContents: String, cls: ClassSymbol, check: AbstractFile)(using Context): Unit = { + if hasDiff(printed, checkContents) then output("after-printing.txt", printed) report.error(em"""TASTy printer difference for $cls in ${cls.source}, for details: | | diff ${check.toString} after-printing.txt""") - end testSamePrinted + } + + /** Reuse diff logic from compiler/test/dotty/tools/vulpix/FileDiff.scala */ + private def hasDiff(actual: String, expect: String): Boolean = + import scala.util.Using + import scala.io.Source + val actualLines = Using(Source.fromString(actual))(_.getLines().toList).get + val expectLines = Using(Source.fromString(expect))(_.getLines().toList).get + !matches(actualLines, expectLines) + + private def matches(actual: String, expect: String): Boolean = { + import java.io.File + val actual1 = actual.stripLineEnd + val expect1 = expect.stripLineEnd + + // handle check file path mismatch on windows + actual1 == expect1 || File.separatorChar == '\\' && actual1.replace('\\', '/') == expect1 + } + + private def matches(actual: Seq[String], expect: Seq[String]): Boolean = { + actual.length == expect.length + && actual.lazyZip(expect).forall(matches) + } } From 6e1bba5793474a8390f7d989b0a6dd475a71f989 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Wed, 13 Mar 2024 13:16:34 +0100 Subject: [PATCH 6/7] print what is the diff --- compiler/src/dotty/tools/dotc/transform/Pickler.scala | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index e467635f25bc..cd5461880655 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -307,9 +307,10 @@ class Pickler extends Phase { private def testSamePrinted(printed: String, checkContents: String, cls: ClassSymbol, check: AbstractFile)(using Context): Unit = { if hasDiff(printed, checkContents) then output("after-printing.txt", printed) - report.error(em"""TASTy printer difference for $cls in ${cls.source}, for details: - | - | diff ${check.toString} after-printing.txt""") + report.error(em"""TASTy printer difference for $cls in ${cls.source}, did not match ${check}, + | output dumped in after-printing.txt, check diff with `git diff --no-index -- after-printing.txt ${check}` + | actual output: + |$printed""") } /** Reuse diff logic from compiler/test/dotty/tools/vulpix/FileDiff.scala */ From 1d5db004270b07d25f902aaba9da96e0c0d6005f Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Wed, 13 Mar 2024 13:57:09 +0100 Subject: [PATCH 7/7] elide base index of sections --- .../tools/dotc/core/tasty/TastyPrinter.scala | 28 +++++++++++++++---- .../dotty/tools/dotc/transform/Pickler.scala | 10 +++---- tests/pos/i19806/J.tastycheck | 10 +++---- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala b/compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala index e63ac4737a09..af2097f347ba 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala @@ -97,10 +97,14 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { sb.append(" tooling: ").append(header.toolingVersion).append("\n") sb.append(" UUID: ").append(header.uuid).append("\n") end if - sb.append("\n") private def printNames(sb: StringBuilder)(using refs: NameRefs): Unit = - sb.append(s"Names (${unpickler.namesEnd.index - unpickler.namesStart.index} bytes, starting from ${unpickler.namesStart.index}):\n") + sb.append(sectionHeader( + name = "Names", + count = (unpickler.namesEnd.index - unpickler.namesStart.index).toString, + base = showBase(unpickler.namesStart.index), + lineEnd = true + )) for ((name, idx) <- nameAtRef.contents.zipWithIndex) { val index = nameStr("%6d".format(idx)) sb.append(index).append(": ").append(refs.nameRefToString(NameRef(idx))).append("\n") @@ -187,11 +191,12 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { } indent -= 2 } - sb.append(s"\n\nTrees (${endAddr.index - startAddr.index} bytes, starting from $base):") + sb.append(sectionHeader("Trees", reader, lineEnd = false)) while (!isAtEnd) { printTree() newLine() } + sb.append("\n") } } @@ -199,7 +204,7 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { def unpickle0(reader: TastyReader)(using tastyName: NameRefs): Unit = { import reader.* val posUnpickler = new PositionUnpickler(reader, tastyName) - sb.append(s"\n\nPositions (${reader.endAddr.index - reader.startAddr.index} bytes, starting from $base):\n") + sb.append(sectionHeader("Positions", reader)) val lineSizes = posUnpickler.lineSizes sb.append(s" lines: ${lineSizes.length}\n") sb.append(s" line sizes:\n") @@ -232,7 +237,7 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { import reader.* val comments = new CommentUnpickler(reader).comments if !comments.isEmpty then - sb.append(s"\n\nComments (${reader.endAddr.index - reader.startAddr.index} bytes, starting from $base):\n") + sb.append(sectionHeader("Comments", reader)) val sorted = comments.toSeq.sortBy(_._1.index) for ((addr, cmt) <- sorted) { sb.append(treeStr("%6d".format(addr.index))) @@ -245,7 +250,7 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { import dotty.tools.tasty.TastyFormat.* def unpickle0(reader: TastyReader)(using nameAtRef: NameRefs): Unit = { import reader.* - sb.append(s"\n\nAttributes (${reader.endAddr.index - reader.startAddr.index} bytes, starting from $base):\n") + sb.append(sectionHeader("Attributes", reader)) while !isAtEnd do // TODO: Should we elide attributes under testPickler? (i.e. // if we add new attributes many check files will need to be updated) @@ -290,6 +295,17 @@ class TastyPrinter(bytes: Array[Byte], val testPickler: Boolean) { } } + private final def showBase(index: Int): String = + if testPickler then "" else index.toString() + + private final def sectionHeader(name: String, reader: TastyReader, lineEnd: Boolean = true): String = + val count = reader.endAddr.index - reader.startAddr.index + sectionHeader(name, count.toString, {showBase(reader.base)}, lineEnd) + + private final def sectionHeader(name: String, count: String, base: String, lineEnd: Boolean): String = + val suffix = if lineEnd then "\n" else "" + s"\n$name ($count bytes, starting from $base):$suffix" + abstract class PrinterSectionUnpickler[T](val name: String) { def unpickle0(reader: TastyReader)(using refs: NameRefs): T } diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index cd5461880655..b0aed580e824 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -305,21 +305,21 @@ class Pickler extends Phase { end testSame private def testSamePrinted(printed: String, checkContents: String, cls: ClassSymbol, check: AbstractFile)(using Context): Unit = { - if hasDiff(printed, checkContents) then + for lines <- diff(printed, checkContents) do output("after-printing.txt", printed) report.error(em"""TASTy printer difference for $cls in ${cls.source}, did not match ${check}, - | output dumped in after-printing.txt, check diff with `git diff --no-index -- after-printing.txt ${check}` + | output dumped in after-printing.txt, check diff with `git diff --no-index -- $check after-printing.txt` | actual output: - |$printed""") + |$lines%\n%""") } /** Reuse diff logic from compiler/test/dotty/tools/vulpix/FileDiff.scala */ - private def hasDiff(actual: String, expect: String): Boolean = + private def diff(actual: String, expect: String): Option[Seq[String]] = import scala.util.Using import scala.io.Source val actualLines = Using(Source.fromString(actual))(_.getLines().toList).get val expectLines = Using(Source.fromString(expect))(_.getLines().toList).get - !matches(actualLines, expectLines) + Option.when(!matches(actualLines, expectLines))(actualLines) private def matches(actual: String, expect: String): Boolean = { import java.io.File diff --git a/tests/pos/i19806/J.tastycheck b/tests/pos/i19806/J.tastycheck index 7fe7970c5099..110c33310e43 100644 --- a/tests/pos/i19806/J.tastycheck +++ b/tests/pos/i19806/J.tastycheck @@ -3,7 +3,7 @@ Header: tooling: UUID: -Names (217 bytes, starting from 80): +Names (217 bytes, starting from ): 0: ASTs 1: p 2: J @@ -31,8 +31,7 @@ Names (217 bytes, starting from 80): 24: Comments 25: Attributes - -Trees (145 bytes, starting from 300): +Trees (145 bytes, starting from ): 0: PACKAGE(142) 3: TERMREFpkg 1 [p] 5: VALDEF(11) 2 [J] @@ -113,7 +112,7 @@ Trees (145 bytes, starting from 300): 143: SHAREDtype 38 145: -Positions (145 bytes, starting from 448): +Positions (145 bytes, starting from ): lines: 23 line sizes: 10, 0, 19, 0, 15, 0, 35, 29, 3, 0, 36, 29, 3, 0, 52, 41, 3, 0, 53, 41 @@ -154,8 +153,7 @@ Positions (145 bytes, starting from 448): source paths: 0: 23 [] - -Attributes (4 bytes, starting from 597): +Attributes (4 bytes, starting from ): JAVAattr OUTLINEattr SOURCEFILEattr 23 []