Skip to content

Commit 806e2b5

Browse files
committed
Tighten level checking
We had the wrong condition in maxSubsumes before. We need to test for isStaticOwner instead of isStatic. The previous test made all fresh capabilities in global classes slip through the net.
1 parent 414d421 commit 806e2b5

16 files changed

+85
-66
lines changed

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ class CCState:
118118

119119
private var capIsRoot: Boolean = false
120120

121-
private var treatFreshAsEqual: Boolean = false
121+
private var ignoreFreshLevels: Boolean = false
122122

123123
object CCState:
124124

@@ -169,15 +169,15 @@ object CCState:
169169
* Asserted in override checking, tested in maxSubsumes.
170170
* Is this sound? Test case is neg-custom-args/captures/leaked-curried.scala.
171171
*/
172-
inline def withTreatFreshAsEqual[T](op: => T)(using Context): T =
172+
inline def ignoringFreshLevels[T](op: => T)(using Context): T =
173173
if isCaptureCheckingOrSetup then
174174
val ccs = ccState
175-
val saved = ccs.treatFreshAsEqual
176-
ccs.treatFreshAsEqual = true
177-
try op finally ccs.treatFreshAsEqual = saved
175+
val saved = ccs.ignoreFreshLevels
176+
ccs.ignoreFreshLevels = true
177+
try op finally ccs.ignoreFreshLevels = saved
178178
else op
179179

180180
/** Should all root.Fresh instances be treated equal? */
181-
def treatFreshAsEqual(using Context): Boolean = ccState.treatFreshAsEqual
181+
def ignoreFreshLevels(using Context): Boolean = ccState.ignoreFreshLevels
182182

183183
end CCState

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

+6-1
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ package cc
44

55
import core.*
66
import Types.*, Symbols.*, Contexts.*, Annotations.*, Flags.*
7-
import Names.TermName
7+
import Names.{Name, TermName}
88
import ast.{tpd, untpd}
99
import Decorators.*, NameOps.*
1010
import config.Printers.capt
1111
import util.Property.Key
1212
import tpd.*
13+
import Annotations.Annotation
1314
import CaptureSet.VarState
1415

1516
/** Attachment key for capturing type trees */
@@ -504,6 +505,10 @@ extension (tp: Type)
504505
case tp: ThisType => ccState.symLevel(tp.cls).nextInner
505506
case _ => CCState.undefinedLevel
506507

508+
def refinedOverride(name: Name, rinfo: Type)(using Context): Type =
509+
RefinedType(tp, name,
510+
AnnotatedType(rinfo, Annotation(defn.RefineOverrideAnnot, util.Spans.NoSpan)))
511+
507512
extension (tp: MethodType)
508513
/** A method marks an existential scope unless it is the prefix of a curried method */
509514
def marksExistentialScope(using Context): Boolean =

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

