Fixes for mutability (#24205)

Whether something is mutable or not depends on the context:

- this of a Mutable type is exclusive in update methods, read-only
elsewhere
- a selection from a mutable type with read-only capture set is again
read-only

Author: odersky

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

@@ -226,7 +226,7 @@ object desugar {
           paramss = (setterParam :: Nil) :: Nil,
           tpt     = TypeTree(defn.UnitType),
           rhs     = setterRhs
-        ).withMods((vdef.mods | Accessor) &~ (CaseAccessor | GivenOrImplicit | Lazy))
+        ).withMods((vdef.mods | Accessor) &~ (CaseAccessor | GivenOrImplicit | Lazy | Transparent))
         .dropEndMarker() // the end marker should only appear on the getter definition
       Thicket(vdef1, setter)
     else vdef1

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

@@ -352,11 +352,14 @@ extension (tp: Type)
     case _ =>
       false
 
-  /** Is this a type extending `Mutable` that has update methods? */
+  /** Is this a type extending `Mutable` that has non-private update methods
+   *  or mutable fields?
+   */
   def isMutableType(using Context): Boolean =
     tp.derivesFrom(defn.Caps_Mutable)
-    && tp.membersBasedOnFlags(Mutable | Method, EmptyFlags)
-      .exists(_.hasAltWith(_.symbol.isUpdateMethod))
+    && tp.membersBasedOnFlags(Mutable, EmptyFlags).exists: mbr =>
+      if mbr.symbol.is(Method) then mbr.symbol.isUpdateMethod
+      else !mbr.symbol.is(Transparent)
 
   /** Is this a reference to caps.cap? Note this is _not_ the GlobalCap capability. */
   def isCapRef(using Context): Boolean = tp match
@@ -496,6 +499,23 @@ extension (tp: Type)
   def classifier(using Context): ClassSymbol =
     tp.classSymbols.map(_.classifier).foldLeft(defn.AnyClass)(leastClassifier)
 
+  def exclusivity(using Context): Exclusivity =
+    if tp.derivesFrom(defn.Caps_Mutable) then
+      tp match
+        case tp: Capability if tp.isExclusive => Exclusivity.OK
+        case _ => tp.captureSet.exclusivity(tp)
+    else Exclusivity.OK
+
+  def exclusivityInContext(using Context): Exclusivity = tp match
+    case tp: ThisType =>
+      if tp.derivesFrom(defn.Caps_Mutable)
+      then ctx.owner.inExclusivePartOf(tp.cls)
+      else Exclusivity.OK
+    case tp @ TermRef(prefix: Capability, _) =>
+      prefix.exclusivityInContext.andAlso(tp.exclusivity)
+    case _ =>
+      tp.exclusivity
+
 extension (tp: MethodType)
   /** A method marks an existential scope unless it is the prefix of a curried method */
   def marksExistentialScope(using Context): Boolean =
@@ -629,8 +649,22 @@ extension (sym: Symbol)
     sym.hasAnnotation(defn.ConsumeAnnot)
     || sym.info.hasAnnotation(defn.ConsumeAnnot)
 
+  /** An update method is either a method marked with `update` or
+   *  a setter of a non-transparent var.
+   */
   def isUpdateMethod(using Context): Boolean =
-    sym.isAllOf(Mutable | Method, butNot = Accessor)
+    sym.isAllOf(Mutable | Method)
+      && (!sym.isSetter || sym.field.is(Transparent))
+
+  def inExclusivePartOf(cls: Symbol)(using Context): Exclusivity =
+    import Exclusivity.*
+    val encl = sym.enclosingMethodOrClass.skipConstructor
+    if sym == cls then OK // we are directly in `cls` or in one of its constructors
+    else if encl.owner == cls then
+      if encl.isUpdateMethod then OK
+      else NotInUpdateMethod(encl, cls)
+    else if encl.isStatic then OutsideClass(cls)
+    else encl.owner.inExclusivePartOf(cls)
 
   def isReadOnlyMethod(using Context): Boolean =
     sym.is(Method, butNot = Mutable | Accessor) && sym.owner.derivesFrom(defn.Caps_Mutable)
@@ -769,3 +803,35 @@ abstract class DeepTypeAccumulator[T](using Context) extends TypeAccumulator[T]:
         foldOver(acc, t)
 end DeepTypeAccumulator
 
