Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(#19806): wrong tasty of scala module class reference #19827

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
5 changes: 1 addition & 4 deletions compiler/src/dotty/tools/dotc/core/ContextOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
130 changes: 99 additions & 31 deletions compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,21 @@ 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:

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 = {
Expand Down Expand Up @@ -62,7 +67,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
Expand All @@ -77,39 +84,53 @@ class TastyPrinter(bytes: Array[Byte]) {
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")
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")
sb.append("\n")
if testPickler then
// these fields are not stable when the TASTy/compiler versions change, so not useful for testing
sb.append(" version: <elided>\n")
sb.append(" tooling: <elided>\n")
sb.append(" UUID: <elided>\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

private def printNames(sb: StringBuilder): Unit =
sb.append(s"Names (${unpickler.namesEnd.index - unpickler.namesStart.index} bytes, starting from ${unpickler.namesStart.index}):\n")
private def printNames(sb: StringBuilder)(using refs: NameRefs): Unit =
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(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() = {
Expand All @@ -119,7 +140,7 @@ class TastyPrinter(bytes: Array[Byte]) {
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()
Expand Down Expand Up @@ -170,19 +191,20 @@ class TastyPrinter(bytes: Array[Byte]) {
}
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")
}
}

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")
sb.append(sectionHeader("Positions", reader))
val lineSizes = posUnpickler.lineSizes
sb.append(s" lines: ${lineSizes.length}\n")
sb.append(s" line sizes:\n")
Expand Down Expand Up @@ -210,12 +232,12 @@ class TastyPrinter(bytes: Array[Byte]) {
}
}

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
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)))
Expand All @@ -224,12 +246,14 @@ class TastyPrinter(bytes: Array[Byte]) {
}
}

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")
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)
val tag = readByte()
sb.append(" ").append(attributeTagToString(tag))
if isBooleanAttrTag(tag) then ()
Expand All @@ -242,6 +266,50 @@ class TastyPrinter(bytes: Array[Byte]) {
}
}

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("<elided source file name>")
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)
}
}

private final def showBase(index: Int): String =
if testPickler then "<elided base index>" 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
}

protected def nameStr(str: String): String = str
protected def treeStr(str: String): String = str
protected def lengthStr(str: String): String = str
Expand Down
5 changes: 4 additions & 1 deletion compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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(_)), ", ")
Expand Down
57 changes: 54 additions & 3 deletions compiler/src/dotty/tools/dotc/transform/Pickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)

Expand All @@ -283,4 +303,35 @@ 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): Unit = {
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 -- $check after-printing.txt`
| actual output:
|$lines%\n%""")
}

/** Reuse diff logic from compiler/test/dotty/tools/vulpix/FileDiff.scala */
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
Option.when(!matches(actualLines, expectLines))(actualLines)

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
Comment on lines +329 to +330
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still necessary? IIUC you elide paths entirely now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, I could remove that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to do another PR to warn about not adding the -Ytest-pickler-check flag in the presence of .tastycheck so it can be picked up there

}

private def matches(actual: Seq[String], expect: Seq[String]): Boolean = {
actual.length == expect.length
&& actual.lazyZip(expect).forall(matches)
}
}
Loading