Don't prematurely force info of fields with inferred types

compiler/src/dotty/tools/dotc/cc/Capability.scala

@@ -378,10 +378,15 @@ object Capabilities:
       case tp: SetCapability => tp.captureSetOfInfo.isReadOnly
       case _ => this ne stripReadOnly
 
-    final def restriction(using Context): Symbol = this match
+    /** The classifier, either given in an explicit `.only` or assumed for a
+     *  FreshCap. AnyRef for unclassified FreshCaps. Otherwise NoSymbol if no
+     *  classifier is given.
+     */
+    final def classifier(using Context): Symbol = this match
       case Restricted(_, cls) => cls
-      case ReadOnly(ref1) => ref1.restriction
-      case Maybe(ref1) => ref1.restriction
+      case ReadOnly(ref1) => ref1.classifier
+      case Maybe(ref1) => ref1.classifier
+      case self: FreshCap => self.hiddenSet.classifier
       case _ => NoSymbol
 
     /** Is this a reach reference of the form `x*` or a readOnly or maybe variant
@@ -617,7 +622,7 @@ object Capabilities:
           case Reach(_) =>
             captureSetOfInfo.transClassifiers
           case self: CoreCapability =>
-            if self.derivesFromCapability then toClassifiers(self.classifier)
+            if self.derivesFromCapability then toClassifiers(self.inheritedClassifier)
             else captureSetOfInfo.transClassifiers
         if myClassifiers != UnknownClassifier then
           classifiersValid == currentId

compiler/src/dotty/tools/dotc/cc/CaptureOps.scala

@@ -487,7 +487,7 @@ extension (tp: Type)
     case _ =>
       tp
 
-  def classifier(using Context): ClassSymbol =
+  def inheritedClassifier(using Context): ClassSymbol =
     tp.classSymbols.map(_.classifier).foldLeft(defn.AnyClass)(leastClassifier)
 
 extension (tp: MethodType)

compiler/src/dotty/tools/dotc/cc/CapturingType.scala

@@ -40,7 +40,7 @@ object CapturingType:
         apply(parent1, refs ++ refs1, boxed)
       case _ =>
         if parent.derivesFromMutable then refs.associateWithMutable()
-        refs.adoptClassifier(parent.classifier)
+        refs.adoptClassifier(parent.inheritedClassifier)
         AnnotatedType(parent, CaptureAnnotation(refs, boxed)(defn.RetainsAnnot))
 
   /** An extractor for CapturingTypes. Capturing types are recognized if

compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala

@@ -947,6 +947,17 @@ class CheckCaptures extends Recheck, SymTransformer:
         .showing(i"constr type $mt with $argTypes%, % in $constr = $result", capt)
     end refineConstructorInstance
 
+    /** If `mbr` is a field that has (possibly restricted) FreshCaps in its span capture set,
+     *  their classifiers, otherwise the empty list.
+     */
+    private def classifiersOfFreshInType(mbr: Symbol)(using Context): List[ClassSymbol] =
+      if contributesFreshToClass(mbr) then
+        mbr.info.spanCaptureSet.elems
+          .filter(_.isTerminalCapability)
+          .toList
+          .map(_.classifier.asClass)
+      else Nil
+
     /** The additional capture set implied by the capture sets of its fields. This
      *  is either empty or, if some fields have a terminal capability in their span
      *  capture sets, it consists of a single fresh cap that subsumes all these terminal
@@ -964,16 +975,9 @@ class CheckCaptures extends Recheck, SymTransformer:
        */
       def impliedClassifiers(cls: Symbol): List[ClassSymbol] = cls match
         case cls: ClassSymbol =>
-          var fieldClassifiers =
-            for
-              sym <- cls.info.decls.toList
-              if contributesFreshToClass(sym)
-              case fresh: FreshCap <- sym.info.spanCaptureSet.elems
-                .filter(_.isTerminalCapability)
-                .map(_.stripReadOnly)
-                .toList
-              _ = pushInfo(i"Note: ${sym.showLocated} captures a $fresh")
-            yield fresh.hiddenSet.classifier
+          var fieldClassifiers = setup.fieldsWithExplicitTypes // pick fields with explicit types for classes in this compilation unit
+              .getOrElse(cls, cls.info.decls.toList)           // pick all symbols in class scope for other classes
+              .flatMap(classifiersOfFreshInType)
           if cls.typeRef.isMutableType then
             fieldClassifiers = defn.Caps_Mutable :: fieldClassifiers
           val parentClassifiers =