+/** Either OK, or a reason why capture set cannot be exclusive */
+enum Exclusivity:
+  case OK
+
+  /** Enclosing symbol `sym` is a method of class `cls`, but not an update method */
+  case NotInUpdateMethod(sym: Symbol, cls: Symbol)
+
+  /** Access to `this` from outside its class (not sure this can happen) */
+  case OutsideClass(cls: Symbol)
+
+  /** A prefix type `tp` has a read-only capture set */
+  case ReadOnly(tp: Type)
+
+  def isOK: Boolean = this == OK
+
+  def andAlso(that: Exclusivity): Exclusivity =
+    if this == OK then that else this
+
+  /** A textual description why `qualType` is not exclusive */
+  def description(qualType: Type)(using Context): String = this.runtimeChecked match
+    case Exclusivity.ReadOnly(tp) =>
+      if qualType eq tp then
+        i"its capture set ${qualType.captureSet} is read-only"
+      else
+        i"the capture set ${tp.captureSet} of its prefix $tp is read-only"
+    case Exclusivity.NotInUpdateMethod(sym: Symbol, cls: Symbol) =>
+      i"the access is in $sym, which is not an update method"
+    case Exclusivity.OutsideClass(cls: Symbol) =>
+      i"the access from is from ouside $cls"
+
+end Exclusivity
+

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

@@ -60,8 +60,15 @@ sealed abstract class CaptureSet extends Showable:
   def mutability_=(x: Mutability): Unit =
     myMut = x
 
-  /** Mark this capture set as belonging to a Mutable type. */
-  def setMutable()(using Context): Unit
+  /** Mark this capture set as belonging to a Mutable type. Called when a new
+   *  CapturingType is formed. This is different from the setter `mutability_=`
+   *  in that it be defined with different behaviors:
+   *
+   *   - set mutability to Mutable (for normal Vars)
+   *   - take mutability from the set's sources (for DerivedVars)
+   *   - compute mutability on demand based on mutability of elements (for Consts)
+   */
+  def associateWithMutable()(using Context): Unit
 
   /** Is this capture set constant (i.e. not an unsolved capture variable)?
    *  Solved capture variables count as constant.
@@ -145,6 +152,9 @@ sealed abstract class CaptureSet extends Showable:
   final def isExclusive(using Context): Boolean =
     elems.exists(_.isExclusive)
 
+  def exclusivity(tp: Type)(using Context): Exclusivity =
+    if isExclusive then Exclusivity.OK else Exclusivity.ReadOnly(tp)
+
   /** Similar to isExlusive, but also includes capture set variables
    *  with unknown status.
    */
@@ -610,7 +620,7 @@ object CaptureSet:
 
     private var isComplete = true
 
-    def setMutable()(using Context): Unit =
+    def associateWithMutable()(using Context): Unit =
       if !elems.isEmpty then
         isComplete = false // delay computation of Mutability status
 
@@ -630,9 +640,9 @@ object CaptureSet:
       else ""
 
   private def capImpliedByCapability(parent: Type)(using Context): Capability =
-    if parent.derivesFromExclusive then GlobalCap.readOnly else GlobalCap
+    if parent.derivesFromMutable then GlobalCap.readOnly else GlobalCap
 
-  /* The same as {cap.rd} but generated implicitly for references of Capability subtypes.
+  /* The same as {cap} but generated implicitly for references of Capability subtypes.
    */
   class CSImpliedByCapability(parent: Type)(using @constructorOnly ctx: Context)
   extends Const(SimpleIdentitySet(capImpliedByCapability(parent)))
@@ -705,7 +715,7 @@ object CaptureSet:
      */
     var deps: Deps = SimpleIdentitySet.empty
 
-    def setMutable()(using Context): Unit =
+    def associateWithMutable()(using Context): Unit =
       mutability = Mutable
 
     def isConst(using Context) = solved >= ccState.iterationId
@@ -1028,6 +1038,9 @@ object CaptureSet:
 
     addAsDependentTo(source)
 
+    /** Mutability is same as in source, except for readOnly */
+    override def associateWithMutable()(using Context): Unit = ()
+
     override def mutableToReader(origin: CaptureSet)(using Context): Boolean =
       super.mutableToReader(origin)
       && ((origin eq source) || source.mutableToReader(this))
