Adjust purity of trait with lazy member (#23788)

Fixes #23245 

jchyb:

Previously, the behavior for setting `NoInits` and `PureInterface` flags
for class symbols in `TreeInfo.defKind` (when compiling from a source)
and in `TreeUnpickler` (when reading Tasty) was inconsistent. This was
problematic, as the `PureInterface` flag dictated whether outer
accessors were generated in `ExplicitOuter` phase (in the
`needsOuterIfReferenced` method), so in the issue minimization, the
inherited trait would have the outer accessors defined, and the
inheriting object would then not implement them. This led to runtime
`MethodNotFound` errors.
The problem was specifically with lazy vals, which were treated as if
they did not have an rhs.

Author: hamzaremmal

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

@@ -483,9 +483,11 @@ trait UntypedTreeInfo extends TreeInfo[Untyped] { self: Trees.Instance[Untyped]
    */
   private def defKind(tree: Tree)(using Context): FlagSet = unsplice(tree) match {
     case EmptyTree | _: Import => NoInitsInterface
-    case tree: TypeDef if Feature.shouldBehaveAsScala2 =>
-      if (tree.isClassDef) EmptyFlags else NoInitsInterface
-    case tree: TypeDef => if (tree.isClassDef) NoInits else NoInitsInterface
+    case tree: TypeDef =>
+      if tree.isClassDef then
+        if Feature.shouldBehaveAsScala2 then EmptyFlags
+        else NoInits
+      else NoInitsInterface
     case tree: DefDef =>
       if tree.unforcedRhs == EmptyTree
          && tree.paramss.forall {
@@ -494,8 +496,6 @@ trait UntypedTreeInfo extends TreeInfo[Untyped] { self: Trees.Instance[Untyped]
             }
       then
         NoInitsInterface
-      else if tree.mods.is(Given) && tree.paramss.isEmpty then
-        EmptyFlags // might become a lazy val: TODO: check whether we need to suppress NoInits once we have new lazy val impl
       else if Feature.shouldBehaveAsScala2 then
         EmptyFlags
       else

compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

@@ -809,8 +809,8 @@ class TreeUnpickler(reader: TastyReader,
             if (sym.isTerm && !sym.isOneOf(DeferredOrLazyOrMethod))
               initsFlags = EmptyFlags
             else if (sym.isClass ||
-              sym.is(Method, butNot = Deferred) && !sym.isConstructor)
-              initsFlags &= NoInits
+              sym.isOneOf(Lazy | Method, butNot = Deferred) && !sym.isConstructor)
+              initsFlags &= NoInits // i.e. initsFlags &~= PureInterface
           case IMPORT | EXPORT =>
             skipTree()
           case PACKAGE =>

compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala

@@ -3,6 +3,7 @@ package dotc
 package transform
 
 import MegaPhase.*
+import ast.tpd.*
 import core.DenotTransformers.*
 import core.Symbols.*
 import core.Contexts.*
@@ -15,10 +16,8 @@ import core.Names.*
 import core.NameOps.*
 import core.NameKinds.SuperArgName
 
-import dotty.tools.dotc.ast.tpd
-
-import collection.mutable
 import scala.annotation.tailrec
+import scala.collection.mutable
 
 /** This phase adds outer accessors to classes and traits that need them.
  *  Compared to Scala 2.x, it tries to minimize the set of classes
@@ -36,7 +35,6 @@ import scala.annotation.tailrec
  */
 class ExplicitOuter extends MiniPhase with InfoTransformer { thisPhase =>
   import ExplicitOuter.*
-  import ast.tpd.*
 
   override def phaseName: String = ExplicitOuter.name
 
@@ -64,7 +62,7 @@ class ExplicitOuter extends MiniPhase with InfoTransformer { thisPhase =>
    *  Furthermore, if a parent trait might have an outer accessor,
    *  provide an implementation for the outer accessor by computing the parent's
    *  outer from the parent type prefix. If the trait ends up not having an outer accessor
-   *  after all, the implementation is redundant, but does not harm.
+   *  after all, the implementation is redundant, but does no harm.
    *  The same logic is not done for non-trait parent classes because for them the outer
    *  pointer is passed in the super constructor, which will be implemented later in
    *  a separate phase which needs to run after erasure. However, we make sure here
@@ -111,7 +109,7 @@ class ExplicitOuter extends MiniPhase with InfoTransformer { thisPhase =>
     else impl
   }
 
-  override def transformClosure(tree: Closure)(using Context): tpd.Tree = {
+  override def transformClosure(tree: Closure)(using Context): Tree = {
     if (tree.tpt ne EmptyTree) {
       val cls = tree.tpt.asInstanceOf[TypeTree].tpe.classSymbol
       if (cls.exists && hasOuter(cls.asClass))
@@ -122,7 +120,6 @@ class ExplicitOuter extends MiniPhase with InfoTransformer { thisPhase =>
 }
 
 object ExplicitOuter {
-  import ast.tpd.*
 
   val name: String = "explicitOuter"
   val description: String = "add accessors to outer classes from nested ones"
@@ -217,11 +214,12 @@ object ExplicitOuter {
    *  - we need to potentially pass along outer to a parent class or trait
    */
   private def needsOuterAlways(cls: ClassSymbol)(using Context): Boolean =
-    needsOuterIfReferenced(cls) &&
-    (!hasOnlyLocalInstantiation(cls) || // needs outer because we might not know whether outer is referenced or not
-     cls.mixins.exists(needsOuterIfReferenced) || // needs outer for parent traits
-     cls.info.parents.exists(parent => // needs outer to potentially pass along to parent
-       needsOuterIfReferenced(parent.classSymbol.asClass)))
+       needsOuterIfReferenced(cls)
+    && (!hasOnlyLocalInstantiation(cls) // needs outer because we might not know whether outer is referenced or not
+        || cls.mixins.exists(needsOuterIfReferenced) // needs outer for parent traits
+        || cls.info.parents.exists: parent => // needs outer to potentially pass along to parent
+             needsOuterIfReferenced(parent.classSymbol.asClass)
+    )
 
   /** Class is only instantiated in the compilation unit where it is defined */
   private def hasOnlyLocalInstantiation(cls: ClassSymbol)(using Context): Boolean =

tests/run/i23245a/api.scala

@@ -0,0 +1,23 @@
+
+package logadapter:
+
+  trait AbstractLogAdapter:
+    def info(message: String): Unit
+
+  trait AbstractApi[T <: AbstractLogAdapter]:
+    def logAdapterFor(loggerName: String): T
+    trait SelfLogging:
+      given adapter: T = logAdapterFor(this.getClass.getName)
+      // workaround:
+      //given () => T = logAdapterFor(this.getClass.getName)
+      // or
+      //private val adapter = logAdapterFor(this.getClass.getName)
+      //given T = adapter
+      // or just pollute the interface so it's never taken as pure
+      //private val z = 42
+
+  object Api extends AbstractApi[LogAdapter]:
+    def logAdapterFor(loggerName: String): LogAdapter = new LogAdapter(loggerName)
+
+  class LogAdapter(loggerName: String) extends AbstractLogAdapter:
+    def info(message: String): Unit = System.err.println(s"INFO [${loggerName}] ${message}")

tests/run/i23245a/test_2.scala

@@ -0,0 +1,6 @@
+
+
+object Test extends logadapter.Api.SelfLogging:
+  def main(args: Array[String]): Unit =
+    summon[logadapter.LogAdapter].info("Hello")
+

tests/run/i23245b/outer.scala

@@ -0,0 +1,10 @@
+
+trait T:
+  def f = 42
+  trait D:
+    lazy val g = f
+
+object C extends T
+
+// D parent of Z is taken as PureInterface under separate compilation
+// and thus doesn't get an outer.

tests/run/i23245b/test_2.scala

@@ -0,0 +1,5 @@
+
+object Z extends C.D
+
+@main def Test = println:
+  Z.g

tests/run/i23245c/outer.scala

@@ -0,0 +1,7 @@
+
+trait T:
+  def f = 42
+  trait D:
+    lazy val g = f
+
+object C extends T

tests/run/i23245c/test.scala

@@ -0,0 +1,5 @@
+
+object Z extends C.D
+
+@main def Test = println:
+  Z.g