Skip to content

Commit 8e5a171

Browse files
committed
Generalize inferred types check
We now need to also check that fields with inferred types would not contribute a fresh capability to the implied capture set of the class.
1 parent 774eaa1 commit 8e5a171

File tree

7 files changed

+115
-38
lines changed

7 files changed

+115
-38
lines changed

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -378,10 +378,15 @@ object Capabilities:
378378
case tp: SetCapability => tp.captureSetOfInfo.isReadOnly
379379
case _ => this ne stripReadOnly
380380

381-
final def restriction(using Context): Symbol = this match
381+
/** The classifier, either given in an explicit `.only` or assumed for a
382+
* FreshCap. AnyRef for unclassified FreshCaps. Otherwise NoSymbol if no
383+
* classifier is given.
384+
*/
385+
final def classifier(using Context): Symbol = this match
382386
case Restricted(_, cls) => cls
383-
case ReadOnly(ref1) => ref1.restriction
384-
case Maybe(ref1) => ref1.restriction
387+
case ReadOnly(ref1) => ref1.classifier
388+
case Maybe(ref1) => ref1.classifier
389+
case self: FreshCap => self.hiddenSet.classifier
385390
case _ => NoSymbol
386391

387392
/** Is this a reach reference of the form `x*` or a readOnly or maybe variant
@@ -617,7 +622,7 @@ object Capabilities:
617622
case Reach(_) =>
618623
captureSetOfInfo.transClassifiers
619624
case self: CoreCapability =>
620-
if self.derivesFromCapability then toClassifiers(self.classifier)
625+
if self.derivesFromCapability then toClassifiers(self.inheritedClassifier)
621626
else captureSetOfInfo.transClassifiers
622627
if myClassifiers != UnknownClassifier then
623628
classifiersValid == currentId

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ extension (tp: Type)
487487
case _ =>
488488
tp
489489

490-
def classifier(using Context): ClassSymbol =
490+
def inheritedClassifier(using Context): ClassSymbol =
491491
tp.classSymbols.map(_.classifier).foldLeft(defn.AnyClass)(leastClassifier)
492492

493493
extension (tp: MethodType)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ object CapturingType:
4040
apply(parent1, refs ++ refs1, boxed)
4141
case _ =>
4242
if parent.derivesFromMutable then refs.associateWithMutable()
43-
refs.adoptClassifier(parent.classifier)
43+
refs.adoptClassifier(parent.inheritedClassifier)
4444
AnnotatedType(parent, CaptureAnnotation(refs, boxed)(defn.RetainsAnnot))
4545

4646
/** An extractor for CapturingTypes. Capturing types are recognized if

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

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -929,6 +929,17 @@ class CheckCaptures extends Recheck, SymTransformer:
929929
.showing(i"constr type $mt with $argTypes%, % in $constr = $result", capt)
930930
end refineConstructorInstance
931931

932+
/** If `mbr` is a field that has (possibly restricted) FreshCaps in its span capture set,
933+
* their classifiers, otherwise the empty list.
934+
*/
935+
private def classifiersOfFreshInType(mbr: Symbol)(using Context): List[ClassSymbol] =
936+
if contributesFreshToClass(mbr) then
937+
mbr.info.spanCaptureSet.elems
938+
.filter(_.isTerminalCapability)
939+
.toList
940+
.map(_.classifier.asClass)
941+
else Nil
942+
932943
/** The additional capture set implied by the capture sets of its fields. This
933944
* is either empty or, if some fields have a terminal capability in their span
934945
* capture sets, it consists of a single fresh cap that subsumes all these terminal
@@ -946,16 +957,9 @@ class CheckCaptures extends Recheck, SymTransformer:
946957
*/
947958
def impliedClassifiers(cls: Symbol): List[ClassSymbol] = cls match
948959
case cls: ClassSymbol =>
949-
var fieldClassifiers =
950-
for
951-
sym <- setup.fieldsWithExplicitTypes.getOrElse(cls, cls.info.decls.toList)
952-
if contributesFreshToClass(sym)
953-
case fresh: FreshCap <- sym.info.spanCaptureSet.elems
954-
.filter(_.isTerminalCapability)
955-
.map(_.stripReadOnly)
956-
.toList
957-
_ = pushInfo(i"Note: ${sym.showLocated} captures a $fresh")
958-
yield fresh.hiddenSet.classifier
960+
var fieldClassifiers = setup.fieldsWithExplicitTypes // pick fields with explicit types for classes in this compilation unit
961+
.getOrElse(cls, cls.info.decls.toList) // pick all symbols in class scope for other classes
962+
.flatMap(classifiersOfFreshInType)
959963
if cls.typeRef.isMutableType then
960964
fieldClassifiers = defn.Caps_Mutable :: fieldClassifiers
961965
val parentClassifiers =
@@ -1199,11 +1203,20 @@ class CheckCaptures extends Recheck, SymTransformer:
11991203
curEnv = saved
12001204
end recheckDefDef
12011205