+3-4
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ trait CaptureRef extends TypeProxy, ValueType:
131131
final def ccOwner(using Context): Symbol = this match
132132
case root.Fresh(hidden) =>
133133
hidden.owner
134-
case ref: TermRef =>
134+
case ref: NamedType =>
135135
if ref.isCap then NoSymbol
136136
else ref.prefix match
137137
case prefix: CaptureRef => prefix.ccOwner
@@ -295,9 +295,9 @@ trait CaptureRef extends TypeProxy, ValueType:
295295
|| this.match
296296
case x @ root.Fresh(hidden) =>
297297
def levelOK =
298-
if ccConfig.useFreshLevels then
298+
if ccConfig.useFreshLevels && !CCState.ignoreFreshLevels then
299299
val yOwner = y.adjustedOwner
300-
yOwner.isStatic || x.ccOwner.isContainedIn(yOwner)
300+
yOwner.isStaticOwner || x.ccOwner.isContainedIn(yOwner)
301301
else
302302
!y.stripReadOnly.isCap
303303
&& !yIsExistential
@@ -307,7 +307,6 @@ trait CaptureRef extends TypeProxy, ValueType:
307307
|| levelOK
308308
&& canAddHidden
309309
&& vs.addHidden(hidden, y)
310-
|| ccConfig.useFreshLevels && CCState.treatFreshAsEqual && y.stripReadOnly.isFresh
311310
case x @ root.Result(binder) =>
312311
val result = y match
313312
case y @ root.Result(_) => vs.unify(x, y)

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,9 @@ sealed abstract class CaptureSet extends Showable:
209209
*/
210210
def mightAccountFor(x: CaptureRef)(using Context): Boolean =
211211
reporting.trace(i"$this mightAccountFor $x, ${x.captureSetOfInfo}?", show = true):
212-
CCState.withCapAsRoot: // OK here since we opportunistically choose an alternative which gets checked later
213-
elems.exists(_.subsumes(x)(using ctx)(using VarState.ClosedUnrecorded))
212+
CCState.withCapAsRoot:
213+
CCState.ignoringFreshLevels: // OK here since we opportunistically choose an alternative which gets checked later
214+
elems.exists(_.subsumes(x)(using ctx)(using VarState.ClosedUnrecorded))
214215
|| !x.isRootCapability
215216
&& {
216217
val elems = x.captureSetOfInfo.elems

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

+8-8
Original file line numberDiff line numberDiff line change
@@ -843,8 +843,7 @@ class CheckCaptures extends Recheck, SymTransformer:
843843
for (getterName, argType) <- mt.paramNames.lazyZip(argTypes) do
844844
val getter = cls.info.member(getterName).suchThat(_.isRefiningParamAccessor).symbol
845845
if !getter.is(Private) && getter.hasTrackedParts then
846-
refined = RefinedType(refined, getterName,
847-
AnnotatedType(argType.unboxed, Annotation(defn.RefineOverrideAnnot, util.Spans.NoSpan))) // Yichen you might want to check this
846+
refined = refined.refinedOverride(getterName, argType.unboxed) // Yichen you might want to check this
848847
allCaptures ++= argType.captureSet
849848
(refined, allCaptures)
850849

@@ -1507,9 +1506,7 @@ class CheckCaptures extends Recheck, SymTransformer:
15071506
val cs = actual.captureSet
15081507
if covariant then cs ++ leaked
15091508
else
1510-
if // CCState.withCapAsRoot: // Not sure withCapAsRoot is OK here, actually
1511-
!leaked.subCaptures(cs).isOK
1512-
then
1509+
if !leaked.subCaptures(cs).isOK then
15131510
report.error(
15141511
em"""$expected cannot be box-converted to ${actual.capturing(leaked)}
15151512
|since the additional capture set $leaked resulting from box conversion is not allowed in $actual""", tree.srcPos)
@@ -1708,7 +1705,8 @@ class CheckCaptures extends Recheck, SymTransformer:
17081705
def traverse(t: Tree)(using Context) =
17091706
t match
17101707
case t: Template =>
1711-
checkAllOverrides(ctx.owner.asClass, OverridingPairsCheckerCC(_, _, t))
1708+
ignoringFreshLevels:
1709+
checkAllOverrides(ctx.owner.asClass, OverridingPairsCheckerCC(_, _, t))
17121710
case _ =>
17131711
traverseChildren(t)
17141712
end checkOverrides
@@ -1912,7 +1910,8 @@ class CheckCaptures extends Recheck, SymTransformer:
19121910
arg.withType(arg.nuType.forceBoxStatus(
19131911
bounds.hi.isBoxedCapturing | bounds.lo.isBoxedCapturing))
19141912
CCState.withCapAsRoot: // OK? We need this since bounds use `cap` instead of `fresh`
1915-
checkBounds(normArgs, tl)
1913+
CCState.ignoringFreshLevels:
1914+
checkBounds(normArgs, tl)
19161915
if ccConfig.postCheckCapturesets then
19171916
args.lazyZip(tl.paramNames).foreach(checkTypeParam(_, _, fun.symbol))
19181917
case _ =>
@@ -1932,7 +1931,8 @@ class CheckCaptures extends Recheck, SymTransformer:
19321931
case tree: New =>
19331932
case tree: TypeTree =>
19341933
CCState.withCapAsRoot:
1935-
checkAppliedTypesIn(tree.withType(tree.nuType))
1934+
CCState.ignoringFreshLevels:
1935+
checkAppliedTypesIn(tree.withType(tree.nuType))
19361936
case _ => traverseChildren(t)
19371937
checkApplied.traverse(unit)
19381938
end postCheck

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -915,7 +915,11 @@ class SepCheck(checker: CheckCaptures.CheckerAPI) extends tpd.TreeTraverser:
915915
traverseChildren(tree)
916916
checkValOrDefDef(tree)
917917
case tree: DefDef =>
918-
inSection:
918+
if tree.symbol.isInlineMethod then
919+
// We currently skip inline method since these seem to generate
920+
// spurious recheck completions. Test case is i20237.scala
921+
capt.println(i"skipping sep check of inline def ${tree.symbol}")
922+
else inSection:
919923
consumed.segment:
920924
for params <- tree.paramss; case param: ValDef <- params do
921925
pushDef(param, emptyRefs)

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

+3-5
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,11 @@ object Synthetics:
7777
case tp: MethodOrPoly =>
7878
tp.derivedLambdaType(resType = augmentResult(tp.resType))
7979
case _ =>
80-
val refined = trackedParams.foldLeft(tp) { (parent, pref) =>
81-
RefinedType(parent, pref.paramName,
80+
val refined = trackedParams.foldLeft(tp): (parent, pref) =>
81+
parent.refinedOverride(pref.paramName,
8282
CapturingType(
8383
atPhase(ctx.phase.next)(pref.underlying.stripCapturing),
8484
CaptureSet(pref)))
85-
}
8685
CapturingType(refined, CaptureSet(trackedParams*))
8786
if trackedParams.isEmpty then info
8887
else augmentResult(info).showing(i"augment apply/copy type $info to $result", capt)
@@ -116,8 +115,7 @@ object Synthetics:
116115
def transformUnapplyCaptures(info: Type)(using Context): Type = info match
117116
case info: MethodType =>
118117
val paramInfo :: Nil = info.paramInfos: @unchecked
119-
val newParamInfo = CapturingType(paramInfo,
120-
CaptureSet.fresh(root.Origin.UnapplyInstance(info)))
118+
val newParamInfo = CapturingType(paramInfo, CaptureSet.universal)
121119
val trackedParam = info.paramRefs.head
122120
def newResult(tp: Type): Type = tp match
123121
case tp: MethodOrPoly =>

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ object root:
155155
case Annot(kind) => this.kind eq kind
156156
case _ => false
157157

158-
/** Special treatment of `SubstBindingMaps` which can change the binder of a
158+
/** Special treatment of `SubstBindingMaps` which can change the binder of
159159
* Result instances
160160
*/
161161
override def mapWith(tm: TypeMap)(using Context) = kind match

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

+13-4
Original file line numberDiff line numberDiff line change
@@ -868,10 +868,19 @@ object Types extends TypeUtils {
868868
pdenot.asSingleDenotation.derivedSingleDenotation(pdenot.symbol, overridingRefinement)
869869
else
870870
val isRefinedMethod = rinfo.isInstanceOf[MethodOrPoly]
871-
val joint = pdenot.meet(
872-
new JointRefDenotation(NoSymbol, rinfo, Period.allInRun(ctx.runId), pre, isRefinedMethod),
873-
pre,
874-
safeIntersection = ctx.base.pendingMemberSearches.contains(name))
871+
val joint = try
872+
CCState.ignoringFreshLevels:
873+
pdenot.meet(
874+
new JointRefDenotation(NoSymbol, rinfo, Period.allInRun(ctx.runId), pre, isRefinedMethod),
875+
pre,
876+
safeIntersection = ctx.base.pendingMemberSearches.contains(name))
877+
pdenot.meet(
878+
new JointRefDenotation(NoSymbol, rinfo, Period.allInRun(ctx.runId), pre, isRefinedMethod),
879+
pre,
880+
safeIntersection = ctx.base.pendingMemberSearches.contains(name))
881+
catch case ex: AssertionError =>
882+
println(i"error while do refined $tp . $name, ${pdenot.info} / $rinfo")
883+
throw ex
875884
joint match
876885
case joint: SingleDenotation
877886
if isRefinedMethod

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,6 @@ object OverridingPairs:
226226
else
227227
member.name.is(DefaultGetterName) // default getters are not checked for compatibility
228228
||
229-
CCState.withTreatFreshAsEqual:
230-
memberTp.overrides(otherTp, member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack)
229+
memberTp.overrides(otherTp, member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack)
231230

232231
end OverridingPairs

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

+18-1
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,27 @@
11
-- Error: tests/neg-custom-args/captures/filevar.scala:8:6 -------------------------------------------------------------
2-
8 | var file: File^ = uninitialized // error, was OK under unsealed
2+
8 | var file: File^ = uninitialized // error, under sealed
33
| ^
44
| Mutable variable file cannot have type File^ since
55
| that type captures the root capability `cap`.
66
|
77
| where: ^ refers to a fresh root capability in the type of variable file
8+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/filevar.scala:15:12 --------------------------------------
9+
15 | withFile: f => // error with level checking, was OK under both schemes before
10+
| ^
11+
|Found: (l: scala.caps.Capability^{cap.rd}) ?->? File^? ->? Unit
12+
|Required: (l: scala.caps.Capability^{cap.rd}) ?-> (f: File^{l}) => Unit
13+
|
14+
|where: => refers to a root capability associated with the result type of (using l: scala.caps.Capability^{cap.rd}): (f: File^{l}) => Unit
15+
| cap is the universal root capability
16+
|
17+
|
18+
|Note that reference l.type
19+
|cannot be included in outer capture set ? of parameter f
20+
16 | val o = Service()
21+
17 | o.file = f
22+
18 | o.log
23+
|
24+
| longer explanation available when compiling with `-explain`
825
-- Warning: tests/neg-custom-args/captures/filevar.scala:11:55 ---------------------------------------------------------
926
11 |def withFile[T](op: (l: caps.Capability) ?-> (f: File^{l}) => T): T =
1027
| ^

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ class File:
55
def write(x: String): Unit = ???
66

77
class Service:
8-
var file: File^ = uninitialized // error, was OK under unsealed
8+
var file: File^ = uninitialized // error, under sealed
99
def log = file.write("log") // OK, was error under unsealed
1010

1111
def withFile[T](op: (l: caps.Capability) ?-> (f: File^{l}) => T): T =
1212
op(using caps.cap)(new File)
1313

1414
def test =
15-
withFile: f =>
15+
withFile: f => // error with level checking, was OK under both schemes before
1616
val o = Service()
1717
o.file = f
1818
o.log

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

+10-11
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,16 @@
7272
| ^² refers to a fresh root capability in the type of value id
7373
|
7474
| longer explanation available when compiling with `-explain`
75-
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/reaches.scala:63:27 --------------------------------------
76-
63 | val f1: File^{id*} = id(f) // error // error
77-
| ^^^^^
78-
| Found: File^
79-
| Required: File^{id*}
75+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/reaches.scala:62:38 --------------------------------------
76+
62 | val leaked = usingFile[File^{id*}]: f => // error // error
77+
| ^
78+
|Found: (f: File^?) ->? box File^?
79+
|Required: (f: File^) => box File^{id*}
8080
|
81-
| where: ^ refers to a fresh root capability in the type of value id
81+
|where: => refers to a fresh root capability created in value leaked when checking argument to parameter f of method usingFile
82+
| ^ refers to the universal root capability
83+
63 | val f1: File^{id*} = id(f)
84+
64 | f1
8285
|
8386
| longer explanation available when compiling with `-explain`
8487
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/reaches.scala:67:32 --------------------------------------
@@ -123,13 +126,9 @@
123126
|
124127
| longer explanation available when compiling with `-explain`
125128
-- Error: tests/neg-custom-args/captures/reaches.scala:62:31 -----------------------------------------------------------
126-
62 | val leaked = usingFile[File^{id*}]: f => // error
129+
62 | val leaked = usingFile[File^{id*}]: f => // error // error
127130
| ^^^
128131
| id* cannot be tracked since its deep capture set is empty
129-
-- Error: tests/neg-custom-args/captures/reaches.scala:63:18 -----------------------------------------------------------
130-
63 | val f1: File^{id*} = id(f) // error // error
131-
| ^^^
132-
| id* cannot be tracked since its deep capture set is empty
133132
-- Error: tests/neg-custom-args/captures/reaches.scala:70:31 -----------------------------------------------------------
134133
70 | val leaked = usingFile[File^{id*}]: f => // error // error
135134
| ^^^

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ def attack2 =
5959
val id: File^ -> File^ = x => x // error
6060
// val id: File^ -> File^{fresh}
6161

62-
val leaked = usingFile[File^{id*}]: f => // error
63-
val f1: File^{id*} = id(f) // error // error
62+
val leaked = usingFile[File^{id*}]: f => // error // error
63+
val f1: File^{id*} = id(f)
6464
f1
6565

6666
def attack3 =

tests/neg-custom-args/captures/scoped-caps.check

+3-15
Original file line numberDiff line numberDiff line change
@@ -98,23 +98,11 @@
9898
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/scoped-caps.scala:26:20 ----------------------------------
9999
26 | val _: A^ => B^ = x => g2(x) // error: g2(x): B^{g2, x}, and the `x` cannot be subsumed by fresh
100100
| ^^^^^^^^^^
101-
| Found: (x: A^) ->{g2} B^²
102-
| Required: (x: A^) => B^³
101+
| Found: (x: A^) ->{g2} B^{g2, x}
102+
| Required: (x: A^) => B^²
103103
|
104104
| where: => refers to a fresh root capability in the type of value _$13
105105
| ^ refers to the universal root capability
106-
| ^² refers to a root capability associated with the result type of (x: A^): B^²
107-
| ^³ refers to a fresh root capability in the type of value _$13
108-
|
109-
| longer explanation available when compiling with `-explain`
110-
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/scoped-caps.scala:28:24 ----------------------------------
111-
28 | val _: A^{io} => B^ = x => g3(x) // error, fails level checking of fresh instances
112-
| ^^^^^^^^^^
113-
| Found: (x: A^{io}) ->{g3} B^
114-
| Required: (x: A^{io}) => B^²
115-
|
116-
| where: => refers to a fresh root capability in the type of value _$14
117-
| ^ refers to a root capability associated with the result type of (x: A^{io}): B^
118-
| ^² refers to a fresh root capability in the type of value _$14
106+
| ^² refers to a fresh root capability in the type of value _$13
119107
|
120108
| longer explanation available when compiling with `-explain`

tests/neg-custom-args/captures/scoped-caps.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,4 @@ def test(io: Object^): Unit =
2525
val g2: A^ => B^ = ???
2626
val _: A^ => B^ = x => g2(x) // error: g2(x): B^{g2, x}, and the `x` cannot be subsumed by fresh
2727
val g3: A^ => B^ = ???
28-
val _: A^{io} => B^ = x => g3(x) // error, fails level checking of fresh instances
28+
val _: A^{io} => B^ = x => g3(x) // ok, now g3(x): B^{g3, x}, which is widened to B^{g3, io}

0 commit comments

Comments
 (0)