scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.88k stars 1.06k forks source link

tasty error when declaring subclass field type and overriding (thrown when publishing) #12834

Closed hughsimpson closed 3 years ago

hughsimpson commented 3 years ago

Compiler version

3.0.0

Minimized code

class A(val ref: Option[B]) {}
class B(override val ref: Option[B]) extends A(ref = ref) {}

Output

runs fine, but when publishing with sbt publishLocal throws

[error] 8 |class B(override val ref: Option[B]) extends A(ref = ref) {}
[error]   |                                             ^^^^^^^^^^^
[error]   |undefined: new bug.A # -1: TermRef(TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>)),object bug),A),<init>) at readTasty

Expectation

This should be fine (is a regression from scala 2)

romanowski commented 3 years ago

It seems that this tasty inspector does not handle cycles well. The problem can be minimized even further:

class A(val ref: Option[B])
class B extends A(None)
smarter commented 3 years ago

This isn't specific to the inspector, it's also reproducible when compiling with -Ythrough-tasty, we can add something like this to find where the undefined TermRef is constructed:

diff --git compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
index c8220d7e760..1e4d6d6235a 100644
--- compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
+++ compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
@@ -1290,6 +1290,11 @@ class TreeUnpickler(reader: TastyReader,
       }

       val tree = if (tag < firstLengthTreeTag) readSimpleTerm() else readLengthTerm()
+      tree.tpe match
+        case tp: TermRef =>
+          assert(tp.denot.exists, tp)
+        case _ =>
+
       if (!tree.isInstanceOf[TypTree]) // FIXME: Necessary to avoid self-type cyclic reference in tasty_tools
         tree.overwriteType(tree.tpe.simplified)
       setSpan(start, tree)

which gives us:

java.lang.AssertionError: assertion failed: TermRef(TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,module class <root>)),module class <empty>)),A),<init>)
        at scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:8)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readTerm(TreeUnpickler.scala:1295)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readLengthTerm$1(TreeUnpickler.scala:1132)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readTerm(TreeUnpickler.scala:1292)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.$anonfun$4(TreeUnpickler.scala:927)
        at dotty.tools.tasty.TastyReader.collectWhile(TastyReader.scala:137)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readTemplate(TreeUnpickler.scala:930)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readNewDef(TreeUnpickler.scala:857)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readIndexedDef(TreeUnpickler.scala:776)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$Completer.complete(TreeUnpickler.scala:122)
        at dotty.tools.dotc.core.SymDenotations$SymDenotation.completeFrom(SymDenotations.scala:167)
        at dotty.tools.dotc.core.Denotations$Denotation.completeInfo$1(Denotations.scala:188)
        at dotty.tools.dotc.core.Denotations$Denotation.info(Denotations.scala:190)
        at dotty.tools.dotc.core.Denotations$Denotation.completeInfo$1(Denotations.scala:188)
        at dotty.tools.dotc.core.Denotations$Denotation.info(Denotations.scala:190)
        at dotty.tools.dotc.core.SymDenotations$SymDenotation.ensureCompleted(SymDenotations.scala:369)
        at dotty.tools.dotc.core.SymDenotations$SymDenotation.flags(SymDenotations.scala:65)
        at dotty.tools.dotc.core.SymDenotations$SymDenotation.is(SymDenotations.scala:116)
        at dotty.tools.dotc.typer.Checking$NotPrivate$1.isLeaked(Checking.scala:569)
        at dotty.tools.dotc.typer.Checking$NotPrivate$1.apply(Checking.scala:580)
        at dotty.tools.dotc.core.Types$TypeMap.op$proxy16$1(Types.scala:5281)
        at dotty.tools.dotc.core.Types$TypeMap.mapArgs(Types.scala:5281)
        at dotty.tools.dotc.core.Types$TypeMap.mapOver(Types.scala:5316)
        at dotty.tools.dotc.typer.Checking$NotPrivate$1.apply(Checking.scala:610)
        at dotty.tools.dotc.typer.Checking$.checkNoPrivateLeaks(Checking.scala:614)
        at dotty.tools.dotc.typer.TypeAssigner.avoidPrivateLeaks(TypeAssigner.scala:47)
        at dotty.tools.dotc.typer.TypeAssigner.avoidPrivateLeaks$(TypeAssigner.scala:19)
        at dotty.tools.dotc.typer.Typer.avoidPrivateLeaks(Typer.scala:106)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readNewDef(TreeUnpickler.scala:889)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readIndexedDef(TreeUnpickler.scala:776)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readIndexedParams$$anonfun$2(TreeUnpickler.scala:1037)
        at dotty.tools.tasty.TastyReader.collectWhile(TastyReader.scala:137)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readIndexedParams(TreeUnpickler.scala:1037)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readTemplate(TreeUnpickler.scala:916)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readNewDef(TreeUnpickler.scala:857)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readIndexedDef(TreeUnpickler.scala:776)
        at dotty.tools.dotc.core.tasty.TreeUnpickler$Completer.complete(TreeUnpickler.scala:122)
        at dotty.tools.dotc.core.SymDenotations$SymDenotation.completeFrom(SymDenotations.scala:167)
        at dotty.tools.dotc.core.Denotations$Denotation.completeInfo$1(Denotations.scala:188)
        at dotty.tools.dotc.core.Denotations$Denotation.info(Denotations.scala:190)
        at dotty.tools.dotc.core.SymDenotations$SymDenotation.ensureCompleted(SymDenotations.scala:369)
        at dotty.tools.dotc.core.Symbols$ClassSymbol.rootTreeContaining(Symbols.scala:393)
        at dotty.tools.dotc.core.Symbols$ClassSymbol.rootTree(Symbols.scala:384)
        at dotty.tools.dotc.fromtasty.ReadTasty.compilationUnit$1(ReadTasty.scala:40)
        at dotty.tools.dotc.fromtasty.ReadTasty.readTASTY(ReadTasty.scala:70)
        at dotty.tools.dotc.fromtasty.ReadTasty.runOn$$anonfun$1(ReadTasty.scala:25)
        at scala.collection.immutable.List.flatMap(List.scala:293)
        at dotty.tools.dotc.fromtasty.ReadTasty.runOn(ReadTasty.scala:25)
        at dotty.tools.dotc.Run.runPhases$4$$anonfun$4(Run.scala:205)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
        at scala.collection.ArrayOps$.foreach$extension(ArrayOps.scala:1323)
        at dotty.tools.dotc.Run.runPhases$5(Run.scala:216)
        at dotty.tools.dotc.Run.compileUnits$$anonfun$1(Run.scala:224)
        at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
        at dotty.tools.dotc.util.Stats$.maybeMonitored(Stats.scala:67)
        at dotty.tools.dotc.Run.compileUnits(Run.scala:231)
        at dotty.tools.dotc.Run.compileUnits(Run.scala:172)
        at dotty.tools.dotc.fromtasty.TASTYRun.compile(TASTYRun.scala:11)
        at dotty.tools.dotc.Driver.doCompile(Driver.scala:39)
        at dotty.tools.dotc.Driver.process(Driver.scala:199)
        at dotty.tools.dotc.Driver.process(Driver.scala:167)
        at dotty.tools.dotc.Driver.process(Driver.scala:179)
        at dotty.tools.dotc.fromtasty.Debug$.main(Debug.scala:52)
        at dotty.tools.dotc.fromtasty.Debug.main(Debug.scala)

so basically when we unpickle A we first unpickle the class parameters like ref, once we have a symbol for that we run avoidPrivateLeaks on it which forces the symbol info, which forces B, which forces the parents of B which contain a reference to the constructor of A which hasn't been unpickled yet, hence the error. If we could avoid running avoidPrivateLeaks we wouldn't run into this issue. The job of avoidPrivateLeaks in the unpickler is to get rid of any inaccessible type alias by de-aliasing them, as in:

class Foo {
  private type T = Int
  val x: T // dealiased to Int when unpickling
}

but I don't think that class parameters can refer to type aliases defined in the class, since they're always of the form class Foo(val someParam: ...), so we might be able to get away with just not running avoidPrivateLeaks at all on them:

diff --git compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
index c8220d7e760..2c32e3c5648 100644
--- compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
+++ compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
@@ -885,7 +885,7 @@ class TreeUnpickler(reader: TastyReader,
       }
       goto(end)
       setSpan(start, tree)
-      if (!sym.isType) // Only terms might have leaky aliases, see the documentation of `checkNoPrivateLeaks`
+      if (!sym.isType && !sym.is(ParamAccessor)) // Only terms might have leaky aliases, see the documentation of `checkNoPrivateLeaks`
         sym.info = ta.avoidPrivateLeaks(sym)

       if (ctx.settings.YreadComments.value) {
smarter commented 3 years ago

By the way, as a work around you can disable scaladoc generation:

Compile / doc / sources := Seq()