Merge pull request #10530 from dotty-staging/reconcile-noinits-with-scala2

Fix #9970: Reconcile how Scala 3 and 2 decide whether a trait has $init$.

Author: sjrd

compiler/src/dotty/tools/dotc/ast/TreeInfo.scala

@@ -316,7 +316,7 @@ trait UntypedTreeInfo extends TreeInfo[Untyped] { self: Trees.Instance[Untyped]
   /**  The largest subset of {NoInits, PureInterface} that a
    *   trait or class enclosing this statement can have as flags.
    */
-  def defKind(tree: Tree)(using Context): FlagSet = unsplice(tree) match {
+  private def defKind(tree: Tree)(using Context): FlagSet = unsplice(tree) match {
     case EmptyTree | _: Import => NoInitsInterface
     case tree: TypeDef => if (tree.isClassDef) NoInits else NoInitsInterface
     case tree: DefDef =>
@@ -402,8 +402,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
         || sym.owner == defn.StringClass
         || defn.pureMethods.contains(sym)
       if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol) // A constant expression with pure arguments is pure.
-          || (fn.symbol.isStableMember && !fn.symbol.is(Lazy))
-          || fn.symbol.isPrimaryConstructor && fn.symbol.owner.isNoInitsClass) // TODO: include in isStable?
+          || (fn.symbol.isStableMember && !fn.symbol.is(Lazy))) // constructors of no-inits classes are stable
         minOf(exprPurity(fn), args.map(exprPurity)) `min` Pure
       else if (fn.symbol.is(Erased)) Pure
       else if (fn.symbol.isStableMember) /* && fn.symbol.is(Lazy) */
@@ -459,7 +458,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
     else if tree.tpe.isInstanceOf[ConstantType] then PurePath
     else if (!sym.isStableMember) Impure
     else if (sym.is(Module))
-      if (sym.moduleClass.isNoInitsClass) PurePath else IdempotentPath
+      if (sym.moduleClass.isNoInitsRealClass) PurePath else IdempotentPath
     else if (sym.is(Lazy)) IdempotentPath
     else if sym.isAllOf(Inline | Param) then Impure
     else PurePath

compiler/src/dotty/tools/dotc/core/Definitions.scala

@@ -181,7 +181,7 @@ class Definitions {
   }
 
   private def completeClass(cls: ClassSymbol, ensureCtor: Boolean = true): ClassSymbol = {
-    if (ensureCtor) ensureConstructor(cls, EmptyScope)
+    if (ensureCtor) ensureConstructor(cls, cls.denot.asClass, EmptyScope)
     if (cls.linkedClass.exists) cls.linkedClass.markAbsent()
     cls
   }

compiler/src/dotty/tools/dotc/core/Flags.scala