@@ -1364,7 +1377,7 @@ object CaptureSet:
     def description(using Context): String =
       def ofType(tp: Type) = if tp.exists then i"of the mutable type $tp" else "of a mutable type"
       i"""$cs is an exclusive capture set ${ofType(hi)},
-          |it cannot subsume a read-only capture set ${ofType(lo)}."""
+          |it cannot subsume a read-only capture set ${ofType(lo)}"""
 
   /** A VarState serves as a snapshot mechanism that can undo
    *  additions of elements or super sets if an operation fails

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

@@ -39,7 +39,7 @@ object CapturingType:
       case parent @ CapturingType(parent1, refs1) if boxed || !parent.isBoxed =>
         apply(parent1, refs ++ refs1, boxed)
       case _ =>
-        if parent.derivesFromMutable then refs.setMutable()
+        if parent.derivesFromMutable then refs.associateWithMutable()
         refs.adoptClassifier(parent.classifier)
         AnnotatedType(parent, CaptureAnnotation(refs, boxed)(defn.RetainsAnnot))
 

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

@@ -680,12 +680,20 @@ class CheckCaptures extends Recheck, SymTransformer:
           if pt.select.symbol.isReadOnlyMethod then
             markFree(ref.readOnly, tree)
           else
-            markPathFree(ref.select(pt.select.symbol).asInstanceOf[TermRef], pt.pt, pt.select)
+            val sel = ref.select(pt.select.symbol).asInstanceOf[TermRef]
+            sel.recomputeDenot()
+              // We need to do a recomputeDenot here since we have not yet properly
+              // computed the type of the full path. This means that we erroneously
+              // think the denotation is the same as in the previous phase so no
+              // member computation is performed. A test case where this matters is
+              // read-only-use.scala, where the error on r3 goes unreported.
+            markPathFree(sel, pt.pt, pt.select)
         case _ =>
-          if ref.derivesFromMutable && pt.isValueType && !pt.isMutableType then
-            markFree(ref.readOnly, tree)
-          else
-            markFree(ref, tree)
+          if ref.derivesFromMutable then
+            if pt.isValueType && !pt.isMutableType || ref.exclusivityInContext != Exclusivity.OK
+            then markFree(ref.readOnly, tree)
+            else markFree(ref, tree)
+          else markFree(ref, tree)
 
     /** The expected type for the qualifier of a selection. If the selection
      *  could be part of a capability path or is a a read-only method, we return
@@ -724,13 +732,10 @@ class CheckCaptures extends Recheck, SymTransformer:
         case _ => denot
 
       // Don't allow update methods to be called unless the qualifier captures
-      // an exclusive reference. TODO This should probably rolled into
-      // qualifier logic once we have it.
-      if tree.symbol.isUpdateMethod && !qualType.captureSet.isExclusive then
-        report.error(
-            em"""cannot call update ${tree.symbol} from $qualType,
-                |since its capture set ${qualType.captureSet} is read-only""",
-            tree.srcPos)
+      // an exclusive reference.
+      if tree.symbol.isUpdateMethod then
+        checkUpdate(qualType, tree.srcPos):
+          i"Cannot call update ${tree.symbol} of ${qualType.showRef}"
 
       val origSelType = recheckSelection(tree, qualType, name, disambiguate)
       val selType = mapResultRoots(origSelType, tree.symbol)
@@ -768,6 +773,12 @@ class CheckCaptures extends Recheck, SymTransformer:
           selType
     }//.showing(i"recheck sel $tree, $qualType = $result")
 
+    def checkUpdate(qualType: Type, pos: SrcPos)(msg: => String)(using Context): Unit =
+      qualType.exclusivityInContext match
+        case Exclusivity.OK =>
+        case err =>
+          report.error(em"$msg\nsince ${err.description(qualType)}.", pos)
+
     /** Recheck applications, with special handling of unsafeAssumePure.
      *  More work is done in `recheckApplication`, `recheckArg` and `instantiate` below.
      */
@@ -997,6 +1008,16 @@ class CheckCaptures extends Recheck, SymTransformer:
             report.error(em"$refArg is not a tracked capability", refArg.srcPos)
       case _ =>
 
+    override def recheckAssign(tree: Assign)(using Context): Type =
+      val lhsType = recheck(tree.lhs, LhsProto)
+      recheck(tree.rhs, lhsType.widen)
+      lhsType match
+        case lhsType @ TermRef(qualType, _)
+        if (qualType ne NoPrefix) && !lhsType.symbol.is(Transparent) =>
+          checkUpdate(qualType, tree.srcPos)(i"Cannot assign to field ${lhsType.name} of ${qualType.showRef}")
+        case _ =>
+      defn.UnitType
+
     /** Recheck Closure node: add the captured vars of the anonymoys function
      *  to the result type. See also `recheckClosureBlock` which rechecks the
      *  block containing the anonymous function and the Closure node.
@@ -1836,6 +1857,17 @@ class CheckCaptures extends Recheck, SymTransformer:
         actual
     end improveReadOnly
 
+    def adaptReadOnly(improved: Type, original: Type, expected: Type, tree: Tree)(using Context): Type = improved match
+      case improved @ CapturingType(parent, refs)
+      if parent.derivesFrom(defn.Caps_Mutable)
+          && expected.isValueType
+          && refs.isExclusive
+          && !original.exclusivityInContext.isOK =>
+        improved.derivedCapturingType(parent, refs.readOnly)
+          .showing(i"Adapted readonly $improved for $tree with original = $original in ${ctx.owner} --> $result", capt)
+      case _ =>
+        improved
+
     /* Currently not needed since it forms part of `adapt`
     private def improve(actual: Type, prefix: Type)(using Context): Type =
       val widened = actual.widen.dealiasKeepAnnots
@@ -1873,10 +1905,11 @@ class CheckCaptures extends Recheck, SymTransformer:
         val widened = actual.widen.dealiasKeepAnnots.dropUseAndConsumeAnnots
         val improvedVAR = improveCaptures(widened, actual)
         val improved = improveReadOnly(improvedVAR, expected)
+        val adaptedReadOnly = adaptReadOnly(improved, actual, expected, tree)
         val adapted = adaptBoxed(
-            improved.withReachCaptures(actual), expected, tree,
+            adaptedReadOnly.withReachCaptures(actual), expected, tree,
             covariant = true, alwaysConst = false)
-        if adapted eq improvedVAR // no .rd improvement, no box-adaptation
+        if adapted eq improvedVAR // no .rd improvement or adaptation, no box-adaptation
         then actual               // might as well use actual instead of improved widened
         else adapted.showing(i"adapt $actual vs $expected = $adapted", capt)
     end adapt

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

@@ -689,9 +689,9 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI:
                 // 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 readonly capability to
-                  // ensure we cannot treat the class as pure.
-                  CaptureSet.fresh(cls, cls.thisType, Origin.InDecl(cls)).readOnly.subCaptures(cs)
+                  // 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

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

@@ -630,8 +630,13 @@ object Checking {
     if sym.is(Transparent) then
       if sym.isType then
         if !sym.isExtensibleClass then fail(em"`transparent` can only be used for extensible classes and traits")
+      else if sym.isMutableVar && sym.owner.isClass && Feature.ccEnabled
+          || sym.isInlineMethod
+      then
+        () // ok
       else
-        if !sym.isInlineMethod then fail(em"`transparent` can only be used for inline methods")
+        def ccAdd = if Feature.ccEnabled then i" and mutable fields" else ""
+        fail(em"`transparent` can only be used for inline methods$ccAdd")
     if (!sym.isClass && sym.is(Abstract))
       fail(OnlyClassesCanBeAbstract(sym))
         // note: this is not covered by the next test since terms can be abstract (which is a dual-mode flag)
@@ -691,7 +696,7 @@ object Checking {
     if sym.isWrappedToplevelDef && !sym.isType && sym.flags.is(Infix, butNot = Extension) then
       fail(ModifierNotAllowedForDefinition(Flags.Infix, s"A top-level ${sym.showKind} cannot be infix."))
     if sym.isUpdateMethod && !sym.owner.derivesFrom(defn.Caps_Mutable) then
-      fail(em"Update methods can only be used as members of classes extending the `Mutable` trait")
+      fail(em"Update method ${sym.name} must be declared in a class extending the `Mutable` trait.")
     if sym.is(Erased) then checkErasedOK(sym)
     checkCombination(Final, Open)
     checkCombination(Sealed, Open)