Skip to content

Commit 0da66e2

Browse files
committed
Fix various issues with maximal capabilities
The subsumes check mistakenly allowed any capability to subsume `cap`, since `cap` is expanded as `caps.cap`, and by the path subcapturing rule `caps.cap <: caps`, where the capture set of `caps` is empty. This allowed quite a few hidden errors to go through. This commit fixes the subcapturing issue and all downstream issues caused by that fix. In particular: - Don't use path comparison for `x subsumes caps.cap` - Don't allow an opened existential on the left of a comparison to leak into a capture set on the right. This would give a "leak" error later in healCaptures. - Print `Fresh.Cap` as `fresh` in error messages where both `cap` and `Fresh.Cap` are printed. This avoid confustion with `cap`. Similarly, print `A => B` as `A ->{fresh B` in that case. - Print pre-cc annotated capturing types with @retains annotations with `^`. The annotation is already rendered as a set in this case, but the `^` was missing. - Don't recheck `_` right hand sides of uninitialized variables. These were handled in ways that broke freshness checking. The new `uninitialied` scheme does not have this problem. - Convert cap to fresh in type arguments of asInstanceOf - Have Fresh.FromCap also work for pre-cc @retains annotated types - Don't cache captureSetOfInfos under mode IgnoreCaptures - Let cap and Fresh.Cap subsume other refs only if these others refs cannot be added separately to a capture set. - When creating a instance of a capability class, assume fresh.cap, not cap as capability. Three tests had to be disabled, they were renamed from test.scala to test.scala.disabled. Two of these will work again when we stop boxing alias types (next commit). The third, ex-fun-aliases needs more investigation. Also, the neg test box-adapt-cases needs to be re-evaluated.
1 parent 0f49ba4 commit 0da66e2

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+355
-130
lines changed

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

+20-1
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,12 @@ import CCState.*
1818
import reporting.Message
1919
import CaptureSet.Frozen
2020

21+
/** Attachment key for capturing type trees */
2122
private val Captures: Key[CaptureSet] = Key()
2223

24+
/** Context property to print Fresh.Cap as "fresh" instead of "cap" */
25+
val PrintFresh: Key[Unit] = Key()
26+
2327
object ccConfig:
2428

2529
/** If true, allow mapping capture set variables under captureChecking with maps that are neither
@@ -50,8 +54,10 @@ object ccConfig:
5054

5155
def useFresh(using Context): Boolean =
5256
Feature.sourceVersion.stable.isAtLeast(SourceVersion.`3.6`)
53-
end ccConfig
5457

58+
def followAliases(using Context): Boolean =
59+
Feature.sourceVersion.stable.isAtLeast(SourceVersion.`3.7`)
60+
end ccConfig
5561

5662
/** Are we at checkCaptures phase? */
5763
def isCaptureChecking(using Context): Boolean =
@@ -668,6 +674,19 @@ class CleanupRetains(using Context) extends TypeMap:
668674
RetainingType(tp, Nil, byName = annot.symbol == defn.RetainsByNameAnnot)
669675
case _ => mapOver(tp)
670676

677+
/** A typemap that follows aliases and keeps their transformed results if
678+
* there is a change.
679+
*/
680+
trait FollowAliasesMap(using Context) extends TypeMap:
681+
var follow = true // Used for debugging so that we can compare results with and w/o following.
682+
def mapFollowingAliases(t: Type): Type =
683+
val t1 = t.dealiasKeepAnnots
684+
if follow && (t1 ne t) then
685+
val t2 = apply(t1)
686+
if t2 ne t1 then t2
687+
else t
688+
else mapOver(t)
689+
671690
/** An extractor for `caps.reachCapability(ref)`, which is used to express a reach
672691
* capability as a tree in a @retains annotation.
673692
*/

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

+30-12
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ trait CaptureRef extends TypeProxy, ValueType:
124124
else
125125
myCaptureSet = CaptureSet.Pending
126126
val computed = CaptureSet.ofInfo(this)
127-
if !isCaptureChecking || underlying.isProvisional then
127+
if !isCaptureChecking || ctx.mode.is(Mode.IgnoreCaptures) || underlying.isProvisional then
128128
myCaptureSet = null
129129
else
130130
myCaptureSet = computed
@@ -165,18 +165,9 @@ trait CaptureRef extends TypeProxy, ValueType:
165165
case _ => false
166166