1202-
/** If val or def definition with inferred (result) type is visible
1203-
* in other compilation units, check that the actual inferred type
1204-
* conforms to the expected type where all inferred capture sets are dropped.
1205-
* This ensures that if files compile separately, they will also compile
1206-
* in a joint compilation.
1206+
/** Two tests for member definitions with inferred types:
1207+
*
1208+
* 1. If val or def definition with inferred (result) type is visible
1209+
* in other compilation units, check that the actual inferred type
1210+
* conforms to the expected type where all inferred capture sets are dropped.
1211+
* This ensures that if files compile separately, they will also compile
1212+
* in a joint compilation.
1213+
* 2. If a val has an inferred type with a terminal capability in its span capset,
1214+
* check that it this capability is subsumed by the capset that was inferred
1215+
* for the class from its other fields via `captureSetImpliedByFields`.
1216+
* That capset is defined to take into account all fields but is computed
1217+
* only from fields with explicitly given types in order to avoid cycles.
1218+
* See comment on Setup.fieldsWithExplicitTypes. So we have to make sure
1219+
* that fields with inferred types would not change that capset.
12071220
*/
12081221
def checkInferredResult(tp: Type, tree: ValOrDefDef)(using Context): Type =
12091222
val sym = tree.symbol
@@ -1238,8 +1251,15 @@ class CheckCaptures extends Recheck, SymTransformer:
12381251
|The new inferred type $tp
12391252
|must conform to this type."""
12401253

1254+
def covers(classCapset: CaptureSet, fieldClassifiers: List[ClassSymbol]): Boolean =
1255+
fieldClassifiers.forall: cls =>
1256+
classCapset.elems.exists:
1257+
case fresh: FreshCap => cls.isSubClass(fresh.hiddenSet.classifier)
1258+
case _ => false
1259+
12411260
tree.tpt match
12421261
case tpt: InferredTypeTree =>
1262+
// Test point (1) of doc comment above
12431263
if !sym.isLocalToCompilationUnit && !isExemptFromChecks
12441264
// Symbols that can't be seen outside the compilation unit can have inferred types
12451265
then
@@ -1250,17 +1270,19 @@ class CheckCaptures extends Recheck, SymTransformer:
12501270
// The check that inferred <: expected is done after recheck so that it
12511271
// does not interfere with normal rechecking by constraining capture set variables.
12521272
}
1253-
if !sym.isLocalToCompilationUnitIgnoringPrivate
1273+
// Test point (2) of doc comment above
1274+
if sym.owner.isClass && !sym.owner.isStaticOwner
12541275
&& contributesFreshToClass(sym)
1255-
// Private symbols capturing a root capability need explicit types
1256-
// so that we can compute field constributions to class instance
1257-
// capture sets across compilation units.
12581276
then
12591277
todoAtPostCheck += { () =>
1260-
if sym.info.spanCaptureSet.containsTerminalCapability then
1278+
val cls = sym.owner.asClass
1279+
val fieldClassifiers = classifiersOfFreshInType(sym)
1280+
val classCapset = captureSetImpliedByFields(cls, cls.appliedRef)
1281+
if !covers(classCapset, fieldClassifiers) then
12611282
report.error(
12621283
em"""$sym needs an explicit type because it captures a root capability in its type ${tree.tpt.nuType}.
1263-
|Fields of publicily accessible classes that capture a root capability need to be given an explicit type.""",
1284+
|Fields capturing a root capability need to be given an explicit type unless the capability is already
1285+
|subsumed by the computed capability of the enclosing class.""",
12641286
tpt.srcPos)
12651287
}
12661288
case _ =>

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

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,3 @@
1-
-- Error: tests/neg-custom-args/captures/check-inferred.scala:12:19 ----------------------------------------------------
2-
12 | private val count = Ref() // error
3-
| ^
4-
| value count needs an explicit type because it captures a root capability in its type test.Ref^.
5-
| Fields of publicily accessible classes that capture a root capability need to be given an explicit type.
6-
|
7-
| where: ^ refers to a fresh root capability classified as Mutable in the type of value count
81
-- Error: tests/neg-custom-args/captures/check-inferred.scala:18:13 ----------------------------------------------------
92
18 | val incr = () => // error
103
| ^
@@ -24,7 +17,7 @@
2417
| Externally visible type: () -> Unit
2518
21 | count.put(count.get - 1)
2619
-- Error: tests/neg-custom-args/captures/check-inferred.scala:24:14 ----------------------------------------------------
27-
24 | val count = Ref(): Object^ // error
20+
24 | val count = Ref(): Object^ // error // error
2821
| ^^^^^^^^^^^^^^
2922
| value count needs an explicit type because the inferred type does not conform to
3023
| the type that is externally visible in other compilation units.
@@ -33,3 +26,36 @@
3326
| Externally visible type: Object
3427
|
3528
| where: ^ refers to a fresh root capability created in value count
29+
-- Error: tests/neg-custom-args/captures/check-inferred.scala:24:11 ----------------------------------------------------
30+
24 | val count = Ref(): Object^ // error // error
31+
| ^
32+
| value count needs an explicit type because it captures a root capability in its type Object^.
33+
| Fields capturing a root capability need to be given an explicit type unless the capability is already
34+
| subsumed by the computed capability of the enclosing class.
35+
|
36+
| where: ^ refers to a fresh root capability in the type of value count
37+
-- Error: tests/neg-custom-args/captures/check-inferred.scala:45:15 ----------------------------------------------------
38+
45 | private val y = ??? : A^ // error
39+
| ^
40+
| value y needs an explicit type because it captures a root capability in its type test.A^.
41+
| Fields capturing a root capability need to be given an explicit type unless the capability is already
42+
| subsumed by the computed capability of the enclosing class.
43+
|
44+
| where: ^ refers to a fresh root capability in the type of value y
45+
-- Error: tests/neg-custom-args/captures/check-inferred.scala:49:15 ----------------------------------------------------
46+
49 | private val y = ??? : (() => A^{cap.only[caps.Read]}) // error
47+
| ^
48+
| value y needs an explicit type because it captures a root capability in its type () => test.A^{cap.only[Read]}.
49+
| Fields capturing a root capability need to be given an explicit type unless the capability is already
50+
| subsumed by the computed capability of the enclosing class.
51+
|
52+
| where: => refers to a fresh root capability in the type of value y
53+
| cap is a fresh root capability created in value y
54+
-- Error: tests/neg-custom-args/captures/check-inferred.scala:29:21 ----------------------------------------------------
55+
29 | private val count = Ref() // error
56+
| ^
57+
| value count needs an explicit type because it captures a root capability in its type test.Ref^.
58+
| Fields capturing a root capability need to be given an explicit type unless the capability is already
59+
| subsumed by the computed capability of the enclosing class.
60+
|
61+
| where: ^ refers to a fresh root capability classified as Mutable in the type of value count

