Cache ResultCaps

This fixes problems encountered in #15923 where we got infinite recursions
caused by infinite streams of new ResultCap instances that were added to capture sets.
The analysis of the problem showed that there is a cycle of dependencies involving
both forwards and backwards propagations of ResultCap instances between
BiMapped capture sets linked by SubstBindingMaps. Each propagation would
create a new derived ResultCap instance.

We could try to solve the problem by having SubstBindingMaps remember their mappings
and have their inverses work backwards. But that could still produce an infinite stream
if there was a cycle of SubstBindingMaps of length > 2. So we fix the problem at the
root by allowing only one derived ResultCap instance per original ResultCap / binder pair.

Fixes #15923

Author: odersky

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

@@ -177,15 +177,54 @@ object Capabilities:
    *  is expanded.
    */
   case class ResultCap(binder: MethodicType) extends RootCapability:
-    private var myOriginalBinder = binder
-    def originalBinder: MethodicType = myOriginalBinder
 
+    private var myOrigin: RootCapability = GlobalCap
+    private var variants: SimpleIdentitySet[ResultCap] = SimpleIdentitySet.empty
+
+    /** Every ResultCap capability has an origin. This is
+     *   - A FreshCap capability `f`, if the current capability was created as a mirror
+     *     of `f` in the ToResult map.
+     *   - Another ResultCap capability `r`, if the current capability was created
+     *     via a chain of `derivedResult` calls from an original ResultCap `r`
+     *     (which was not created using `derivedResult`).
+     *   - GlobalCap otherwise
+     */
+    def origin: RootCapability = myOrigin
+
+    /** Initialize origin of this capability to a FreshCap instance (or to GlobalCap
+     *  if separation checks are turned off).
+     *  @pre The capability's origin was not yet set.
+     */
+    def setOrigin(freshOrigin: FreshCap | GlobalCap.type): Unit =
+      assert(myOrigin eq GlobalCap)
+      myOrigin = freshOrigin
+
+    /** If the current capability was created via a chain of `derivedResult` calls
+     *  from an original ResultCap `r`, that `r`. Otherwise `this`.
+     */
+    def primaryResultCap: ResultCap = origin match
+      case origin: ResultCap => origin
+      case _ => this
+
+    def originalBinder: MethodicType = primaryResultCap.binder
+
+    /** A ResultCap with given `binder1` derived from this capability.
+     *  This is typically done as a result of a SubstBinding map.
+     *  ResultCaps so created are cached, so that for every pair
+     *  of a ResultCap `r` and a binder `b`, there exists at most one ResultCap
+     *  instance that is derived transitively from `r` and has binder `b`.
+     */
     def derivedResult(binder1: MethodicType): ResultCap =
       if binder1 eq binder then this
       else
-        val res = ResultCap(binder1)
-        res.myOriginalBinder = myOriginalBinder
-        res
+        val primary = primaryResultCap
+        primary.variants.iterator.find(_.binder eq binder1) match
+          case Some(rcap) => rcap
+          case None =>
+            val rcap = ResultCap(binder1)
+            rcap.myOrigin = primary
+            primary.variants += rcap
+            rcap
   end ResultCap
 
   /** A trait for references in CaptureSets. These can be NamedTypes, ThisTypes or ParamRefs,
@@ -718,7 +757,6 @@ object Capabilities:
           super.mapOver(t)
 
     object toVar extends CapMap:
-      private val seen = EqHashMap[RootCapability, ResultCap]()
 
       def apply(t: Type) = t match
         case defn.FunctionNOf(args, res, contextual) if t.typeSymbol.name.isImpureFunction =>
@@ -733,7 +771,11 @@ object Capabilities:
       override def mapCapability(c: Capability, deep: Boolean) = c match
         case c: (FreshCap | GlobalCap.type) =>
           if variance > 0 then
-            seen.getOrElseUpdate(c, ResultCap(mt))
+            val res = ResultCap(mt)
+            c match
+              case c: FreshCap => res.setOrigin(c)
+              case _ =>
+            res
           else
             if variance == 0 then
               fail(em"""$tp captures the root capability `cap` in invariant position.
@@ -753,15 +795,17 @@ object Capabilities:
           case c @ ResultCap(`mt`) =>
             // do a reverse getOrElseUpdate on `seen` to produce the
             // `Fresh` assosicated with `t`
-            val it = seen.iterator
-            var ref: RootCapability | Null = null
-            while it.hasNext && ref == null do
-              val (k, v) = it.next
-              if v eq c then ref = k
-            if ref == null then
-              ref = FreshCap(Origin.Unknown)
-              seen(ref) = c
-            ref
+            if !ccConfig.useSepChecks then
+              FreshCap(Origin.Unknown) // warning: this can cause cycles
+            else
+              val primary = c.primaryResultCap
+              primary.origin match
+                case GlobalCap =>
+                  val fresh = FreshCap(Origin.Unknown)
+                  primary.setOrigin(fresh)
+                  fresh
+                case origin: FreshCap =>
+                  origin
           case _ =>
             super.mapCapability(c, deep)
 

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

@@ -74,6 +74,9 @@ sealed abstract class CaptureSet extends Showable:
   /** Is this capture set definitely non-empty? */
   final def isNotEmpty: Boolean = !elems.isEmpty
 
