Skip to content

Commit

Permalink
Handle reach capabilities correctly in markFree
Browse files Browse the repository at this point in the history
The correct point to address charging reach capabilities is in markFree itself:
When a reach capability goes out of scope, and that capability is not a parameter,
we need to continue with the underlying capture set.
  • Loading branch information
odersky committed Oct 30, 2024
1 parent f6f178b commit 913c5bb
Show file tree
Hide file tree
Showing 25 changed files with 323 additions and 114 deletions.
39 changes: 30 additions & 9 deletions compiler/src/dotty/tools/dotc/cc/CaptureOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -221,19 +221,22 @@ extension (tp: Type)
case tp: SingletonCaptureRef => tp.captureSetOfInfo
case _ => CaptureSet.ofType(tp, followResult = false)

/** The deep capture set of a type.
* For singleton capabilities `x` and reach capabilities `x*`, this is `{x*}`, provided
* the underlying capture set resulting from traversing the type is non-empty.
* For other types this is the union of all covariant capture sets embedded
* in the type, as computed by `CaptureSet.ofTypeDeeply`.
/** The deep capture set of a type. This is by default the union of all
* covariant capture sets embedded in the widened type, as computed by
* `CaptureSet.ofTypeDeeply`. If that set is nonempty, and the type is
* a singleton capability `x` or a reach capability `x*`, the deep capture
* set can be narrowed to`{x*}`.
*/
def deepCaptureSet(using Context): CaptureSet =
val dcs = CaptureSet.ofTypeDeeply(tp.widen.stripCapturing)
if dcs.isAlwaysEmpty then tp.captureSet
else tp match
case tp @ ReachCapability(_) => tp.singletonCaptureSet
case tp: SingletonCaptureRef if tp.isTrackableRef => tp.reach.singletonCaptureSet
case _ => tp.captureSet ++ dcs
case tp @ ReachCapability(_) =>
tp.singletonCaptureSet
case tp: SingletonCaptureRef if tp.isTrackableRef =>
tp.reach.singletonCaptureSet
case _ =>
tp.captureSet ++ dcs

/** A type capturing `ref` */
def capturing(ref: CaptureRef)(using Context): Type =
Expand Down Expand Up @@ -274,10 +277,28 @@ extension (tp: Type)
tp

/** The first element of this path type */
final def pathRoot(using Context): Type = tp.dealias match
final def pathRoot(using Context): Type = tp.dealiasKeepAnnots match
case tp1: NamedType if tp1.symbol.owner.isClass => tp1.prefix.pathRoot
case ReachCapability(tp1) => tp1.pathRoot
case _ => tp

/** If this part starts with `C.this`, the class `C`.
* Otherwise, if it starts with a reference `r`, `r`'s owner.
* Otherwise NoSymbol.
*/
final def pathOwner(using Context): Symbol = pathRoot match
case tp1: NamedType => tp1.symbol.owner
case tp1: ThisType => tp1.cls
case _ => NoSymbol

final def isParamPath(using Context): Boolean = tp.dealias match
case tp1: NamedType =>
tp1.prefix match
case _: ThisType | NoPrefix =>
tp1.symbol.is(Param) || tp1.symbol.is(ParamAccessor)
case prefix => prefix.isParamPath
case _ => false

/** If this is a unboxed capturing type with nonempty capture set, its boxed version.
* Or, if type is a TypeBounds of capturing types, the version where the bounds are boxed.
* The identity for all other types.
Expand Down
134 changes: 70 additions & 64 deletions compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala
Original file line number Diff line number Diff line change
Expand Up @@ -328,20 +328,22 @@ class CheckCaptures extends Recheck, SymTransformer:
then CaptureSet.Var(sym.owner, level = sym.ccLevel)
else CaptureSet.empty)

/** For all nested environments up to `limit` or a closed environment perform `op`,
* but skip environmenrts directly enclosing environments of kind ClosureResult.
/** The next environment enclosing `env` that needs to be charged
* with free references.
* Skips environments directly enclosing environments of kind ClosureResult.
* @param included Whether an environment is included in the range of
* environments to charge. Once `included` is false, no
* more environments need to be charged.
*/
def forallOuterEnvsUpTo(limit: Symbol)(op: Env => Unit)(using Context): Unit =
def recur(env: Env, skip: Boolean): Unit =
if env.isOpen && env.owner != limit then
if !skip then op(env)
if !env.isOutermost then
var nextEnv = env.outer
if env.owner.isConstructor then
if nextEnv.owner != limit && !nextEnv.isOutermost then
nextEnv = nextEnv.outer
recur(nextEnv, skip = env.kind == EnvKind.ClosureResult)
recur(curEnv, skip = false)
def nextEnvToCharge(env: Env, included: Env => Boolean)(using Context): Env =
var nextEnv = env.outer
if env.owner.isConstructor then
if included(nextEnv) then nextEnv = nextEnv.outer
if env.kind == EnvKind.ClosureResult then
// skip this one
nextEnvToCharge(nextEnv, included)
else
nextEnv

