Fix #6603: Make privateWithin force less

At this point, ModuleCompleters and SymbolLoaders are the only two kinds
of completers which need to set privateWithin, this is now enforced by
the setter (which is therefore renamed to `setPrivateWithin` to make it
clearer that it doesn't just set a field) and taken advantage of by the
getter to avoid forcing unless necessary.

This is the last piece needed to finally fix #6603. Unfortunately, the
cumulative changes to completions done so far have caused cyclic
reference errors to pop up in new situations, including bootstrapping
Dotty itself. This is fixed by the remaining commits in this PR.

Author: smarter

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

@@ -277,11 +277,31 @@ object SymDenotations {
 
     /** The privateWithin boundary, NoSymbol if no boundary is given.
      */
-    final def privateWithin(implicit ctx: Context): Symbol = { ensureCompleted(); myPrivateWithin }
+    @tailrec
+    final def privateWithin(implicit ctx: Context): Symbol = myInfo match {
+      case myInfo: ModuleCompleter =>
+        // Instead of completing the ModuleCompleter, we can get `privateWithin`
+        // directly from the module class, which might require less completions.
+        myInfo.moduleClass.privateWithin
+      case _: SymbolLoader =>
+         // Completing a SymbolLoader might call `setPrivateWithin()`
+        completeOnce()
+        privateWithin
+      case _ =>
+        // Otherwise, no completion is necessary, see the preconditions of `markAbsent()`.
+        myPrivateWithin
+    }
 
-    /** Set privateWithin. */
-    protected[dotc] final def privateWithin_=(sym: Symbol): Unit =
-      myPrivateWithin = sym
+    /** Set privateWithin, prefer setting it at symbol-creation time instead if
+     *  possible.
+     *  @pre `isCompleting` is false, or this is a ModuleCompleter or SymbolLoader
+     */
+    protected[dotc] final def setPrivateWithin(pw: Symbol)(implicit ctx: Context): Unit = {
+      if (isCompleting)
+        assert(myInfo.isInstanceOf[ModuleCompleter | SymbolLoader],
+          s"Illegal call to `setPrivateWithin($pw)` while completing $this using completer $myInfo")
+      myPrivateWithin = pw
+    }
 
     /** The annotations of this denotation */
     final def annotations(implicit ctx: Context): List[Annotation] = {
@@ -2227,7 +2247,7 @@ object SymDenotations {
         // is to have the module class completer set the annotations of both the
         // class and the module.
       denot.info = moduleClass.typeRef
-      denot.privateWithin = from.privateWithin
+      denot.setPrivateWithin(from.privateWithin)
     }
   }
 
@@ -2241,7 +2261,7 @@ object SymDenotations {
         case _ =>
           ErrorType(errMsg)
       }
-      denot.privateWithin = NoSymbol
+      denot.setPrivateWithin(NoSymbol)
     }
 
     def complete(denot: SymDenotation)(implicit ctx: Context): Unit = {

compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala

@@ -159,9 +159,9 @@ class ClassfileParser(
 
       val privateWithin = getPrivateWithin(jflags)
 
-      classRoot.privateWithin = privateWithin
-      moduleRoot.privateWithin = privateWithin
-      moduleRoot.sourceModule.privateWithin = privateWithin
+      classRoot.setPrivateWithin(privateWithin)
+      moduleRoot.setPrivateWithin(privateWithin)
+      moduleRoot.sourceModule.setPrivateWithin(privateWithin)
 
       for (i <- 0 until in.nextChar) parseMember(method = false)
       for (i <- 0 until in.nextChar) parseMember(method = true)

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

@@ -546,7 +546,7 @@ class TreeUnpickler(reader: TastyReader,
             rootd.info = adjustIfModule(
                 new Completer(subReader(start, end)) with SymbolLoaders.SecondCompleter)
             rootd.flags = flags &~ Touched // allow one more completion
-            rootd.privateWithin = privateWithin
+            rootd.setPrivateWithin(privateWithin)
             seenRoots += rootd.symbol
             rootd.symbol
           case _ =>

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

@@ -479,7 +479,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
       if (owner == defn.ScalaPackageClass && ((name eq tpnme.Serializable) || (name eq tpnme.Product)))
         denot.setFlag(NoInits)
 
-      denot.privateWithin = privateWithin
+      denot.setPrivateWithin(privateWithin)
       denot.info = completer
       denot.symbol
     }

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

@@ -332,7 +332,7 @@ class Namer { typer: Typer =>
         if (prev.exists) {
           prev.flags = flags1
           prev.info = infoFn(prev.asInstanceOf[S])
-          prev.privateWithin = privateWithin
+          prev.setPrivateWithin(privateWithin)
           prev
         }
         else symFn(flags1, infoFn, privateWithin)

compiler/test/dotty/tools/dotc/BootstrappedOnlyCompilationTests.scala

@@ -57,6 +57,15 @@ class BootstrappedOnlyCompilationTests extends ParallelTesting {
         ),
         withCompilerOptions
       ),
+      compileList(
+        "testIssue6603",
+        List(
+          "compiler/src/dotty/tools/dotc/ast/Desugar.scala",
+          "compiler/src/dotty/tools/dotc/ast/Trees.scala",
+          "compiler/src/dotty/tools/dotc/core/Types.scala"
+        ),
+        withCompilerOptions
+      ),
     ).checkCompile()
   }
 

tests/neg/exports.check

@@ -41,9 +41,9 @@
    |             ^^^
    |             no eligible member foo at this.foo
    |             this.foo.foo cannot be exported because it is already a member of class Foo
--- [E046] Cyclic Error: tests/neg/exports.scala:45:11 ------------------------------------------------------------------
-45 |  val bar: Bar = new Bar // error: cyclic reference
-   |           ^
-   |           Cyclic reference involving value bar
-
-longer explanation available when compiling with `-explain`
+-- [E120] Duplicate Symbol Error: tests/neg/exports.scala:46:13 --------------------------------------------------------
+46 |  export bar._  // error: double definition
+   |             ^
+   |             Double definition:
+   |             val bar: Bar in class Baz at line 45 and
+   |             final def bar: => => Bar(Baz.this.bar.baz.bar)(Baz.this.bar.bar) in class Baz at line 46

tests/neg/exports.scala

@@ -42,8 +42,8 @@ class Foo {
 }
 
 class Baz {
-  val bar: Bar = new Bar // error: cyclic reference
-  export bar._
+  val bar: Bar = new Bar
+  export bar._  // error: double definition
 }
 class Bar {
   val baz: Baz = new Baz

tests/neg/i4368.scala

@@ -149,13 +149,13 @@ object Test9 {
 object i4369 {
   trait X { self =>
     type R <: Z
-    type Z >: X { type R = self.R; type Z = self.R }
+    type Z >: X { type R = self.R; type Z = self.R } // error: cyclic // error: cyclic // error: cyclic
   }
-  class Foo extends X { type R = Foo; type Z = Foo } // error: too deep
+  class Foo extends X { type R = Foo; type Z = Foo }
 }
 object i4370 {
-  class Foo { type R = A } // error: cyclic
-  type A = List[Foo#R]
+  class Foo { type R = A }
+  type A = List[Foo#R] // error: cyclic
 }
 object i4371 {
   class Foo { type A = Boo#B } // error: cyclic
@@ -170,4 +170,4 @@ object i318 {
   val y: Y = ???
   val a: y.A = ??? // error: too deep
   val b: y.B = a   // error: too deep
-}
+}

tests/neg/i4370.scala

@@ -1,4 +1,4 @@
 object App {
-  class Foo { type R = A } // error
-  type A = List[Foo#R]
+  class Foo { type R = A }
+  type A = List[Foo#R] // error
 }