+  /** If this is a Var, its `id`, otherwise -1 */
+  def maybeId: Int = -1
+
   /** Convert to Const. @pre: isConst */
   def asConst(using Context): Const = this match
     case c: Const => c
@@ -129,7 +132,7 @@ sealed abstract class CaptureSet extends Showable:
    *  element is not the root capability, try instead to include its underlying
    *  capture set.
    */
-  protected def tryInclude(elem: Capability, origin: CaptureSet)(using Context, VarState): CompareResult =
+  protected def tryInclude(elem: Capability, origin: CaptureSet)(using Context, VarState): CompareResult = reporting.trace(i"try include $elem in $this # ${maybeId}"):
     if accountsFor(elem) then CompareResult.OK
     else addNewElem(elem)
 
@@ -518,6 +521,8 @@ object CaptureSet:
       ccs.varId += 1
       ccs.varId
 
+    override def maybeId = id
+
     //assert(id != 8, this)
 
     /** A variable is solved if it is aproximated to a from-then-on constant set.
@@ -607,7 +612,9 @@ object CaptureSet:
         val normElem = if isMaybeSet then elem else elem.stripMaybe
         // assert(id != 5 || elems.size != 3, this)
         val res = (CompareResult.OK /: deps): (r, dep) =>
-          r.andAlso(dep.tryInclude(normElem, this))
+          r.andAlso:
+            reporting.trace(i"forward $normElem from $this # $id to $dep # ${dep.maybeId} of class ${dep.getClass.toString}"):
+              dep.tryInclude(normElem, this)
         if ccConfig.checkSkippedMaps && res.isOK then checkSkippedMaps(elem)
         res.orElse:
           elems -= elem
@@ -819,12 +826,14 @@ object CaptureSet:
       else if accountsFor(elem) then
         CompareResult.OK
       else
+        // Propagate backwards to source. The element will be added then by another
+        // forward propagation from source that hits the first branch `if origin eq source then`.
         try
-          source.tryInclude(bimap.inverse.mapCapability(elem), this)
-            .showing(i"propagating new elem $elem backward from $this to $source = $result", captDebug)
-            .andAlso(addNewElem(elem))
+          reporting.trace(i"prop backwards $elem from $this # $id to $source # ${source.id} via $summarize"):
+            source.tryInclude(bimap.inverse.mapCapability(elem), this)
+              .showing(i"propagating new elem $elem backward from $this/$id to $source = $result", captDebug)
         catch case ex: AssertionError =>
-          println(i"fail while prop backwards tryInclude $elem of ${elem.getClass} in $this / ${this.summarize}")
+          println(i"fail while prop backwards tryInclude $elem of ${elem.getClass} from $this # $id / ${this.summarize} to $source # ${source.id}")
           throw ex
 
     /** For a BiTypeMap, supertypes of the mapped type also constrain

tests/neg-custom-args/captures/i15923.check

@@ -0,0 +1,23 @@
+-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/i15923.scala:12:21 ---------------------------------------
+12 |  val leak = withCap(cap => mkId(cap)) // error
+   |                     ^^^^^^^^^^^^^^^^
+   |Found:    (lcap: scala.caps.Capability^{cap.rd}) ?->? Cap^{lcap} ->? box Id[box Cap^{cap².rd}]^?
+   |Required: (lcap: scala.caps.Capability^{cap.rd}) ?-> Cap^{lcap} => box Id[box Cap^?]^?
+   |
+   |where:    =>   refers to a root capability associated with the result type of (using lcap: scala.caps.Capability^{cap.rd}): Cap^{lcap} => box Id[box Cap^?]^?
+   |          cap  is the universal root capability
+   |          cap² is a root capability associated with the result type of (x$0: Cap^{lcap}): box Id[box Cap^{cap².rd}]^?
+   |
+   |
+   |Note that reference <cap of (x$0: Cap^{lcap}): box Id[box Cap^{cap.rd}]^?>.rd
+   |cannot be included in outer capture set ?
+   |
+   | longer explanation available when compiling with `-explain`
+-- Warning: tests/neg-custom-args/captures/i15923.scala:21:56 ----------------------------------------------------------
+21 |    def withCap[X](op: (lcap: caps.Capability) ?-> Cap^{lcap} => X): X = {
+   |                                                        ^^^^
+   |                                                     redundant capture: test2.Cap already accounts for lcap.type
+-- Warning: tests/neg-custom-args/captures/i15923.scala:6:54 -----------------------------------------------------------
+6 |  def withCap[X](op: (lcap: caps.Capability) ?-> Cap^{lcap} => X): X = {
+  |                                                      ^^^^
+  |                                                      redundant capture: Cap already accounts for lcap.type

tests/neg-custom-args/captures/i15923.scala

@@ -0,0 +1,29 @@
+trait Cap { def use(): Int }
+class Id[X](val value: [T] -> (op: X => T) -> T)
+def mkId[X](x: X): Id[X] = Id([T] => (op: X => T) => op(x))
+
+def bar() = {
+  def withCap[X](op: (lcap: caps.Capability) ?-> Cap^{lcap} => X): X = {
+    val cap: Cap = new Cap { def use() = { println("cap is used"); 0 } }
+    val result = op(using caps.cap)(cap)
+    result
+  }
+
+  val leak = withCap(cap => mkId(cap)) // error
+}
+
+package test2:
+  trait Cap { def use(): Int }
+  type Id[X] = [T] -> (op: X => T) -> T
+  def mkId[X](x: X): Id[X] = [T] => (op: X => T) => op(x)
+
+  def bar() = {
+    def withCap[X](op: (lcap: caps.Capability) ?-> Cap^{lcap} => X): X = {
+      val cap: Cap = new Cap { def use() = { println("cap is used"); 0 } }
+      val result = op(using caps.cap)(cap)
+      result
+    }
+
+    val leak = withCap(cap => mkId(cap))  // no error here since type aliases don't box
+    leak { cap => cap.use() }
+  }

tests/neg-custom-args/captures/i15923a.check

@@ -0,0 +1,16 @@
+-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/i15923a.scala:7:21 ---------------------------------------
+7 |  val leak = withCap(lcap => () => mkId(lcap)) // error
+  |                     ^^^^^^^^^^^^^^^^^^^^^^^^
+  |Found:    (lcap: Cap^) ->? () ->? box Id[box Cap^²]^?
+  |Required: (lcap: Cap^) => () =>² box Id[box Cap^?]^?
+  |
+  |where:    =>  refers to a fresh root capability created in value leak when checking argument to parameter op of method withCap
+  |          =>² refers to a root capability associated with the result type of (lcap: Cap^): () =>² box Id[box Cap^?]^?
+  |          ^   refers to the universal root capability
+  |          ^²  refers to a root capability associated with the result type of (): box Id[box Cap^²]^?
+  |
+  |
+  |Note that reference <cap of (): box Id[box Cap^]^?>
+  |cannot be included in outer capture set ?
+  |
+  | longer explanation available when compiling with `-explain`

tests/neg-custom-args/captures/i15923a.scala

@@ -0,0 +1,7 @@
+import language.experimental.captureChecking
+trait Cap
+case class Id[X](x: X)
+def mkId[X](x: X): Id[X] = Id(x)
+def withCap[X](op: (lcap: Cap^) => () => X): X = ???
+def bar() =
+  val leak = withCap(lcap => () => mkId(lcap)) // error