@@ -1227,11 +1231,20 @@ class CheckCaptures extends Recheck, SymTransformer:
           curEnv = saved
     end recheckDefDef
 
-    /** If val or def definition with inferred (result) type is visible
-     *  in other compilation units, check that the actual inferred type
-     *  conforms to the expected type where all inferred capture sets are dropped.
-     *  This ensures that if files compile separately, they will also compile
-     *  in a joint compilation.
+    /** Two tests for member definitions with inferred types:
+     *
+     *   1. If val or def definition with inferred (result) type is visible
+     *      in other compilation units, check that the actual inferred type
+     *      conforms to the expected type where all inferred capture sets are dropped.
+     *      This ensures that if files compile separately, they will also compile
+     *      in a joint compilation.
+     *   2. If a val has an inferred type with a terminal capability in its span capset,
+     *      check that it this capability is subsumed by the capset that was inferred
+     *      for the class from its other fields via `captureSetImpliedByFields`.
+     *      That capset is defined to take into account all fields but is computed
+     *      only from fields with explicitly given types in order to avoid cycles.
+     *      See comment on Setup.fieldsWithExplicitTypes. So we have to make sure
+     *      that fields with inferred types would not change that capset.
      */
     def checkInferredResult(tp: Type, tree: ValOrDefDef)(using Context): Type =
       val sym = tree.symbol