@@ -293,7 +293,14 @@ object Flags {
    */
   val (Abstract @ _, _, _) = newFlags(23, "abstract")
 
-  /** Lazy val or method is known or assumed to be stable and realizable */
+  /** Lazy val or method is known or assumed to be stable and realizable.
+   *
+   *  For a trait constructor, this is set if and only if owner.is(NoInits),
+   *  including for Java interfaces and for Scala 2 traits. It will be used by
+   *
+   *  - the purity analysis used by the inliner to decide whether it is safe to elide, and
+   *  - the TASTy reader of Scala 2.13, to determine whether there is a $init$ method.
+   */
   val (_, StableRealizable @ _, _) = newFlags(24, "<stable>")
 
   /** A case parameter accessor */
@@ -319,7 +326,9 @@ object Flags {
 
   /** Variable is accessed from nested function
    *    /
-   *  Trait does not have fields or initialization code.
+   *  Trait does not have own fields or initialization code or class does not
+   *  have own or inherited initialization code.
+   *
    *  Warning: NoInits is set during regular typer pass, should be tested only after typer.
    */
   val (_, Captured @ _, NoInits @ _) = newFlags(32, "<captured>", "<noinits>")
@@ -536,7 +545,6 @@ object Flags {
   val DeferredOrTermParamOrAccessor: FlagSet = Deferred | ParamAccessor | TermParam           // term symbols without right-hand sides
   val DeferredOrTypeParam: FlagSet           = Deferred | TypeParam                           // type symbols without right-hand sides
   val EnumValue: FlagSet                     = Enum | StableRealizable                        // A Scala enum value
-  val StableOrErased: FlagSet                = Erased | StableRealizable                      // Assumed to be pure
   val FinalOrInline: FlagSet                 = Final | Inline
   val FinalOrModuleClass: FlagSet            = Final | ModuleClass                            // A module class or a final class
   val EffectivelyFinalFlags: FlagSet         = Final | Private

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -725,11 +725,11 @@ object SymDenotations {
       isType || is(StableRealizable) || exists && !isUnstableValue
     }
 
-    /** Is this a denotation of a class that does not have - either direct or inherited -
+    /** Is this a denotation of a real class that does not have - either direct or inherited -
      *  initialization code?
      */
-    def isNoInitsClass(using Context): Boolean =
-      isClass &&
+    def isNoInitsRealClass(using Context): Boolean =
+      isRealClass &&
       (asClass.baseClasses.forall(_.is(NoInits)) || defn.isAssuredNoInits(symbol))
 
     /** Is this a "real" method? A real method is a method which is:

compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala

@@ -59,9 +59,14 @@ object Scala2Unpickler {
     denot.info = PolyType.fromParams(denot.owner.typeParams, denot.info)
   }
 
-  def ensureConstructor(cls: ClassSymbol, scope: Scope)(using Context): Unit = {
+  def ensureConstructor(cls: ClassSymbol, clsDenot: ClassDenotation, scope: Scope)(using Context): Unit = {
     if (scope.lookup(nme.CONSTRUCTOR) == NoSymbol) {
       val constr = newDefaultConstructor(cls)
+      // Scala 2 traits have a constructor iff they have initialization code
+      // In dotc we represent that as !StableRealizable, which is also owner.is(NoInits)
+      if clsDenot.flagsUNSAFE.is(Trait) then
+        constr.setFlag(StableRealizable)
+        clsDenot.setFlag(NoInits)
       addConstructorTypeParams(constr)
       cls.enter(constr, scope)
     }
@@ -95,7 +100,7 @@ object Scala2Unpickler {
       if (tsym.exists) tsym.setFlag(TypeParam)
       else denot.enter(tparam, decls)
     }
-    if (!denot.flagsUNSAFE.isAllOf(JavaModule)) ensureConstructor(denot.symbol.asClass, decls)
+    if (!denot.flagsUNSAFE.isAllOf(JavaModule)) ensureConstructor(cls, denot, decls)
 
     val scalacCompanion = denot.classSymbol.scalacLinkedClass
 
@@ -465,11 +470,13 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
       denot.setFlag(flags)
       denot.resetFlag(Touched) // allow one more completion
 
-      // Temporary measure, as long as we do not read these classes from Tasty.
-      // Scala-2 classes don't have NoInits set even if they are pure. We override this
-      // for Product and Serializable so that case classes can be pure. A full solution
-      // requires that we read all Scala code from Tasty.
-      if (owner == defn.ScalaPackageClass && ((name eq tpnme.Serializable) || (name eq tpnme.Product)))
+      // Temporary measure, as long as we do not recompile these traits with Scala 3.
+      // Scala 2 is more aggressive when it comes to defining a $init$ method than Scala 3.
+      // Any concrete definition, even a `def`, causes a trait to receive a $init$ method
+      // to be created, even if it does not do anything, and hence causes not have the NoInits flag.
+      // We override this for Product so that cases classes can be pure.
+      // A full solution requires that we compile Product with Scala 3 in the future.
+      if (owner == defn.ScalaPackageClass && (name eq tpnme.Product))
         denot.setFlag(NoInits)
 
       denot.setPrivateWithin(privateWithin)

compiler/src/dotty/tools/dotc/typer/Inliner.scala

@@ -555,11 +555,10 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
           meth.name == nme.apply &&
           meth.flags.is(Synthetic) &&
           meth.owner.linkedClass.is(Case) &&
-          cls.isNoInitsClass
+          cls.isNoInitsRealClass
         }
         if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol) // A constant expression with pure arguments is pure.
-            || (fn.symbol.isStableMember && !fn.symbol.is(Lazy))
-            || fn.symbol.isPrimaryConstructor && fn.symbol.owner.isNoInitsClass) // TODO: include in isStable?
+            || (fn.symbol.isStableMember && !fn.symbol.is(Lazy))) // constructors of no-inits classes are stable
           apply(fn) && args.forall(apply)
         else if (isCaseClassApply)
           args.forall(apply)
@@ -864,7 +863,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
     def reduceProjection(tree: Tree)(using Context): Tree = {
       if (ctx.debug) inlining.println(i"try reduce projection $tree")
       tree match {
-        case Select(NewInstance(cls, args, prefix, precomputed), field) if cls.isNoInitsClass =>
+        case Select(NewInstance(cls, args, prefix, precomputed), field) if cls.isNoInitsRealClass =>
           def matches(param: Symbol, selection: Symbol): Boolean =
             param == selection || {
               selection.name match {

compiler/src/dotty/tools/dotc/typer/Namer.scala

@@ -1197,7 +1197,10 @@ class Namer { typer: Typer =>
       cls.info = avoidPrivateLeaks(cls)
       cls.baseClasses.foreach(_.invalidateBaseTypeCache()) // we might have looked before and found nothing
       cls.setNoInitsFlags(parentsKind(parents), untpd.bodyKind(rest))
-      if (cls.isNoInitsClass) cls.primaryConstructor.setFlag(StableRealizable)
+      val ctorStable =
+        if cls.is(Trait) then cls.is(NoInits)
+        else cls.isNoInitsRealClass
+      if ctorStable then cls.primaryConstructor.setFlag(StableRealizable)
       processExports(using localCtx)
       defn.patchStdLibClass(denot.asClass)
     }

tests/run-custom-args/tasty-inspector/i9970.scala

@@ -0,0 +1,89 @@
+import scala.quoted._
+import scala.tasty.inspector._
+
+/* Test that the constructor of a trait has the StableRealizable trait if and
+ * only if the trait has NoInits. This is used by the TASTy reader in Scala 2
+ * to determine whether the constructor should be kept or discarded, and
+ * consequently whether calls to $init$ should be emitted for the trait or not.
+ */
+
+// Definitions to be inspected
+
+trait I9970IOApp {
+  protected val foo = 23
+
+  def run(args: List[String]): Int
+
+  final def main(args: Array[String]): Unit = {
+    sys.exit(run(args.toList))
+  }
+}
+
+object I9970IOApp {
+  trait Simple extends I9970IOApp {
+    def run: Unit
+
+    final def run(args: List[String]): Int = {
+      run
+      0
+    }
+  }
+}
+
+// TASTy Inspector test boilerplate
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    // Artefact of the current test infrastructure
+    // TODO improve infrastructure to avoid needing this code on each test
+    val classpath = dotty.tools.dotc.util.ClasspathFromClassloader(this.getClass.getClassLoader).split(java.io.File.pathSeparator).find(_.contains("runWithCompiler")).get
+    val allTastyFiles = dotty.tools.io.Path(classpath).walkFilter(_.extension == "tasty").map(_.toString).toList
+    val tastyFiles = allTastyFiles.filter(_.contains("I9970"))
+
+    new TestInspector().inspectTastyFiles(tastyFiles)
+  }
+}
+
+// Inspector that performs the actual tests
+
+class TestInspector() extends TastyInspector:
+
+  private var foundIOApp: Boolean = false
+  private var foundSimple: Boolean = false
+
+  protected def processCompilationUnit(using Quotes)(root: quotes.reflect.Tree): Unit =
+    foundIOApp = false
+    foundSimple = false
+    inspectClass(root)
+    // Sanity check to make sure that our pattern matches are not simply glossing over the things we want to test
+    assert(foundIOApp, "the inspector did not encounter IOApp")
+    assert(foundSimple, "the inspect did not encounter IOApp.Simple")
+
+  private def inspectClass(using Quotes)(tree: quotes.reflect.Tree): Unit =
+    import quotes.reflect._
+    tree match
+      case t: PackageClause =>
+        t.stats.foreach(inspectClass(_))
+
+      case t: ClassDef if t.name.endsWith("$") =>
+        t.body.foreach(inspectClass(_))
+
+      case t: ClassDef =>
+        t.name match
+          case "I9970IOApp" =>
+            foundIOApp = true
+            // Cannot test the following because NoInits is not part of the quotes API
+            //assert(!t.symbol.flags.is(Flags.NoInits))
+            assert(!t.constructor.symbol.flags.is(Flags.StableRealizable))
+
+          case "Simple" =>
+            foundSimple = true
+            // Cannot test the following because NoInits is not part of the quotes API
+            //assert(t.symbol.flags.is(Flags.NoInits))
+            assert(t.constructor.symbol.flags.is(Flags.StableRealizable))
+
+          case _ =>
+            assert(false, s"unexpected ClassDef '${t.name}'")
+
+      case _ =>
+  end inspectClass