/** A description where this environment comes from */
private def provenance(env: Env)(using Context): String =
Expand All @@ -355,7 +357,6 @@ class CheckCaptures extends Recheck, SymTransformer:
else
i"\nof the enclosing ${owner.showLocated}"


/** Include `sym` in the capture sets of all enclosing environments nested in the
* the environment in which `sym` is defined.
*/
Expand All @@ -364,9 +365,12 @@ class CheckCaptures extends Recheck, SymTransformer:

def markFree(sym: Symbol, ref: TermRef, pos: SrcPos)(using Context): Unit =
if sym.exists && ref.isTracked then
forallOuterEnvsUpTo(sym.enclosure): env =>
capt.println(i"Mark $sym with cs ${ref.captureSet} free in ${env.owner}")
checkElem(ref, env.captured, pos, provenance(env))
def recur(env: Env): Unit =
if env.isOpen && env.owner != sym.enclosure then
capt.println(i"Mark $sym with cs ${ref.captureSet} free in ${env.owner}")
checkElem(ref, env.captured, pos, provenance(env))
recur(nextEnvToCharge(env, _.owner != sym.enclosure))
recur(curEnv)

/** Make sure (projected) `cs` is a subset of the capture sets of all enclosing
* environments. At each stage, only include references from `cs` that are outside
Expand All @@ -381,46 +385,53 @@ class CheckCaptures extends Recheck, SymTransformer:
else
!sym.isContainedIn(env.owner)

def checkSubsetEnv(cs: CaptureSet, env: Env)(using Context): Unit =
// Only captured references that are visible from the environment
// should be included.
val included = cs.filter: c =>
c.stripReach match
case ref: NamedType =>
val refSym = ref.symbol
val refOwner = refSym.owner
val isVisible = isVisibleFromEnv(refOwner, env)
if isVisible && !ref.isRootCapability then
ref match
case ref: TermRef if ref.prefix `ne` NoPrefix =>
// If c is a path of a class defined outside the environment,
// we check the capture set of its info.
checkSubsetEnv(ref.captureSetOfInfo, env)
case _ =>
if !isVisible
&& (c.isReach || ref.isType)
&& (!ccConfig.useSealed || refSym.is(Param))
&& refOwner == env.owner
then
if refSym.hasAnnotation(defn.UnboxAnnot) then
capt.println(i"exempt: $ref in $refOwner")
else
// Reach capabilities that go out of scope have to be approximated
// by their underlying capture set, which cannot be universal.
// Reach capabilities of @unboxed parameters are exempted.
val cs = CaptureSet.ofInfo(c)
cs.disallowRootCapability: () =>
report.error(em"Local reach capability $c leaks into capture scope of ${env.ownerString}", pos)
checkSubset(cs, env.captured, pos, provenance(env))
isVisible
case ref: ThisType => isVisibleFromEnv(ref.cls, env)
case _ => false
checkSubset(included, env.captured, pos, provenance(env))
capt.println(i"Include call or box capture $included from $cs in ${env.owner} --> ${env.captured}")

if !cs.isAlwaysEmpty then
forallOuterEnvsUpTo(ctx.owner.topLevelClass): env =>
checkSubsetEnv(cs, env)
def checkUseDeclared(c: CaptureRef, env: Env) =
c.pathRoot match
case ref: NamedType if !ref.symbol.hasAnnotation(defn.UnboxAnnot) =>
val what = if ref.isType then "Capture set parameter" else "Local reach capability"
report.error(
em"""$what $c leaks into capture scope of ${env.ownerString}.
|To allow this, the ${ref.symbol} should be declared with a @use annotation""", pos)
case _ =>

def recur(cs: CaptureSet, env: Env)(using Context): Unit =
if env.isOpen && !env.owner.isStaticOwner && !cs.isAlwaysEmpty then
// Only captured references that are visible from the environment
// should be included.
val included = cs.filter: c =>
val isVisible = c.pathRoot match
case ref: NamedType => isVisibleFromEnv(ref.symbol.owner, env)
case ref: ThisType => isVisibleFromEnv(ref.cls, env)
case ref =>
false
if !isVisible then
c match
case ReachCapability(c1) =>
if c1.isParamPath then
checkUseDeclared(c, env)
else
// When a reach capabilty x* where `x` is not a parameter goes out
// of scope, we need to continue with `x`'s underlying deep capture set.
// It is an error if that set contains cap.
// The same is not an issue for normal capabilities since in a local
// definition `val x = e`, the capabilities of `e` have already been charged.
// Note: It's not true that the underlying capture set of a reach capability
// is always cap. Reach capabilities over paths depend on the prefix, which
// might turn a cap into something else.
// The path-use.scala neg test contains an example.
val underlying = CaptureSet.ofTypeDeeply(c1.widen)
capt.println(i"Widen reach $c to $underlying in ${env.owner}")
underlying.disallowRootCapability: () =>
report.error(em"Local reach capability $c leaks into capture scope of ${env.ownerString}", pos)
recur(underlying, env)
case c: TypeRef if c.isParamPath =>
checkUseDeclared(c, env)
case _ =>
isVisible
checkSubset(included, env.captured, pos, provenance(env))
capt.println(i"Include call or box capture $included from $cs in ${env.owner} --> ${env.captured}")
recur(included, nextEnvToCharge(env, !_.owner.isStaticOwner))
recur(cs, curEnv)
end markFree

/** Include references captured by the called method in the current environment stack */
Expand Down Expand Up @@ -1139,13 +1150,8 @@ class CheckCaptures extends Recheck, SymTransformer:
(erefs /: erefs.elems): (erefs, eref) =>
eref match
case eref: ThisType if isPureContext(ctx.owner, eref.cls) =>
def isOuterRef(aref: Type): Boolean = aref.pathRoot match
case aref: NamedType => eref.cls.isProperlyContainedIn(aref.symbol.owner)
case aref: ThisType => eref.cls.isProperlyContainedIn(aref.cls)
case _ => false