tests/neg-custom-args/captures/check-inferred.scala

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class Ref extends Mutable:
99
update def put(y: Int): Unit = x = y
1010

1111
class Counter:
12-
private val count = Ref() // error
12+
private val count = Ref()
1313
private val altCount: Ref^ = Ref() // ok
1414

1515
@untrackedCaptures
@@ -21,14 +21,30 @@ class Counter:
2121
count.put(count.get - 1)
2222

2323
trait CounterAPI:
24-
val count = Ref(): Object^ // error
24+
val count = Ref(): Object^ // error // error
2525
private def count2 = Ref() // ok
2626

2727
def test() =
2828
class Counter:
29-
private val count = Ref() // ok
29+
private val count = Ref() // error
3030
val incr = () =>
3131
count.put(count.get + 1)
3232
val decr = () =>
3333
count.put(count.get - 1)
3434

35+
class A:
36+
val x: A^{cap.only[caps.Control]} = ???
37+
private val y = ??? : A^{cap.only[caps.Control]} // ok
38+
39+
class B:
40+
val x: A^ = ???
41+
private val y = ??? : A^{cap.only[caps.Control]} // ok
42+
43+
class C:
44+
val x: A^{cap.only[caps.Control]} = ???
45+
private val y = ??? : A^ // error
46+
47+
class D:
48+
val x: A^{cap.only[caps.Control]} = ???
49+
private val y = ??? : (() => A^{cap.only[caps.Read]}) // error
50+

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,11 @@
77
| Note that capability C.this.c.io is not included in capture set {}.
88
|
99
| longer explanation available when compiling with `-explain`
10+
-- Error: tests/neg-custom-args/captures/i24335.scala:13:7 -------------------------------------------------------------
11+
13 | val r = Ref() // error
12+
| ^
13+
| value r needs an explicit type because it captures a root capability in its type Ref^.
14+
| Fields capturing a root capability need to be given an explicit type unless the capability is already
15+
| subsumed by the computed capability of the enclosing class.
16+
|
17+
| where: ^ refers to a fresh root capability classified as Mutable in the type of value r

0 commit comments

Comments
 (0)