From 3c36c3cb98c28ee7703eb87cb6f59cae8d861082 Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 10 Jun 2025 13:56:52 +0200 Subject: [PATCH 1/4] Keep @use and @consume on parameter accessors --- .../src/dotty/tools/dotc/core/Definitions.scala | 2 +- .../dotty/tools/dotc/core/SymDenotations.scala | 15 ++++++++++----- .../dotty/tools/dotc/transform/PostTyper.scala | 6 ++---- tests/neg-custom-args/captures/i23303.scala | 17 +++++++++++++++++ 4 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 tests/neg-custom-args/captures/i23303.scala diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 6bce582958a6..381caa775dbd 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -1107,7 +1107,7 @@ class Definitions { @tu lazy val NonBeanMetaAnnots: Set[Symbol] = Set(FieldMetaAnnot, GetterMetaAnnot, ParamMetaAnnot, SetterMetaAnnot, CompanionClassMetaAnnot, CompanionMethodMetaAnnot) @tu lazy val NonBeanParamAccessorAnnots: Set[Symbol] = - Set(PublicInBinaryAnnot) + Set(PublicInBinaryAnnot, UseAnnot, ConsumeAnnot) @tu lazy val MetaAnnots: Set[Symbol] = NonBeanMetaAnnots + BeanGetterMetaAnnot + BeanSetterMetaAnnot diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 271df0b715ab..d200680d172b 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -252,11 +252,16 @@ object SymDenotations { final def filterAnnotations(p: Annotation => Boolean)(using Context): Unit = annotations = annotations.filterConserve(p) - def annotationsCarrying(meta: Set[Symbol], orNoneOf: Set[Symbol] = Set.empty)(using Context): List[Annotation] = - annotations.filterConserve(_.hasOneOfMetaAnnotation(meta, orNoneOf = orNoneOf)) - - def keepAnnotationsCarrying(phase: DenotTransformer, meta: Set[Symbol], orNoneOf: Set[Symbol] = Set.empty)(using Context): Unit = - updateAnnotationsAfter(phase, annotationsCarrying(meta, orNoneOf = orNoneOf)) + def annotationsCarrying(meta: Set[Symbol], + orNoneOf: Set[Symbol] = Set.empty, + andAlso: Set[Symbol] = Set.empty)(using Context): List[Annotation] = + annotations.filterConserve: annot => + annot.hasOneOfMetaAnnotation(meta, orNoneOf = orNoneOf) + || andAlso.contains(annot.symbol) + + def keepAnnotationsCarrying(phase: DenotTransformer, meta: Set[Symbol], + orNoneOf: Set[Symbol] = Set.empty, andAlso: Set[Symbol] = Set.empty)(using Context): Unit = + updateAnnotationsAfter(phase, annotationsCarrying(meta, orNoneOf, andAlso)) def updateAnnotationsAfter(phase: DenotTransformer, annots: List[Annotation])(using Context): Unit = if annots ne annotations then diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index 9ea8a5e7969c..ea35d429e3ef 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -255,10 +255,8 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase => sym.keepAnnotationsCarrying(thisPhase, Set(defn.ParamMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots) unusing.foreach(sym.addAnnotation) else if sym.is(ParamAccessor) then - // @publicInBinary is not a meta-annotation and therefore not kept by `keepAnnotationsCarrying` - val publicInBinaryAnnotOpt = sym.getAnnotation(defn.PublicInBinaryAnnot) - sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot)) - for publicInBinaryAnnot <- publicInBinaryAnnotOpt do sym.addAnnotation(publicInBinaryAnnot) + sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot), + andAlso = defn.NonBeanParamAccessorAnnots) else sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots) if sym.isScala2Macro && !ctx.settings.XignoreScala2Macros.value && diff --git a/tests/neg-custom-args/captures/i23303.scala b/tests/neg-custom-args/captures/i23303.scala new file mode 100644 index 000000000000..09a2c7ea9f4e --- /dev/null +++ b/tests/neg-custom-args/captures/i23303.scala @@ -0,0 +1,17 @@ +import language.experimental.captureChecking +import caps.use + +/*class Test: + class Runner(ops: List[() => Unit]): + def execute: Unit = ops.foreach(f => f()) // error + + def Runner2(ops: List[() => Unit]) = + () => ops.foreach(f => f()) // error +*/ + +class Test2: + class Runner(@use ops: List[() => Unit]): + def execute: Unit = ops.foreach(f => f()) //ok + + private def Runner2(@use ops: List[() => Unit]) = + () => ops.foreach(f => f()) // ok From eaa56a3c5ba0300fd17e8bc5ec1bdb44c430325f Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 10 Jun 2025 15:15:11 +0200 Subject: [PATCH 2/4] Create environments for classes even if their capture set is empty Needed so that markFree can get there to check reach capabilities --- compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala index 20824f7eac80..83582f91707d 100644 --- a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala +++ b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala @@ -1106,7 +1106,6 @@ class CheckCaptures extends Recheck, SymTransformer: try // Setup environment to reflect the new owner. val envForOwner: Map[Symbol, Env] = curEnv.outersIterator - .takeWhile(e => !capturedVars(e.owner).isAlwaysEmpty) // no refs can leak beyond this point .map(e => (e.owner, e)) .toMap def restoreEnvFor(sym: Symbol): Env = @@ -1142,8 +1141,7 @@ class CheckCaptures extends Recheck, SymTransformer: checkSubset(capturedVars(parent.tpe.classSymbol), localSet, parent.srcPos, i"\nof the references allowed to be captured by $cls") val saved = curEnv - if localSet ne CaptureSet.empty then - curEnv = Env(cls, EnvKind.Regular, localSet, curEnv) + curEnv = Env(cls, EnvKind.Regular, localSet, curEnv) try val thisSet = cls.classInfo.selfType.captureSet.withDescription(i"of the self type of $cls") checkSubset(localSet, thisSet, tree.srcPos) // (2) From bd4f772ac77c52af6a3e05e133df4f37b82b7d0f Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 10 Jun 2025 15:17:32 +0200 Subject: [PATCH 3/4] Don't stop at environments with empty capture sets in markFree Needed so that markFree can get to where it needs to check reach capabilities of classes. --- compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala index 83582f91707d..dc82dcd3446b 100644 --- a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala +++ b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala @@ -59,9 +59,6 @@ object CheckCaptures: def isOutermost = outer0 == null - /** If an environment is open it tracks free references */ - def isOpen(using Context) = !captured.isAlwaysEmpty && kind != EnvKind.Boxed - def outersIterator: Iterator[Env] = new: private var cur = Env.this def hasNext = !cur.isOutermost @@ -528,7 +525,7 @@ class CheckCaptures extends Recheck, SymTransformer: case _ => def recur(cs: CaptureSet, env: Env, lastEnv: Env | Null): Unit = - if env.isOpen && !env.owner.isStaticOwner && !cs.isAlwaysEmpty then + if env.kind != EnvKind.Boxed && !env.owner.isStaticOwner && !cs.isAlwaysEmpty then // Only captured references that are visible from the environment // should be included. val included = cs.filter: c => @@ -556,7 +553,7 @@ class CheckCaptures extends Recheck, SymTransformer: def isRetained(ref: Capability): Boolean = ref.pathRoot match case root: ThisType => ctx.owner.isContainedIn(root.cls) case _ => true - if sym.exists && curEnv.isOpen then + if sym.exists && curEnv.kind != EnvKind.Boxed then markFree(capturedVars(sym).filter(isRetained), tree) /** If `tp` (possibly after widening singletons) is an ExprType From c8a0a2672fcd7cba86d7d9fcae20f7a6617b6356 Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 10 Jun 2025 15:18:36 +0200 Subject: [PATCH 4/4] Fix concept of a parameter in which a path is rooted pathRoot overshoots, selects `this` as the root of `this.param`. --- compiler/src/dotty/tools/dotc/cc/Capability.scala | 15 +++++++++------ .../src/dotty/tools/dotc/cc/CheckCaptures.scala | 2 +- tests/neg-custom-args/captures/i23303.check | 10 ++++++++++ tests/neg-custom-args/captures/i23303.scala | 4 ++-- 4 files changed, 22 insertions(+), 9 deletions(-) create mode 100644 tests/neg-custom-args/captures/i23303.check diff --git a/compiler/src/dotty/tools/dotc/cc/Capability.scala b/compiler/src/dotty/tools/dotc/cc/Capability.scala index ca15fb397a42..1373593bd097 100644 --- a/compiler/src/dotty/tools/dotc/cc/Capability.scala +++ b/compiler/src/dotty/tools/dotc/cc/Capability.scala @@ -375,15 +375,18 @@ object Capabilities: case tp1: FreshCap => tp1.ccOwner case _ => NoSymbol - final def isParamPath(using Context): Boolean = this match + final def paramPathRoot(using Context): Type = core match case tp1: NamedType => tp1.prefix match case _: ThisType | NoPrefix => - tp1.symbol.is(Param) || tp1.symbol.is(ParamAccessor) - case prefix: CoreCapability => prefix.isParamPath - case _ => false - case _: ParamRef => true - case _ => false + if tp1.symbol.is(Param) || tp1.symbol.is(ParamAccessor) then tp1 + else NoType + case prefix: CoreCapability => prefix.paramPathRoot + case _ => NoType + case tp1: ParamRef => tp1 + case _ => NoType + + final def isParamPath(using Context): Boolean = paramPathRoot.exists final def ccOwner(using Context): Symbol = this match case self: ThisType => self.cls diff --git a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala index dc82dcd3446b..47e0379c09f1 100644 --- a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala +++ b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala @@ -463,7 +463,7 @@ class CheckCaptures extends Recheck, SymTransformer: def checkUseDeclared(c: Capability, env: Env, lastEnv: Env | Null) = if lastEnv != null && env.nestedClosure.exists && env.nestedClosure == lastEnv.owner then assert(ccConfig.deferredReaches) // access is from a nested closure under deferredReaches, so it's OK - else c.pathRoot match + else c.paramPathRoot match case ref: NamedType if !ref.symbol.isUseParam => val what = if ref.isType then "Capture set parameter" else "Local reach capability" report.error( diff --git a/tests/neg-custom-args/captures/i23303.check b/tests/neg-custom-args/captures/i23303.check new file mode 100644 index 000000000000..437cd471a42c --- /dev/null +++ b/tests/neg-custom-args/captures/i23303.check @@ -0,0 +1,10 @@ +-- Error: tests/neg-custom-args/captures/i23303.scala:6:36 ------------------------------------------------------------- +6 | def execute: Unit = ops.foreach(f => f()) // error + | ^^^^^^^^ + | Local reach capability Runner.this.ops* leaks into capture scope of class Runner. + | To allow this, the value ops should be declared with a @use annotation +-- Error: tests/neg-custom-args/captures/i23303.scala:9:22 ------------------------------------------------------------- +9 | () => ops.foreach(f => f()) // error + | ^^^^^^^^ + | Local reach capability ops* leaks into capture scope of method Runner2. + | To allow this, the parameter ops should be declared with a @use annotation diff --git a/tests/neg-custom-args/captures/i23303.scala b/tests/neg-custom-args/captures/i23303.scala index 09a2c7ea9f4e..81bb45af315c 100644 --- a/tests/neg-custom-args/captures/i23303.scala +++ b/tests/neg-custom-args/captures/i23303.scala @@ -1,13 +1,13 @@ import language.experimental.captureChecking import caps.use -/*class Test: +class Test: class Runner(ops: List[() => Unit]): def execute: Unit = ops.foreach(f => f()) // error def Runner2(ops: List[() => Unit]) = () => ops.foreach(f => f()) // error -*/ + class Test2: class Runner(@use ops: List[() => Unit]):