val outerRefs = arefs.filter(isOuterRef)

val outerRefs = arefs.filter: aref =>
eref.cls.isProperlyContainedIn(aref.pathOwner)
// Include implicitly added outer references in the capture set of the class of `eref`.
for outerRef <- outerRefs.elems do
if !erefs.elems.contains(outerRef)
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/cc/Setup.scala
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,8 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI:
report.warning(em"redundant capture: $dom already accounts for $ref", pos)

if ref.captureSetOfInfo.elems.isEmpty && !ref.derivesFrom(defn.Caps_Capability) then
report.error(em"$ref cannot be tracked since its capture set is empty", pos)
val deepStr = if ref.isReach then " deep" else ""
report.error(em"$ref cannot be tracked since its$deepStr capture set is empty", pos)
check(parent.captureSet, parent)

val others =
Expand Down
3 changes: 2 additions & 1 deletion scala2-library-cc/src/scala/collection/Iterator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1159,7 +1159,8 @@ object Iterator extends IterableFactory[Iterator] {
// If we advanced the current iterator to a ConcatIterator, merge it into this one
@tailrec def merge(): Unit =
if (current.isInstanceOf[ConcatIterator[_]]) {
val c = current.asInstanceOf[ConcatIterator[A]]
val c: ConcatIterator[A] { val from: Iterator[A] }
= current.asInstanceOf
current = c.current.asInstanceOf // !!! CC unsafe op
currentHasNextChecked = c.currentHasNextChecked
if (c.tail != null) {
Expand Down
14 changes: 14 additions & 0 deletions tests/neg-custom-args/captures/delayedRunops.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
-- Error: tests/neg-custom-args/captures/delayedRunops.scala:16:13 -----------------------------------------------------
16 | runOps(ops1) // error
| ^^^^
| reference ops* is not included in the allowed capture set {}
| of an enclosing function literal with expected type () -> Unit
-- Error: tests/neg-custom-args/captures/delayedRunops.scala:22:13 -----------------------------------------------------
22 | runOps(ops1) // error
| ^^^^
| Local reach capability ops1* leaks into capture scope of enclosing function
-- Error: tests/neg-custom-args/captures/delayedRunops.scala:28:13 -----------------------------------------------------
28 | runOps(ops1) // error
| ^^^^
| reference ops* is not included in the allowed capture set {}
| of an enclosing function literal with expected type () -> Unit
28 changes: 28 additions & 0 deletions tests/neg-custom-args/captures/delayedRunops.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import language.experimental.captureChecking
import caps.unbox

// ok
def runOps(@unbox ops: List[() => Unit]): Unit =
ops.foreach(op => op())

// ok
def delayedRunOps(@unbox ops: List[() => Unit]): () ->{ops*} Unit = // @unbox should not be necessary in the future
() => runOps(ops)

// unsound: impure operation pretended pure
def delayedRunOps1(ops: List[() => Unit]): () ->{} Unit =
() =>
val ops1 = ops
runOps(ops1) // error

// unsound: impure operation pretended pure
def delayedRunOps2(ops: List[() => Unit]): () ->{} Unit =
() =>
val ops1: List[() => Unit] = ops
runOps(ops1) // error

// unsound: impure operation pretended pure
def delayedRunOps3(ops: List[() => Unit]): () ->{} Unit =
() =>
val ops1: List[() ->{ops*} Unit] = ops
runOps(ops1) // error
55 changes: 55 additions & 0 deletions tests/neg-custom-args/captures/i16114.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
-- Error: tests/neg-custom-args/captures/i16114.scala:18:12 ------------------------------------------------------------
18 | expect[Cap^] { // error
| ^^^^^^^^^^^^
| Sealed type variable T cannot be instantiated to box Cap^ since
| that type captures the root capability `cap`.
| This is often caused by a local capability in an argument of method expect
| leaking as part of its result.
-- Error: tests/neg-custom-args/captures/i16114.scala:20:8 -------------------------------------------------------------
20 | fs // error (limitation)
| ^^
| (fs : Cap^) cannot be referenced here; it is not included in the allowed capture set {io}
| of an enclosing function literal with expected type Unit ->{io} Unit
-- Error: tests/neg-custom-args/captures/i16114.scala:24:12 ------------------------------------------------------------
24 | expect[Cap^] { // error
| ^^^^^^^^^^^^
| Sealed type variable T cannot be instantiated to box Cap^ since
| that type captures the root capability `cap`.
| This is often caused by a local capability in an argument of method expect
| leaking as part of its result.
-- Error: tests/neg-custom-args/captures/i16114.scala:26:8 -------------------------------------------------------------
26 | io // error (limitation)
| ^^
| (io : Cap^) cannot be referenced here; it is not included in the allowed capture set {fs}
| of an enclosing function literal with expected type Unit ->{fs} Unit
-- Error: tests/neg-custom-args/captures/i16114.scala:30:12 ------------------------------------------------------------
30 | expect[Cap^] { // error
| ^^^^^^^^^^^^
| Sealed type variable T cannot be instantiated to box Cap^ since
| that type captures the root capability `cap`.
| This is often caused by a local capability in an argument of method expect
| leaking as part of its result.
-- Error: tests/neg-custom-args/captures/i16114.scala:36:12 ------------------------------------------------------------
36 | expect[Cap^](io) // error
| ^^^^^^^^^^^^
| Sealed type variable T cannot be instantiated to box Cap^ since
| that type captures the root capability `cap`.
| This is often caused by a local capability in an argument of method expect
| leaking as part of its result.
-- Error: tests/neg-custom-args/captures/i16114.scala:39:12 ------------------------------------------------------------
39 | expect[Cap^] { // error
| ^^^^^^^^^^^^
| Sealed type variable T cannot be instantiated to box Cap^ since
| that type captures the root capability `cap`.
| This is often caused by a local capability in an argument of method expect
| leaking as part of its result.
-- Error: tests/neg-custom-args/captures/i16114.scala:40:8 -------------------------------------------------------------
40 | io.use() // error
| ^^
| (io : Cap^) cannot be referenced here; it is not included in the allowed capture set {}
| of an enclosing function literal with expected type Unit -> Unit
-- Error: tests/neg-custom-args/captures/i16114.scala:41:8 -------------------------------------------------------------
41 | io // error
| ^^
| (io : Cap^) cannot be referenced here; it is not included in the allowed capture set {}
| of an enclosing function literal with expected type Unit -> Unit
11 changes: 4 additions & 7 deletions tests/neg-custom-args/captures/i21347.check
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
-- Error: tests/neg-custom-args/captures/i21347.scala:4:15 -------------------------------------------------------------
4 | ops.foreach: op => // error
| ^
| Local reach capability C leaks into capture scope of method runOps
| Capture set parameter C leaks into capture scope of method runOps.
| To allow this, the type C should be declared with a @use annotation
5 | op()
-- Error: tests/neg-custom-args/captures/i21347.scala:8:14 -------------------------------------------------------------
8 | () => runOps(f :: Nil) // error
| ^^^^^^^^^^^^^^^^
| reference (caps.cap : caps.Capability) is not included in the allowed capture set {}
| of an enclosing function literal with expected type () -> Unit
-- Error: tests/neg-custom-args/captures/i21347.scala:11:15 ------------------------------------------------------------
11 | ops.foreach: op => // error
| ^
| Local reach capability ops* leaks into capture scope of method runOpsAlt
| Local reach capability ops* leaks into capture scope of method runOpsAlt.
| To allow this, the parameter ops should be declared with a @use annotation
12 | op()
2 changes: 1 addition & 1 deletion tests/neg-custom-args/captures/i21347.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def runOps[C^](ops: List[() ->{C^} Unit]): Unit =
op()

def boom(f: () => Unit): () -> Unit =
() => runOps(f :: Nil) // error
() => runOps(f :: Nil) // now ok

def runOpsAlt(ops: List[() => Unit]): Unit =
ops.foreach: op => // error
Expand Down
Loading

0 comments on commit 913c5bb

Please sign in to comment.