Avoid forcing ctors & parents which caused cycles

Author: dwijnand

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

@@ -93,7 +93,24 @@ object desugar {
     override def ensureCompletions(using Context): Unit = {
       def completeConstructor(sym: Symbol) =
         sym.infoOrCompleter match {
-          case completer: Namer#ClassCompleter =>
+          case completer: Namer#ClassCompleter if !sym.isCompleting =>
+            // An example, derived from tests/run/t6385.scala
+            //
+            //     class Test():
+            //       def t1: Foo = Foo(1)
+            //     final case class Foo(value: Int)
+            //
+            // Here's the sequence of events:
+            //  * The symbol for Foo.apply is forced to complete
+            //  * The symbol for the `value` parameter of the apply method is forced to complete
+            //  * Completing that value parameter requires typing its type, which is a DerivedTypeTrees,
+            //    which only types if it has an OriginalSymbol.
+            //  * So if the case class hasn't been completed, we need (at least) its constructor to be completed
+            //
+            // Test tests/neg/i9294.scala is an example of why isCompleting is necessary.
+            // Annotations are added while completing the constructor,
+            // so the back reference to foo reaches here which re-initiates the constructor completion.
+            // So we just skip, as completion is already being triggered.
             completer.completeConstructor(sym)
           case _ =>
         }

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

@@ -272,7 +272,7 @@ object NamerOps:
    *  where
    *
    *      <context-bound-companion> is the CBCompanion type created in Definitions
-   *      withnessRefK is a refence to the K'th witness.
+   *      withnessRefK is a reference to the K'th witness.
    *
    *  The companion has the same access flags as the original type.
    */

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

@@ -267,7 +267,9 @@ class TypeApplications(val self: Type) extends AnyVal {
    */
   def hkResult(using Context): Type = self.dealias match {
     case self: TypeRef =>
-      if (self.symbol == defn.AnyKindClass) self else self.info.hkResult
+      if self.symbol == defn.AnyKindClass then self
+      else if self.symbol.isClass then NoType // avoid forcing symbol if it's a class, not an alias to a HK type lambda
+      else self.info.hkResult
     case self: AppliedType =>
       if (self.tycon.typeSymbol.isClass) NoType else self.superType.hkResult
     case self: HKTypeLambda => self.resultType

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

@@ -199,7 +199,9 @@ object Types extends TypeUtils {
      */
     def isRef(sym: Symbol, skipRefined: Boolean = true)(using Context): Boolean = this match {
       case this1: TypeRef =>
-        this1.info match { // see comment in Namer#TypeDefCompleter#typeSig
+        // avoid forcing symbol if it's a class, not a type alias (see i15177.FakeEnum.scala)
+        if this1.symbol.isClass then this1.symbol eq sym
+        else this1.info match { // see comment in Namer#TypeDefCompleter#typeSig
           case TypeAlias(tp) => tp.isRef(sym, skipRefined)
           case _ => this1.symbol eq sym
         }

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

@@ -822,8 +822,11 @@ class Namer { typer: Typer =>
         if (sym.is(Module)) moduleValSig(sym)
         else valOrDefDefSig(original, sym, Nil, identity)(using localContext(sym).setNewScope)
       case original: DefDef =>
-        val typer1 = ctx.typer.newLikeThis(ctx.nestingLevel + 1)
-        nestedTyper(sym) = typer1
+        // For the primary constructor DefDef, it is:
+        // * indexed as a part of completing the class, with indexConstructor; and
+        // * typed ahead when completing the constructor
+        // So we need to make sure to reuse the same local/nested typer.
+        val typer1 = nestedTyper.getOrElseUpdate(sym, ctx.typer.newLikeThis(ctx.nestingLevel + 1))
         typer1.defDefSig(original, sym, this)(using localContext(sym).setTyper(typer1))
       case imp: Import =>
         try
@@ -833,6 +836,12 @@ class Namer { typer: Typer =>
           typr.println(s"error while completing ${imp.expr}")
           throw ex
 
+    /** Context setup for indexing the constructor. */
+    def indexConstructor(constr: DefDef, sym: Symbol): Unit =
+      val typer1 = ctx.typer.newLikeThis(ctx.nestingLevel + 1)
+      nestedTyper(sym) = typer1
+      typer1.indexConstructor(constr, sym)(using localContext(sym).setTyper(typer1))
+
     final override def complete(denot: SymDenotation)(using Context): Unit = {
       if (Config.showCompletions && ctx.typerState != creationContext.typerState) {
         def levels(c: Context): Int =
@@ -993,15 +1002,19 @@ class Namer { typer: Typer =>
 
     /** If completion of the owner of the to be completed symbol has not yet started,
      *  complete the owner first and check again. This prevents cyclic references
-     *  where we need to copmplete a type parameter that has an owner that is not
+     *  where we need to complete a type parameter that has an owner that is not
      *  yet completed. Test case is pos/i10967.scala.
      */
     override def needsCompletion(symd: SymDenotation)(using Context): Boolean =
       val owner = symd.owner
       !owner.exists
       || owner.is(Touched)
       || {
-        owner.ensureCompleted()
+        // Only complete the owner if it's a type (eg. the class that owns a type parameter)
+        // This avoids completing primary constructor methods while completing the type of one of its type parameters
+        // See i15177.scala.
+        if owner.isType then
+          owner.ensureCompleted()
         !symd.isCompleted
       }
 
@@ -1526,7 +1539,10 @@ class Namer { typer: Typer =>
       index(constr)
       index(rest)(using localCtx)
 
-      checkCaseClassParamDependencies(symbolOfTree(constr).info, cls) // Completes constr symbol as a side effect
+      val constrSym = symbolOfTree(constr)
+      constrSym.infoOrCompleter match
+        case completer: Completer => completer.indexConstructor(constr, constrSym)
+        case _ =>
 
       tempInfo = denot.asClass.classInfo.integrateOpaqueMembers.asInstanceOf[TempClassInfo]
       denot.info = savedInfo
@@ -1756,6 +1772,17 @@ class Namer { typer: Typer =>
       val sym = tree.symbol
       if sym.isConstructor then sym.owner else sym
 
+  /** Index the primary constructor of a class, as a part of completing that class.
+   *  This allows the rest of the constructor completion to be deferred,
+   *  which avoids non-cyclic classes failing, e.g. pos/i15177.
+   */
+  def indexConstructor(constr: DefDef, sym: Symbol)(using Context): Unit =
+    index(constr.leadingTypeParams)
+    sym.owner.typeParams.foreach(_.ensureCompleted())
+    completeTrailingParamss(constr, sym, indexingCtor = true)
+    if Feature.enabled(modularity) then
+      constr.termParamss.foreach(_.foreach(setTracked))
+
   /** The signature of a module valdef.
    *  This will compute the corresponding module class TypeRef immediately
    *  without going through the defined type of the ValDef. This is necessary
@@ -1877,13 +1904,13 @@ class Namer { typer: Typer =>
     //   3. Info of CP is computed (to be copied to DP).
     //   4. CP is completed.
     //   5. Info of CP is copied to DP and DP is completed.
-    index(ddef.leadingTypeParams)
-    if (isConstructor) sym.owner.typeParams.foreach(_.ensureCompleted())
+    if !sym.isPrimaryConstructor then
+      index(ddef.leadingTypeParams)
     val completedTypeParams =
       for tparam <- ddef.leadingTypeParams yield typedAheadExpr(tparam).symbol
     if completedTypeParams.forall(_.isType) then
       completer.setCompletedTypeParams(completedTypeParams.asInstanceOf[List[TypeSymbol]])
-    completeTrailingParamss(ddef, sym)
+    completeTrailingParamss(ddef, sym, indexingCtor = false)
     val paramSymss = normalizeIfConstructor(ddef.paramss.nestedMap(symbolOfTree), isConstructor)
     sym.setParamss(paramSymss)
 
@@ -1895,11 +1922,11 @@ class Namer { typer: Typer =>
       wrapMethType(addParamRefinements(restpe, paramSymss))
 
     if isConstructor then
-      if sym.isPrimaryConstructor && Feature.enabled(modularity) then
-        ddef.termParamss.foreach(_.foreach(setTracked))
       // set result type tree to unit, but take the current class as result type of the symbol
       typedAheadType(ddef.tpt, defn.UnitType)
-      wrapMethType(effectiveResultType(sym, paramSymss))
+      val mt = wrapMethType(effectiveResultType(sym, paramSymss))
+      if sym.isPrimaryConstructor then checkCaseClassParamDependencies(mt, sym.owner)
+      mt
     else if sym.isAllOf(Given | Method) && Feature.enabled(modularity) then
       // set every context bound evidence parameter of a given companion method
       // to be tracked, provided it has a type that has an abstract type member.
@@ -1912,30 +1939,37 @@ class Namer { typer: Typer =>
       valOrDefDefSig(ddef, sym, paramSymss, wrapMethType)
   end defDefSig
 
-  def completeTrailingParamss(ddef: DefDef, sym: Symbol)(using Context): Unit =
+  /** Complete the trailing parameters of a DefDef,
+   *  as a part of indexing the primary constructor or
+   *  as a part of completing a DefDef, including the primary constructor.
+   */
+  def completeTrailingParamss(ddef: DefDef, sym: Symbol, indexingCtor: Boolean)(using Context): Unit =
     // A map from context-bounded type parameters to associated evidence parameter names
     val witnessNamesOfParam = mutable.Map[TypeDef, List[TermName]]()
-    if !ddef.name.is(DefaultGetterName) && !sym.is(Synthetic) then
+    if !ddef.name.is(DefaultGetterName) && !sym.is(Synthetic) && (indexingCtor || !sym.isPrimaryConstructor) then
       for params <- ddef.paramss; case tdef: TypeDef <- params do
         for case WitnessNamesAnnot(ws) <- tdef.mods.annotations do
           witnessNamesOfParam(tdef) = ws
 
-    /** Is each name in `wnames` defined somewhere in the longest prefix of all `params`
-     *  that have been typed ahead (i.e. that carry the TypedAhead attachment)?
-     */
-    def allParamsSeen(wnames: List[TermName], params: List[MemberDef]) =
-      (wnames.toSet[Name] -- params.takeWhile(_.hasAttachment(TypedAhead)).map(_.name)).isEmpty
+    /** Is each name in `wnames` defined somewhere in the previous parameters? */
+    def allParamsSeen(wnames: List[TermName], prevParams: Set[Name]) =
+      (wnames.toSet[Name] -- prevParams).isEmpty
 
     /** Enter and typecheck parameter list.
      *  Once all witness parameters for a context bound are seen, create a
      *  context bound companion for it.
      */
     def completeParams(params: List[MemberDef])(using Context): Unit =
-      index(params)
+      if indexingCtor || !sym.isPrimaryConstructor then
+        index(params)
+      var prevParams = Set.empty[Name]
       for param <- params do
-        typedAheadExpr(param)
+        if !indexingCtor then
+          typedAheadExpr(param)
+
+        prevParams += param.name
         for (tdef, wnames) <- witnessNamesOfParam do
-          if wnames.contains(param.name) && allParamsSeen(wnames, params) then
+          if wnames.contains(param.name) && allParamsSeen(wnames, prevParams) then
             addContextBoundCompanionFor(symbolOfTree(tdef), wnames, params.map(symbolOfTree))
 
     ddef.trailingParamss.foreach(completeParams)
@@ -1966,7 +2000,7 @@ class Namer { typer: Typer =>
   def setTracked(param: ValDef)(using Context): Unit =
     val sym = symbolOfTree(param)
     sym.maybeOwner.maybeOwner.infoOrCompleter match
-      case info: TempClassInfo if needsTracked(sym, param) =>
+      case info: ClassInfo if needsTracked(sym, param) =>
         typr.println(i"set tracked $param, $sym: ${sym.info} containing ${sym.info.memberNames(abstractTypeNameFilter).toList}")
         for acc <- info.decls.lookupAll(sym.name) if acc.is(ParamAccessor) do
           acc.resetFlag(PrivateLocal)

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

@@ -2516,12 +2516,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
               (arg, tparamBounds)
             else
               (arg, WildcardType)
-          if (tpt1.symbol.isClass)
-            tparam match {
-              case tparam: Symbol =>
-                tparam.ensureCompleted() // This is needed to get the test `compileParSetSubset` to work
-              case _ =>
-            }
           if (desugaredArg.isType)
             arg match {
               case untpd.WildcardTypeBoundsTree()

tests/neg/i15177.FakeEnum.min.alt1.scala

@@ -0,0 +1,7 @@
+// Like tests/neg/i15177.FakeEnum.min.scala
+// But with an actual upper-bound requirement
+// Which shouldn't be ignored as a part of overcoming the the cycle
+trait Foo
+trait X[T <: Foo] { trait Id }
+object A extends X[B] // error: Type argument B does not conform to upper bound Foo
+class B extends A.Id

tests/neg/i15177.constr-dep.scala

@@ -0,0 +1,9 @@
+// An example of how constructor _type_ parameters
+// Which can _not_ be passed to the extends part
+// That makes it part of the parent type,
+// which has been found to be unsound.
+class Foo[A]
+class Foo1(val x: Int)
+  extends Foo[ // error: The type of a class parent cannot refer to constructor parameters, but Foo[(Foo1.this.x : Int)] refers to x
+    x.type
+  ]

tests/neg/i15177.ub.scala

@@ -0,0 +1,13 @@
+// like tests/pos/i15177.scala
+// but with T having an upper bound
+// that B doesn't conform to
+// just to be sure that not forcing B
+// doesn't backdoor an illegal X[B]
+class X[T <: C] {
+  type Id
+}
+object A
+  extends X[ // error
+    B] // error
+class B(id: A.Id)
+class C

tests/pos/i15177.FakeEnum.min.alt2.scala

@@ -0,0 +1,7 @@
+// Like tests/neg/i15177.FakeEnum.min.scala
+// With an actual upper-bound requirement
+// But that is satisfied on class B
+trait Foo
+trait X[T <: Foo] { trait Id }
+object A extends X[B]
+class B extends A.Id with Foo

tests/pos/i15177.FakeEnum.min.alt3.scala

@@ -0,0 +1,7 @@
+// Like tests/neg/i15177.FakeEnum.min.scala
+// With an actual upper-bound requirement
+// But that is satisfied on trait Id
+trait Foo
+trait X[T <: Foo] { trait Id extends Foo }
+object A extends X[B]
+class B extends A.Id

tests/pos/i15177.FakeEnum.min.scala

@@ -0,0 +1,4 @@
+// Minimisation of tests/neg/i15177.FakeEnum.scala
+trait X[T] { trait Id }
+object A extends X[B]
+class B extends A.Id

tests/pos/i15177.FakeEnum.scala

@@ -0,0 +1,21 @@
+// From https://github.com/scala/scala3/issues/15177#issuecomment-1463088400
+trait FakeEnum[A, @specialized(Byte, Short, Int, Long) B]
+{
+  trait Value {
+    self: A =>
+      def name: String
+      def id: B
+  }
+}
+
+object FakeEnumType
+  extends FakeEnum[FakeEnumType, Short]
+{
+  val MEMBER1 = new FakeEnumType((0: Short), "MEMBER1") {}
+  val MEMBER2 = new FakeEnumType((1: Short), "MEMBER2") {}
+}
+
+sealed abstract
+class FakeEnumType(val id: Short, val name: String)
+  extends FakeEnumType.Value
+{}

tests/pos/i15177.app.scala

@@ -0,0 +1,6 @@
+// like tests/pos/i15177.scala
+// but with an applied type B[D]
+class X[T] { type Id }
+object A extends X[B[D]]
+class B[C](id: A.Id)
+class D

tests/pos/i15177.constr-dep.scala

@@ -0,0 +1,6 @@
+// An example of how constructor _term_ parameters
+// Can be passed to the extends part
+// But that doesn't mean the parent type,
+// it's just the super constructor call.
+class Bar(val y: Long)
+class Bar1(val z: Long) extends Bar(z)

tests/pos/i15177.hk.scala

@@ -0,0 +1,5 @@
+// like tests/pos/i15177.scala
+// but with B being higher kinded
+class X[T[_]] { type Id }
+object A extends X[B]
+class B[C](id: A.Id)

tests/pos/i15177.hk2.scala

@@ -0,0 +1,6 @@
+// like tests/pos/i15177.scala
+// but with B being higher kinded
+// but without the actual cycle (like .without)
+class X[T[_]] { type Id }
+class A extends X[B]
+class B[C]

tests/pos/i15177.hylolib.scala

@@ -0,0 +1,11 @@
+//> using options -language:experimental.modularity -source future
+// A minimisation of pos/hylolib-cb that broke while fixing i15177
+trait Value[Self]
+trait Coll[Self]:
+  type Pos: Value
+  extension (self: Self) def pos: Pos
+extension [Self: Coll](self: Self) def trigger = self.pos
+class Slice[Base]
+given SliceIsColl[T: Coll as c]: Coll[Slice[T]] with
+  type Pos = c.Pos
+  extension (self: Slice[T]) def pos: Pos = ???

tests/pos/i15177.orig.scala

@@ -0,0 +1,7 @@
+trait DomainIdProvider[T] {
+  type Id = List[T]
+}
+object Country extends DomainIdProvider[Country]
+case class Country(
+    id: Country.Id,
+)

tests/pos/i15177.scala

@@ -0,0 +1,3 @@
+class X[T] { trait Id }
+object A extends X[B]
+class B(id: A.Id)

tests/pos/i15177.without.scala

@@ -0,0 +1,5 @@
+// like tests/pos/i15177.scala
+// but without the actual cycle
+class X[T] { trait Id }
+class A extends X[B]
+class B