@@ -1266,11 +1279,17 @@ class CheckCaptures extends Recheck, SymTransformer:
           |The new inferred type $tp
           |must conform to this type."""
 
+      def covers(classCapset: CaptureSet, fieldClassifiers: List[ClassSymbol]): Boolean =
+        fieldClassifiers.forall: cls =>
+          classCapset.elems.exists:
+            case fresh: FreshCap => cls.isSubClass(fresh.hiddenSet.classifier)
+            case _ => false
+
       tree.tpt match
-        case tpt: InferredTypeTree if !isExemptFromChecks =>
-          if !sym.isLocalToCompilationUnit
-             // Symbols that can't be seen outside the compilation unit can have inferred types
-             // except for the else clause below.
+        case tpt: InferredTypeTree =>
+          // Test point (1) of doc comment above
+          if !sym.isLocalToCompilationUnit && !isExemptFromChecks
+            // Symbols that can't be seen outside the compilation unit can have inferred types
           then
             val expected = tpt.tpe.dropAllRetains
             todoAtPostCheck += { () =>
@@ -1279,18 +1298,21 @@ class CheckCaptures extends Recheck, SymTransformer:
                 // The check that inferred <: expected is done after recheck so that it
                 // does not interfere with normal rechecking by constraining capture set variables.
             }
-          else if sym.is(Private)
-              && !sym.isLocalToCompilationUnitIgnoringPrivate
-              && tree.tpt.nuType.spanCaptureSet.containsTerminalCapability
+          // Test point (2) of doc comment above
+          if sym.owner.isClass && !sym.owner.isStaticOwner
               && contributesFreshToClass(sym)
-              // Private symbols capturing a root capability need explicit types
-              // so that we can compute field constributions to class instance
-              // capture sets across compilation units.
           then
-            report.error(
-              em"""$sym needs an explicit type because it captures a root capability in its type ${tree.tpt.nuType}.
-                  |Fields of publicily accessible classes that capture a root capability need to be given an explicit type.""",
-            tpt.srcPos)
+            todoAtPostCheck += { () =>
+              val cls = sym.owner.asClass
+              val fieldClassifiers = classifiersOfFreshInType(sym)
+              val classCapset = captureSetImpliedByFields(cls, cls.appliedRef)
+              if !covers(classCapset, fieldClassifiers) then
+                report.error(
+                  em"""$sym needs an explicit type because it captures a root capability in its type ${tree.tpt.nuType}.
+                      |Fields capturing a root capability need to be given an explicit type unless the capability is already
+                      |subsumed by the computed capability of the enclosing class.""",
+                tpt.srcPos)
+            }
         case _ =>
       tp
     end checkInferredResult

compiler/src/dotty/tools/dotc/cc/Setup.scala

@@ -40,6 +40,14 @@ trait SetupAPI:
   /** Check to do after the capture checking traversal */
   def postCheck()(using Context): Unit
 
+  /** A map from currently compiled class symbols to those of their fields
+   *  that have an explicit type given. Used in `captureSetImpliedByFields`
+   *  to avoid forcing fields with inferred types prematurely. The test file
+   *  where this matters is i24335.scala. The precise failure scenario which
+   *  this avoids is described in #24335.
+   */
+  def fieldsWithExplicitTypes: collection.Map[ClassSymbol, List[Symbol]]
+
   /** Used for error reporting:
    *  Maps mutable variables to the symbols that capture them (in the
    *  CheckCaptures sense, i.e. symbol is referred to from a different method
@@ -52,6 +60,7 @@ trait SetupAPI:
    *  the function that is called.
    */
   def anonFunCallee: collection.Map[Symbol, Symbol]
+
 end SetupAPI
 
 object Setup:
@@ -489,6 +498,12 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI:
   extension (sym: Symbol) def nextInfo(using Context): Type =
     atPhase(thisPhase.next)(sym.info)
 
+  val fieldsWithExplicitTypes: mutable.HashMap[ClassSymbol, List[Symbol]] = mutable.HashMap()
+
+  val capturedBy: mutable.HashMap[Symbol, Symbol] = mutable.HashMap()
+
+  val anonFunCallee: mutable.HashMap[Symbol, Symbol] = mutable.HashMap()
+
   /** A traverser that adds knownTypes and updates symbol infos */
   def setupTraverser(checker: CheckerAPI) = new TreeTraverserWithPreciseImportContexts:
     import checker.*
@@ -693,59 +708,65 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI:
       case tree: Bind =>
         val sym = tree.symbol
         updateInfo(sym, transformInferredType(sym.info), sym.owner)
-      case tree: TypeDef =>
-        tree.symbol match
-          case cls: ClassSymbol =>
-            checkClassifiedInheritance(cls)
-            val cinfo @ ClassInfo(prefix, _, ps, decls, selfInfo) = cls.classInfo
-
-            // Compute new self type
-            def isInnerModule = cls.is(ModuleClass) && !cls.isStatic
-            val selfInfo1 =
-              if (selfInfo ne NoType) && !isInnerModule then
-                // if selfInfo is explicitly given then use that one, except if
-                // self info applies to non-static modules, these still need to be inferred
-                selfInfo
-              else if cls.isPureClass then
-                // is cls is known to be pure, nothing needs to be added to self type
-                selfInfo
-              else if !cls.isEffectivelySealed && !cls.baseClassHasExplicitNonUniversalSelfType then
-                // assume {cap} for completely unconstrained self types of publicly extensible classes
-                CapturingType(cinfo.selfType, CaptureSet.universal)
-              else
-                // Infer the self type for the rest, which is all classes without explicit
-                // self types (to which we also add nested module classes), provided they are
-                // neither pure, nor are publicily extensible with an unconstrained self type.
-                val cs = CaptureSet.ProperVar(cls, CaptureSet.emptyRefs, nestedOK = false, isRefining = false)
-                if cls.derivesFrom(defn.Caps_Capability) then
-                  // If cls is a capability class, we need to add a fresh capability to ensure
-                  // we cannot treat the class as pure.
-                  CaptureSet.fresh(cls, cls.thisType, Origin.InDecl(cls)).subCaptures(cs)
-                CapturingType(cinfo.selfType, cs)
-
-            // Compute new parent types
-            val ps1 = inContext(ctx.withOwner(cls)):
-              ps.mapConserve(transformExplicitType(_, NoSymbol, freshen = false))
-
-            // Install new types and if it is a module class also update module object
-            if (selfInfo1 ne selfInfo) || (ps1 ne ps) then
-              val newInfo = ClassInfo(prefix, cls, ps1, decls, selfInfo1)
-              updateInfo(cls, newInfo, cls.owner)
-              capt.println(i"update class info of $cls with parents $ps selfinfo $selfInfo to $newInfo")
-              cls.thisType.asInstanceOf[ThisType].invalidateCaches()
-              if cls.is(ModuleClass) then
-                // if it's a module, the capture set of the module reference is the capture set of the self type
-                val modul = cls.sourceModule
-                val selfCaptures = selfInfo1 match
-                  case CapturingType(_, refs) => refs
-                  case _ => CaptureSet.empty
-                // Note: Can't do val selfCaptures = selfInfo1.captureSet here.
-                // This would potentially give stackoverflows when setup is run repeatedly.
-                // One test case is pos-custom-args/captures/checkbounds.scala under
-                // ccConfig.alwaysRepeatRun = true.
-                updateInfo(modul, CapturingType(modul.info, selfCaptures), modul.owner)
-                modul.termRef.invalidateCaches()
-          case _ =>
+      case tree @ TypeDef(_, impl: Template) =>
+        val cls: ClassSymbol = tree.symbol.asClass
+
+        fieldsWithExplicitTypes(cls) =
+          for
+            case vd @ ValDef(_, tpt: TypeTree, _) <- impl.body
+            if !tpt.isInferred && vd.symbol.exists && !vd.symbol.is(NonMember)
+          yield
+            vd.symbol
+
+        checkClassifiedInheritance(cls)
+        val cinfo @ ClassInfo(prefix, _, ps, decls, selfInfo) = cls.classInfo
+
+        // Compute new self type
+        def isInnerModule = cls.is(ModuleClass) && !cls.isStatic
+        val selfInfo1 =
+          if (selfInfo ne NoType) && !isInnerModule then
+            // if selfInfo is explicitly given then use that one, except if
+            // self info applies to non-static modules, these still need to be inferred
+            selfInfo
+          else if cls.isPureClass then
+            // is cls is known to be pure, nothing needs to be added to self type
+            selfInfo
+          else if !cls.isEffectivelySealed && !cls.baseClassHasExplicitNonUniversalSelfType then
+            // assume {cap} for completely unconstrained self types of publicly extensible classes
+            CapturingType(cinfo.selfType, CaptureSet.universal)
+          else
+            // Infer the self type for the rest, which is all classes without explicit
+            // self types (to which we also add nested module classes), provided they are
+            // neither pure, nor are publicily extensible with an unconstrained self type.
+            val cs = CaptureSet.ProperVar(cls, CaptureSet.emptyRefs, nestedOK = false, isRefining = false)
+            if cls.derivesFrom(defn.Caps_Capability) then
+              // If cls is a capability class, we need to add a fresh capability to ensure
+              // we cannot treat the class as pure.
+              CaptureSet.fresh(cls, cls.thisType, Origin.InDecl(cls)).subCaptures(cs)
+            CapturingType(cinfo.selfType, cs)
+
+        // Compute new parent types
+        val ps1 = inContext(ctx.withOwner(cls)):
+          ps.mapConserve(transformExplicitType(_, NoSymbol, freshen = false))
+
+        // Install new types and if it is a module class also update module object
+        if (selfInfo1 ne selfInfo) || (ps1 ne ps) then
+          val newInfo = ClassInfo(prefix, cls, ps1, decls, selfInfo1)
+          updateInfo(cls, newInfo, cls.owner)
+          capt.println(i"update class info of $cls with parents $ps selfinfo $selfInfo to $newInfo")
+          cls.thisType.asInstanceOf[ThisType].invalidateCaches()
+          if cls.is(ModuleClass) then
+            // if it's a module, the capture set of the module reference is the capture set of the self type
+            val modul = cls.sourceModule
+            val selfCaptures = selfInfo1 match
+              case CapturingType(_, refs) => refs
+              case _ => CaptureSet.empty
+            // Note: Can't do val selfCaptures = selfInfo1.captureSet here.
+            // This would potentially give stackoverflows when setup is run repeatedly.
+            // One test case is pos-custom-args/captures/checkbounds.scala under
+            // ccConfig.alwaysRepeatRun = true.
+            updateInfo(modul, CapturingType(modul.info, selfCaptures), modul.owner)
+            modul.termRef.invalidateCaches()
       case _ =>
     end postProcess
 
@@ -918,16 +939,11 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI:
         else t
       case _ => mapFollowingAliases(t)
 
-  val capturedBy: mutable.HashMap[Symbol, Symbol] = mutable.HashMap[Symbol, Symbol]()
-
-  val anonFunCallee: mutable.HashMap[Symbol, Symbol] = mutable.HashMap[Symbol, Symbol]()
-
   /** Run setup on a compilation unit with given `tree`.
    *  @param recheckDef   the function to run for completing a val or def
    */
   def setupUnit(tree: Tree, checker: CheckerAPI)(using Context): Unit =
-    inContext(ctx.withPhase(thisPhase)):
-      setupTraverser(checker).traverse(tree)
+    setupTraverser(checker).traverse(tree)(using ctx.withPhase(thisPhase))
 
   // ------ Checks to run at Setup ----------------------------------------