167167
(this eq y)
168-
|| this.isCap
169-
|| this.match
170-
case Fresh.Cap(hidden) =>
171-
if vs.ifNotSeen(this)(hidden.elems.exists(_.subsumes(y))) then true
172-
else if !hidden.recordElemsState() || y.stripReadOnly.isCap then false
173-
else
174-
hidden.elems += y
175-
true
176-
case _ =>
177-
false
168+
|| maxSubsumes(y, canAddHidden = vs.frozen != CaptureSet.Frozen.None)
178169
|| y.match
179-
case y: TermRef =>
170+
case y: TermRef if !y.isCap =>
180171
y.prefix.match
181172
case ypre: CaptureRef =>
182173
this.subsumes(ypre)
@@ -221,6 +212,33 @@ trait CaptureRef extends TypeProxy, ValueType:
221212
case _ => false
222213
end subsumes
223214

215+
/** This is a maximal capabaility that subsumes `y` in given context and VarState.
216+
* @param canAddHidden If true we allow maximal capabilties to subsume all other capabilities.
217+
* We add those capabilities to the hidden set if this is Fresh.Cap
218+
* If false we only accept `y` elements that are already in the
219+
* hidden set of this Fresh.Cap. The idea is that in a VarState that
220+
* accepts additions we first run `maxSubsumes` with `canAddHidden = false`
221+
* so that new variables get added to the sets. If that fails, we run
222+
* the test again with canAddHidden = true as a last effort before we
223+
* fail a comparison.
224+
*/
225+
def maxSubsumes(y: CaptureRef, canAddHidden: Boolean)(using ctx: Context, vs: VarState = FrozenAllState): Boolean =
226+
this.match
227+
case Fresh.Cap(hidden) =>
228+
if vs.ifNotSeen(this)(hidden.elems.exists(_.subsumes(y))) then true
229+
else if !y.stripReadOnly.isCap && hidden.recordElemsState() then
230+
if canAddHidden then
231+
hidden.elems += y
232+
true
233+
else
234+
false
235+
else false
236+
case _ =>
237+
this.isCap && canAddHidden
238+
|| y.match
239+
case ReadOnlyCapability(y1) => this.stripReadOnly.maxSubsumes(y1, canAddHidden)
240+
case _ => false
241+
224242
def assumedContainsOf(x: TypeRef)(using Context): SimpleIdentitySet[CaptureRef] =
225243
CaptureSet.assumedContains.getOrElse(x, SimpleIdentitySet.empty)
226244

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

+27-16
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,11 @@ sealed abstract class CaptureSet extends Showable:
162162
*/
163163
protected def addThisElem(elem: CaptureRef)(using Context, VarState): CompareResult
164164

165+
protected def addHiddenElem(elem: CaptureRef)(using ctx: Context, vs: VarState): CompareResult =
166+
if elems.exists(_.maxSubsumes(elem, canAddHidden = true))
167+
then CompareResult.OK
168+
else CompareResult.Fail(this :: Nil)
169+
165170
/** If this is a variable, add `cs` as a dependent set */
166171
protected def addDependent(cs: CaptureSet)(using Context, VarState): CompareResult
167172

@@ -399,12 +404,6 @@ object CaptureSet:
399404
type Vars = SimpleIdentitySet[Var]
400405
type Deps = SimpleIdentitySet[CaptureSet]
401406

402-
/** An enum indicating a Frozen degree for subCapturing tests */
403-
enum Frozen:
404-
case None // operations are performed in a regular VarState
405-
case Vars // operations are performed in a FrozenVarState
406-
case All // operations are performed in FrozenAllState
407-
408407
@sharable private var varId = 0
409408

410409
/** If set to `true`, capture stack traces that tell us where sets are created */
@@ -442,7 +441,7 @@ object CaptureSet:
442441
def isAlwaysEmpty = elems.isEmpty
443442

444443
def addThisElem(elem: CaptureRef)(using Context, VarState): CompareResult =
445-
CompareResult.Fail(this :: Nil)
444+
addHiddenElem(elem)
446445

447446
def addDependent(cs: CaptureSet)(using Context, VarState) = CompareResult.OK
448447

@@ -538,15 +537,14 @@ object CaptureSet:
538537
deps = state.deps(this)
539538

