Skip to content

Commit eb1a0af

Browse files
authored
Fix handling of @use and @consume in class parameters (#23342)
Multiple fixes, both to keep the annotations in place for capture checking, and to consult them when we do leak checking. Fixes #23303
2 parents 145340d + c8a0a26 commit eb1a0af

File tree

7 files changed

+53
-25
lines changed

7 files changed

+53
-25
lines changed

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -375,15 +375,18 @@ object Capabilities:
375375
case tp1: FreshCap => tp1.ccOwner
376376
case _ => NoSymbol
377377

378-
final def isParamPath(using Context): Boolean = this match
378+
final def paramPathRoot(using Context): Type = core match
379379
case tp1: NamedType =>
380380
tp1.prefix match
381381
case _: ThisType | NoPrefix =>
382-
tp1.symbol.is(Param) || tp1.symbol.is(ParamAccessor)
383-
case prefix: CoreCapability => prefix.isParamPath
384-
case _ => false
385-
case _: ParamRef => true
386-
case _ => false
382+
if tp1.symbol.is(Param) || tp1.symbol.is(ParamAccessor) then tp1
383+
else NoType
384+
case prefix: CoreCapability => prefix.paramPathRoot
385+
case _ => NoType
386+
case tp1: ParamRef => tp1
387+
case _ => NoType
388+
389+
final def isParamPath(using Context): Boolean = paramPathRoot.exists
387390

388391
final def ccOwner(using Context): Symbol = this match
389392
case self: ThisType => self.cls

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,6 @@ object CheckCaptures:
5959

6060
def isOutermost = outer0 == null
6161

62-
/** If an environment is open it tracks free references */
63-
def isOpen(using Context) = !captured.isAlwaysEmpty && kind != EnvKind.Boxed
64-
6562
def outersIterator: Iterator[Env] = new:
6663
private var cur = Env.this
6764
def hasNext = !cur.isOutermost
@@ -466,7 +463,7 @@ class CheckCaptures extends Recheck, SymTransformer:
466463
def checkUseDeclared(c: Capability, env: Env, lastEnv: Env | Null) =
467464
if lastEnv != null && env.nestedClosure.exists && env.nestedClosure == lastEnv.owner then
468465
assert(ccConfig.deferredReaches) // access is from a nested closure under deferredReaches, so it's OK
469-
else c.pathRoot match
466+
else c.paramPathRoot match
470467
case ref: NamedType if !ref.symbol.isUseParam =>
471468
val what = if ref.isType then "Capture set parameter" else "Local reach capability"
472469
report.error(
@@ -528,7 +525,7 @@ class CheckCaptures extends Recheck, SymTransformer:
528525
case _ =>
529526

530527
def recur(cs: CaptureSet, env: Env, lastEnv: Env | Null): Unit =
531-
if env.isOpen && !env.owner.isStaticOwner && !cs.isAlwaysEmpty then
528+
if env.kind != EnvKind.Boxed && !env.owner.isStaticOwner && !cs.isAlwaysEmpty then
532529
// Only captured references that are visible from the environment
533530
// should be included.
534531
val included = cs.filter: c =>
@@ -556,7 +553,7 @@ class CheckCaptures extends Recheck, SymTransformer:
556553
def isRetained(ref: Capability): Boolean = ref.pathRoot match
557554
case root: ThisType => ctx.owner.isContainedIn(root.cls)
558555
case _ => true
559-
if sym.exists && curEnv.isOpen then
556+
if sym.exists && curEnv.kind != EnvKind.Boxed then
560557
markFree(capturedVars(sym).filter(isRetained), tree)
561558

562559
/** If `tp` (possibly after widening singletons) is an ExprType
@@ -1106,7 +1103,6 @@ class CheckCaptures extends Recheck, SymTransformer:
11061103
try
11071104
// Setup environment to reflect the new owner.
11081105
val envForOwner: Map[Symbol, Env] = curEnv.outersIterator
1109-
.takeWhile(e => !capturedVars(e.owner).isAlwaysEmpty) // no refs can leak beyond this point
11101106
.map(e => (e.owner, e))
11111107
.toMap
11121108
def restoreEnvFor(sym: Symbol): Env =
@@ -1142,8 +1138,7 @@ class CheckCaptures extends Recheck, SymTransformer:
11421138
checkSubset(capturedVars(parent.tpe.classSymbol), localSet, parent.srcPos,
11431139
i"\nof the references allowed to be captured by $cls")
11441140
val saved = curEnv
1145-
if localSet ne CaptureSet.empty then
1146-
curEnv = Env(cls, EnvKind.Regular, localSet, curEnv)
1141+
curEnv = Env(cls, EnvKind.Regular, localSet, curEnv)
11471142
try
11481143
val thisSet = cls.classInfo.selfType.captureSet.withDescription(i"of the self type of $cls")
11491144
checkSubset(localSet, thisSet, tree.srcPos) // (2)

compiler/src/dotty/tools/dotc/core/Definitions.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1107,7 +1107,7 @@ class Definitions {
11071107
@tu lazy val NonBeanMetaAnnots: Set[Symbol] =
11081108
Set(FieldMetaAnnot, GetterMetaAnnot, ParamMetaAnnot, SetterMetaAnnot, CompanionClassMetaAnnot, CompanionMethodMetaAnnot)
11091109
@tu lazy val NonBeanParamAccessorAnnots: Set[Symbol] =
1110-
Set(PublicInBinaryAnnot)
1110+
Set(PublicInBinaryAnnot, UseAnnot, ConsumeAnnot)
11111111
@tu lazy val MetaAnnots: Set[Symbol] =
11121112
NonBeanMetaAnnots + BeanGetterMetaAnnot + BeanSetterMetaAnnot
11131113

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -252,11 +252,16 @@ object SymDenotations {
252252
final def filterAnnotations(p: Annotation => Boolean)(using Context): Unit =
253253
annotations = annotations.filterConserve(p)
254254

255-
def annotationsCarrying(meta: Set[Symbol], orNoneOf: Set[Symbol] = Set.empty)(using Context): List[Annotation] =
256-
annotations.filterConserve(_.hasOneOfMetaAnnotation(meta, orNoneOf = orNoneOf))
257-
258-
def keepAnnotationsCarrying(phase: DenotTransformer, meta: Set[Symbol], orNoneOf: Set[Symbol] = Set.empty)(using Context): Unit =
259-
updateAnnotationsAfter(phase, annotationsCarrying(meta, orNoneOf = orNoneOf))
255+
def annotationsCarrying(meta: Set[Symbol],
256+
orNoneOf: Set[Symbol] = Set.empty,
257+
andAlso: Set[Symbol] = Set.empty)(using Context): List[Annotation] =
258+
annotations.filterConserve: annot =>
259+
annot.hasOneOfMetaAnnotation(meta, orNoneOf = orNoneOf)
260+
|| andAlso.contains(annot.symbol)
261+
262+
def keepAnnotationsCarrying(phase: DenotTransformer, meta: Set[Symbol],
263+
orNoneOf: Set[Symbol] = Set.empty, andAlso: Set[Symbol] = Set.empty)(using Context): Unit =
264+
updateAnnotationsAfter(phase, annotationsCarrying(meta, orNoneOf, andAlso))
260265

261266
def updateAnnotationsAfter(phase: DenotTransformer, annots: List[Annotation])(using Context): Unit =
262267
if annots ne annotations then

compiler/src/dotty/tools/dotc/transform/PostTyper.scala

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,8 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
255255
sym.keepAnnotationsCarrying(thisPhase, Set(defn.ParamMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots)
256256
unusing.foreach(sym.addAnnotation)
257257
else if sym.is(ParamAccessor) then
258-
// @publicInBinary is not a meta-annotation and therefore not kept by `keepAnnotationsCarrying`
259-
val publicInBinaryAnnotOpt = sym.getAnnotation(defn.PublicInBinaryAnnot)
260-
sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot))
261-
for publicInBinaryAnnot <- publicInBinaryAnnotOpt do sym.addAnnotation(publicInBinaryAnnot)
258+
sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot),
259+
andAlso = defn.NonBeanParamAccessorAnnots)
262260
else
263261
sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots)
264262
if sym.isScala2Macro && !ctx.settings.XignoreScala2Macros.value &&
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
-- Error: tests/neg-custom-args/captures/i23303.scala:6:36 -------------------------------------------------------------
2+
6 | def execute: Unit = ops.foreach(f => f()) // error
3+
| ^^^^^^^^
4+
| Local reach capability Runner.this.ops* leaks into capture scope of class Runner.
5+
| To allow this, the value ops should be declared with a @use annotation
6+
-- Error: tests/neg-custom-args/captures/i23303.scala:9:22 -------------------------------------------------------------
7+
9 | () => ops.foreach(f => f()) // error
8+
| ^^^^^^^^
9+
| Local reach capability ops* leaks into capture scope of method Runner2.
10+
| To allow this, the parameter ops should be declared with a @use annotation
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import language.experimental.captureChecking
2+
import caps.use
3+
4+
class Test:
5+
class Runner(ops: List[() => Unit]):
6+
def execute: Unit = ops.foreach(f => f()) // error
7+
8+
def Runner2(ops: List[() => Unit]) =
9+
() => ops.foreach(f => f()) // error
10+
11+
12+
class Test2:
13+
class Runner(@use ops: List[() => Unit]):
14+
def execute: Unit = ops.foreach(f => f()) //ok
15+
16+
private def Runner2(@use ops: List[() => Unit]) =
17+
() => ops.foreach(f => f()) // ok

0 commit comments

Comments
 (0)