Avoid forcing ctors & parents which caused cycles (#17086)

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,12 +1539,9 @@ class Namer { typer: Typer =>
       index(constr)
       index(rest)(using localCtx)
 
-      symbolOfTree(constr).info.stripPoly match // Completes constr symbol as a side effect
-        case mt: MethodType if cls.is(Case) && mt.isParamDependent =>
-          // See issue #8073 for background
-          report.error(
-              em"""Implementation restriction: case classes cannot have dependencies between parameters""",
-              cls.srcPos)
+      val constrSym = symbolOfTree(constr)
+      constrSym.infoOrCompleter match
+        case completer: Completer => completer.indexConstructor(constr, constrSym)
         case _ =>
 
       tempInfo = denot.asClass.classInfo.integrateOpaqueMembers.asInstanceOf[TempClassInfo]
@@ -1762,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
@@ -1860,31 +1881,6 @@ class Namer { typer: Typer =>
     // Beware: ddef.name need not match sym.name if sym was freshened!
     val isConstructor = sym.name == nme.CONSTRUCTOR
 
-    // 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
-      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
-
-    /** 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)
-      for param <- params do
-        typedAheadExpr(param)
-        for (tdef, wnames) <- witnessNamesOfParam do
-          if wnames.contains(param.name) && allParamsSeen(wnames, params) then
-            addContextBoundCompanionFor(symbolOfTree(tdef), wnames, params.map(symbolOfTree))
-
     // The following 3 lines replace what was previously just completeParams(tparams).
     // But that can cause bad bounds being computed, as witnessed by
     // tests/pos/paramcycle.scala. The problematic sequence is this:
@@ -1908,39 +1904,16 @@ 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]])
-    ddef.trailingParamss.foreach(completeParams)
+    completeTrailingParamss(ddef, sym, indexingCtor = false)
     val paramSymss = normalizeIfConstructor(ddef.paramss.nestedMap(symbolOfTree), isConstructor)
     sym.setParamss(paramSymss)
 
-    /** Under x.modularity, we add `tracked` to context bound witnesses
-     *  that have abstract type members
-     */
-    def needsTracked(sym: Symbol, param: ValDef)(using Context) =
-      !sym.is(Tracked)
-      && param.hasAttachment(ContextBoundParam)
-      && sym.info.memberNames(abstractTypeNameFilter).nonEmpty
-
-    /** Under x.modularity, set every context bound evidence parameter of a class to be tracked,
-     *  provided it has a type that has an abstract type member. Reset private and local flags
-     *  so that the parameter becomes a `val`.
-     */
-    def setTracked(param: ValDef): Unit =
-      val sym = symbolOfTree(param)
-      sym.maybeOwner.maybeOwner.infoOrCompleter match
-        case info: TempClassInfo 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)
-            acc.setFlag(Tracked)
-            sym.setFlag(Tracked)
-        case _ =>
-
     def wrapMethType(restpe: Type): Type =
       instantiateDependent(restpe, paramSymss)
       methodType(paramSymss, restpe, ddef.mods.is(JavaDefined))
@@ -1949,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.
@@ -1966,6 +1939,75 @@ class Namer { typer: Typer =>
       valOrDefDefSig(ddef, sym, paramSymss, wrapMethType)
   end defDefSig
 
+  /** 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) && (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 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 =
+      if indexingCtor || !sym.isPrimaryConstructor then
+        index(params)
+      var prevParams = Set.empty[Name]
+      for param <- params do
+        if !indexingCtor then
+          typedAheadExpr(param)
+
+        prevParams += param.name
+        for (tdef, wnames) <- witnessNamesOfParam do
+          if wnames.contains(param.name) && allParamsSeen(wnames, prevParams) then
+            addContextBoundCompanionFor(symbolOfTree(tdef), wnames, params.map(symbolOfTree))
+
+    ddef.trailingParamss.foreach(completeParams)
+  end completeTrailingParamss
+
+  /** Checks an implementation restriction on case classes. */
+  def checkCaseClassParamDependencies(mt: Type, cls: Symbol)(using Context): Unit =
+    mt.stripPoly match
+      case mt: MethodType if cls.is(Case) && mt.isParamDependent =>
+        // See issue #8073 for background
+        report.error(
+            em"""Implementation restriction: case classes cannot have dependencies between parameters""",
+            cls.srcPos)
+      case _ =>
+
+  /** Under x.modularity, we add `tracked` to context bound witnesses
+   *  that have abstract type members
+   */
+  def needsTracked(sym: Symbol, param: ValDef)(using Context) =
+    !sym.is(Tracked)
+    && param.hasAttachment(ContextBoundParam)
+    && sym.info.memberNames(abstractTypeNameFilter).nonEmpty
+
+  /** Under x.modularity, set every context bound evidence parameter of a class to be tracked,
+   *  provided it has a type that has an abstract type member. Reset private and local flags
+   *  so that the parameter becomes a `val`.
+   */
+  def setTracked(param: ValDef)(using Context): Unit =
+    val sym = symbolOfTree(param)
+    sym.maybeOwner.maybeOwner.infoOrCompleter match
+      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)
+          acc.setFlag(Tracked)
+          sym.setFlag(Tracked)
+      case _ =>
+
   def inferredResultType(
       mdef: ValOrDefDef,
       sym: Symbol,

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