540539
final def addThisElem(elem: CaptureRef)(using Context, VarState): CompareResult =
541-
if isConst // Fail if variable is solved,
542-
|| !recordElemsState() // or given VarState is frozen,
543-
|| Existential.isBadExistential(elem) // or `elem` is an out-of-scope existential,
544-
then
540+
if isConst || !recordElemsState() then // Fail if variable is solved or given VarState is frozen
541+
addHiddenElem(elem)
542+
else if Existential.isBadExistential(elem) then // Fail if `elem` is an out-of-scope existential
545543
CompareResult.Fail(this :: Nil)
546544
else if !levelOK(elem) then
547545
CompareResult.LevelError(this, elem) // or `elem` is not visible at the level of the set.
548546
else
549-
//if id == 34 then assert(!elem.isUniversalRootCapability)
547+
// id == 108 then assert(false, i"trying to add $elem to $this")
550548
assert(elem.isTrackableRef, elem)
551549
assert(!this.isInstanceOf[HiddenSet] || summon[VarState] == FrozenAllState, summon[VarState])
552550
elems += elem
@@ -562,8 +560,13 @@ object CaptureSet:
562560
res.addToTrace(this)
563561

564562
private def levelOK(elem: CaptureRef)(using Context): Boolean =
565-
if elem.isRootCapability || Existential.isExistentialVar(elem) then
563+
if elem.isRootCapability then
564+
!noUniversal
565+
else if Existential.isExistentialVar(elem) then
566566
!noUniversal
567+
&& !TypeComparer.isOpenedExistential(elem)
568+
// Opened existentials on the left cannot be added to nested capture sets on the right
569+
// of a comparison. Test case is open-existential.scala.
567570
else elem match
568571
case elem: TermRef if level.isDefined =>
569572
elem.prefix match
@@ -635,7 +638,7 @@ object CaptureSet:
635638
*/
636639
def solve()(using Context): Unit =
637640
if !isConst then
638-
val approx = upperApprox(empty)
641+
val approx = upperApprox(empty).map(Fresh.FromCap(NoSymbol).inverse)
639642
.showing(i"solve $this = $result", capt)
640643
//println(i"solving var $this $approx ${approx.isConst} deps = ${deps.toList}")
641644
val newElems = approx.elems -- elems
@@ -1035,11 +1038,19 @@ object CaptureSet:
10351038
case _ => this
10361039
end CompareResult
10371040

1041+
/** An enum indicating a Frozen degree for subCapturing tests */
1042+
enum Frozen:
1043+
case None // operations are performed in a regular VarState
1044+
case Vars // operations are performed in a FrozenVarState
1045+
case All // operations are performed in FrozenAllState
1046+
10381047
/** A VarState serves as a snapshot mechanism that can undo
10391048
* additions of elements or super sets if an operation fails
10401049
*/
10411050
class VarState:
10421051

1052+
def frozen: Frozen = Frozen.None
1053+
10431054
/** A map from captureset variables to their elements at the time of the snapshot. */
10441055
protected val elemsMap: util.EqHashMap[Var, Refs] = new util.EqHashMap
10451056

@@ -1054,7 +1065,6 @@ object CaptureSet:
10541065

10551066
/** Record elements, return whether this was allowed.
10561067
* By default, recording is allowed in regular both not in frozen states.
1057-
* overrides this.
10581068
*/
10591069
def putElems(v: Var, elems: Refs): Boolean = { elemsMap(v) = elems; true }
10601070

@@ -1066,7 +1076,6 @@ object CaptureSet:
10661076

10671077
/** Record dependent sets, return whether this was allowed.
10681078
* By default, recording is allowed in regular both not in frozen states.
1069-
* overrides this.
10701079
*/
10711080
def putDeps(v: Var, deps: Deps): Boolean = { depsMap(v) = deps; true }
10721081

@@ -1097,6 +1106,7 @@ object CaptureSet:
10971106
* subsume arbitary types, which are then recorded in their hidden sets.
10981107
*/
10991108
class FrozenVarState extends VarState:
1109+
override def frozen = Frozen.Vars
11001110
override def putElems(v: Var, refs: Refs) = false
11011111
override def putDeps(v: Var, deps: Deps) = false
11021112
override def putHidden(v: HiddenSet, elems: Refs): Boolean = { elemsMap(v) = elems; true }
@@ -1107,6 +1117,7 @@ object CaptureSet:
11071117
* No new references can be added.
11081118
*/
11091119
object FrozenAllState extends FrozenVarState:
1120+
override def frozen = Frozen.All
11101121
override def putHidden(v: HiddenSet, elems: Refs): Boolean = false
11111122

11121123
@sharable

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

+15-13
Original file line numberDiff line numberDiff line change
@@ -333,20 +333,21 @@ class CheckCaptures extends Recheck, SymTransformer:
333333
assert(cs1.subCaptures(cs2, Frozen.None).isOK, i"$cs1 is not a subset of $cs2")
334334

335335
/** If `res` is not CompareResult.OK, report an error */
336-
def checkOK(res: CompareResult, prefix: => String, pos: SrcPos, provenance: => String = "")(using Context): Unit =
336+
def checkOK(res: CompareResult, prefix: => String, added: CaptureRef | CaptureSet, pos: SrcPos, provenance: => String = "")(using Context): Unit =
337337
if !res.isOK then
338-
def toAdd: String = CaptureSet.levelErrors.toAdd.mkString
339-
def descr: String =
340-
val d = res.blocking.description
341-
if d.isEmpty then provenance else ""
342-
report.error(em"$prefix included in the allowed capture set ${res.blocking}$descr$toAdd", pos)
338+
inContext(Fresh.printContext(added, res.blocking)):
339+
def toAdd: String = CaptureSet.levelErrors.toAdd.mkString
340+
def descr: String =
341+
val d = res.blocking.description
342+
if d.isEmpty then provenance else ""
343+
report.error(em"$prefix included in the allowed capture set ${res.blocking}$descr$toAdd", pos)
343344

344345
/** Check subcapturing `{elem} <: cs`, report error on failure */
345346
def checkElem(elem: CaptureRef, cs: CaptureSet, pos: SrcPos, provenance: => String = "")(using Context) =
346347
checkOK(
347348
elem.singletonCaptureSet.subCaptures(cs, Frozen.None),
348349
i"$elem cannot be referenced here; it is not",
349-
pos, provenance)
350+
elem, pos, provenance)
350351

351352
/** Check subcapturing `cs1 <: cs2`, report error on failure */
352353
def checkSubset(cs1: CaptureSet, cs2: CaptureSet, pos: SrcPos,
@@ -355,7 +356,7 @@ class CheckCaptures extends Recheck, SymTransformer:
355356
cs1.subCaptures(cs2, Frozen.None),
356357
if cs1.elems.size == 1 then i"reference ${cs1.elems.toList.head}$cs1description is not"
357358
else i"references $cs1$cs1description are not all",
358-
pos, provenance)
359+
cs1, pos, provenance)
359360

360361
/** If `sym` is a class or method nested inside a term, a capture set variable representing
361362
* the captured variables of the environment associated with `sym`.
@@ -771,7 +772,7 @@ class CheckCaptures extends Recheck, SymTransformer:
771772
var refined: Type = core
772773
var allCaptures: CaptureSet =
773774
if core.derivesFromMutable then CaptureSet.fresh()
774-
else if core.derivesFromCapability then initCs ++ defn.universalCSImpliedByCapability
775+
else if core.derivesFromCapability then initCs ++ Fresh.Cap().readOnly.singletonCaptureSet
775776
else initCs
776777
for (getterName, argType) <- mt.paramNames.lazyZip(argTypes) do
777778
val getter = cls.info.member(getterName).suchThat(_.isRefiningParamAccessor).symbol
@@ -1227,10 +1228,11 @@ class CheckCaptures extends Recheck, SymTransformer:
12271228
actualBoxed
12281229
else
12291230
capt.println(i"conforms failed for ${tree}: $actual vs $expected")
1230-
err.typeMismatch(tree.withType(actualBoxed), expected1,
1231-
addApproxAddenda(
1232-
addenda ++ CaptureSet.levelErrors ++ boxErrorAddenda(boxErrors),
1233-
expected1))
1231+
inContext(Fresh.printContext(actualBoxed, expected1)):
1232+
err.typeMismatch(tree.withType(actualBoxed), expected1,
1233+
addApproxAddenda(
1234+
addenda ++ CaptureSet.levelErrors ++ boxErrorAddenda(boxErrors),
1235+
expected1))
12341236
actual
12351237
end checkConformsExpr
12361238

0 commit comments

Comments
 (0)