Skip to content

Commit 0ad5aa0

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 bbe3650 commit 0ad5aa0

File tree

10 files changed

+286
-46
lines changed

10 files changed

+286
-46
lines changed

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -375,10 +375,14 @@ object Capabilities:
375375
case tp: SetCapability => tp.captureSetOfInfo.isReadOnly
376376
case _ => this ne stripReadOnly
377377

378-
/** The restriction of this capability or NoSymbol if there is none */
379-
final def restriction(using Context): Symbol = this match
378+
/** The classifier, either given in an explicit `.only` or assumed for a
379+
* FreshCap. AnyRef for unclassified FreshCaps. Otherwise NoSymbol if no
380+
* classifier is given.
381+
*/
382+
final def classifier(using Context): Symbol = this match
380383
case Restricted(_, cls) => cls
381-
case Maybe(ref1) => ref1.restriction
384+
case Maybe(ref1) => ref1.classifier
385+
case self: FreshCap => self.hiddenSet.classifier
382386
case _ => NoSymbol
383387

384388
/** Is this a reach reference of the form `x*` or a readOnly or maybe variant
@@ -609,7 +613,7 @@ object Capabilities:
609613
case Reach(_) =>
610614
captureSetOfInfo.transClassifiers
611615
case self: CoreCapability =>
612-
if self.derivesFromCapability then toClassifiers(self.classifier)
616+
if self.derivesFromCapability then toClassifiers(self.inheritedClassifier)
613617
else captureSetOfInfo.transClassifiers
614618
if myClassifiers != UnknownClassifier then
615619
classifiersValid == currentId

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

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

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

494494
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: 51 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,17 @@ class CheckCaptures extends Recheck, SymTransformer:
945945
.showing(i"constr type $mt with $argTypes%, % in $constr = $result", capt)
946946
end refineConstructorInstance
947947

948+
/** If `mbr` is a field that has (possibly restricted) FreshCaps in its span capture set,
949+
* their classifiers, otherwise the empty list.
950+
*/
951+
private def classifiersOfFreshInType(mbr: Symbol)(using Context): List[ClassSymbol] =
952+
if contributesFreshToClass(mbr) then
953+
mbr.info.spanCaptureSet.elems
954+
.filter(_.isTerminalCapability)
955+
.toList
956+
.map(_.classifier.asClass)
957+
else Nil
958+
948959
/** The additional capture set implied by the capture sets of its fields. This
949960
* is either empty or, if some fields have a terminal capability in their span
950961
* capture sets, it consists of a single fresh cap that subsumes all these terminal
@@ -962,16 +973,9 @@ class CheckCaptures extends Recheck, SymTransformer:
962973
*/
963974
def impliedClassifiers(cls: Symbol): List[ClassSymbol] = cls match
964975
case cls: ClassSymbol =>
965-
var fieldClassifiers =
966-
for
967-
sym <- setup.fieldsWithExplicitTypes.getOrElse(cls, cls.info.decls.toList)
968-
if contributesFreshToClass(sym)
969-
case fresh: FreshCap <- sym.info.spanCaptureSet.elems
970-
.filter(_.isTerminalCapability)
971-
.map(_.stripReadOnly)
972-
.toList
973-
_ = pushInfo(i"Note: ${sym.showLocated} captures a $fresh")
974-
yield fresh.hiddenSet.classifier
976+
var fieldClassifiers = setup.fieldsWithExplicitTypes // pick fields with explicit types for classes in this compilation unit
977+
.getOrElse(cls, cls.info.decls.toList) // pick all symbols in class scope for other classes
978+
.flatMap(classifiersOfFreshInType)
975979
if cls.typeRef.isMutableType then
976980
fieldClassifiers = defn.Caps_Mutable :: fieldClassifiers
977981
val parentClassifiers =
@@ -1225,11 +1229,20 @@ class CheckCaptures extends Recheck, SymTransformer:
12251229
curEnv = saved
12261230
end recheckDefDef
12271231

1228-
/** If val or def definition with inferred (result) type is visible
1229-
* in other compilation units, check that the actual inferred type
1230-
* conforms to the expected type where all inferred capture sets are dropped.
1231-
* This ensures that if files compile separately, they will also compile
1232-
* in a joint compilation.
1232+
/** Two tests for member definitions with inferred types:
1233+
*
1234+
* 1. If val or def definition with inferred (result) type is visible
1235+
* in other compilation units, check that the actual inferred type
1236+
* conforms to the expected type where all inferred capture sets are dropped.
1237+
* This ensures that if files compile separately, they will also compile
1238+
* in a joint compilation.
1239+
* 2. If a val has an inferred type with a terminal capability in its span capset,
1240+
* check that it this capability is subsumed by the capset that was inferred
1241+
* for the class from its other fields via `captureSetImpliedByFields`.
1242+
* That capset is defined to take into account all fields but is computed
1243+
* only from fields with explicitly given types in order to avoid cycles.
1244+
* See comment on Setup.fieldsWithExplicitTypes. So we have to make sure
1245+
* that fields with inferred types would not change that capset.
12331246
*/
12341247
def checkInferredResult(tp: Type, tree: ValOrDefDef)(using Context): Type =
12351248
val sym = tree.symbol
@@ -1264,11 +1277,17 @@ class CheckCaptures extends Recheck, SymTransformer:
12641277
|The new inferred type $tp
12651278
|must conform to this type."""
12661279

1280+
def covers(classCapset: CaptureSet, fieldClassifiers: List[ClassSymbol]): Boolean =
1281+
fieldClassifiers.forall: cls =>
1282+
classCapset.elems.exists:
1283+
case fresh: FreshCap => cls.isSubClass(fresh.hiddenSet.classifier)
1284+
case _ => false
1285+
12671286
tree.tpt match
1268-
case tpt: InferredTypeTree if !isExemptFromChecks =>
1269-
if !sym.isLocalToCompilationUnit
1270-
// Symbols that can't be seen outside the compilation unit can have inferred types
1271-
// except for the else clause below.
1287+
case tpt: InferredTypeTree =>
1288+
// Test point (1) of doc comment above
1289+
if !sym.isLocalToCompilationUnit && !isExemptFromChecks
1290+
// Symbols that can't be seen outside the compilation unit can have inferred types
12721291
then
12731292
val expected = tpt.tpe.dropAllRetains
12741293
todoAtPostCheck += { () =>
@@ -1277,18 +1296,21 @@ class CheckCaptures extends Recheck, SymTransformer:
12771296
// The check that inferred <: expected is done after recheck so that it
12781297
// does not interfere with normal rechecking by constraining capture set variables.
12791298
}
1280-
else if sym.is(Private)
1281-
&& !sym.isLocalToCompilationUnitIgnoringPrivate
1282-
&& tree.tpt.nuType.spanCaptureSet.containsTerminalCapability
1299+
// Test point (2) of doc comment above
1300+
if sym.owner.isClass && !sym.owner.isStaticOwner
12831301
&& contributesFreshToClass(sym)
1284-
// Private symbols capturing a root capability need explicit types
1285-
// so that we can compute field constributions to class instance
1286-
// capture sets across compilation units.
12871302
then
1288-
report.error(
1289-
em"""$sym needs an explicit type because it captures a root capability in its type ${tree.tpt.nuType}.
1290-
|Fields of publicily accessible classes that capture a root capability need to be given an explicit type.""",
1291-
tpt.srcPos)
1303+
todoAtPostCheck += { () =>
1304+
val cls = sym.owner.asClass
1305+
val fieldClassifiers = classifiersOfFreshInType(sym)
1306+
val classCapset = captureSetImpliedByFields(cls, cls.appliedRef)
1307+
if !covers(classCapset, fieldClassifiers) then
1308+
report.error(
1309+
em"""$sym needs an explicit type because it captures a root capability in its type ${tree.tpt.nuType}.
1310+
|Fields capturing a root capability need to be given an explicit type unless the capability is already
1311+
|subsumed by the computed capability of the enclosing class.""",
1312+
tpt.srcPos)
1313+
}
12921314
case _ =>
12931315
tp
12941316
end checkInferredResult

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.rd}.
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

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,13 @@ class C(val io: IO):
66
val l1 = () => c.io.write()
77
val _: () -> Unit = l1 // error
88

9+
class Ref extends caps.Mutable:
10+
var x: Int = 0
11+
12+
class D:
13+
val r = Ref() // error
14+
15+
def test =
16+
val d = D()
17+
val _: D^{} = d
918

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazyvals6.scala:7:22 -------------------------------------
2+
7 | val _: () -> Unit = x // error
3+
| ^
4+
| Found: (C.this.x : () ->{C.this.io} Unit)
5+
| Required: () -> Unit
6+
|
7+
| Note that capability C.this.io is not included in capture set {}.
8+
|
9+
| longer explanation available when compiling with `-explain`
10+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazyvals6.scala:12:22 ------------------------------------
11+
12 | val _: () -> Unit = l1 // error
12+
| ^^
13+
| Found: (C.this.l1 : () ->{C.this.c.io} Unit)
14+
| Required: () -> Unit
15+
|
16+
| Note that capability C.this.c.io is not included in capture set {}.
17+
|
18+
| longer explanation available when compiling with `-explain`
19+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazyvals6.scala:16:22 ------------------------------------
20+
16 | val _: () -> Unit = l2 // error
21+
| ^^
22+
| Found: (C.this.l2 : () ->{C.this.io} Unit)
23+
| Required: () -> Unit
24+
|
25+
| Note that capability C.this.io is not included in capture set {}.
26+
|
27+
| longer explanation available when compiling with `-explain`
28+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazyvals6.scala:20:22 ------------------------------------
29+
20 | val _: () -> Unit = s1 // error
30+
| ^^
31+
| Found: (C.this.s1 : () ->{C.this.c.io} Unit)
32+
| Required: () -> Unit
33+
|
34+
| Note that capability C.this.c.io is not included in capture set {}.
35+
|
36+
| longer explanation available when compiling with `-explain`
37+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazyvals6.scala:24:22 ------------------------------------
38+
24 | val _: () -> Unit = s2 // error
39+
| ^^
40+
| Found: (C.this.s2 : () ->{C.this.io} Unit)
41+
| Required: () -> Unit
42+
|
43+
| Note that capability C.this.io is not included in capture set {}.
44+
|
45+
| longer explanation available when compiling with `-explain`
46+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazyvals6.scala:32:22 ------------------------------------
47+
32 | val _: () -> Unit = l11 // error
48+
| ^^^
49+
| Found: (C.this.l11 : () ->{C.this} Unit)
50+
| Required: () -> Unit
51+
|
52+
| Note that capability C.this is not included in capture set {}.
53+
|
54+
| longer explanation available when compiling with `-explain`
55+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazyvals6.scala:36:22 ------------------------------------
56+
36 | val _: () -> Unit = l12 // error
57+
| ^^^
58+
| Found: (C.this.l12 : () ->{C.this.io} Unit)
59+
| Required: () -> Unit
60+
|
61+
| Note that capability C.this.io is not included in capture set {}.
62+
|
63+
| longer explanation available when compiling with `-explain`
64+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazyvals6.scala:41:22 ------------------------------------
65+
41 | val _: () -> Unit = s11 // error
66+
| ^^^
67+
| Found: (C.this.s11 : () ->{C.this} Unit)
68+
| Required: () -> Unit
69+
|
70+
| Note that capability C.this is not included in capture set {}.
71+
|
72+
| longer explanation available when compiling with `-explain`
73+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazyvals6.scala:45:22 ------------------------------------
74+
45 | val _: () -> Unit = s12 // error
75+
| ^^^
76+
| Found: (C.this.s12 : () ->{C.this.io} Unit)
77+
| Required: () -> Unit
78+
|
79+
| Note that capability C.this.io is not included in capture set {}.
80+
|
81+
| longer explanation available when compiling with `-explain`
82+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazyvals6.scala:50:22 ------------------------------------
83+
50 | val _: () -> Unit = x // error
84+
| ^
85+
| Found: (x : () ->{io} Unit)
86+
| Required: () -> Unit
87+
|
88+
| Note that capability io is not included in capture set {}.
89+
|
90+
| longer explanation available when compiling with `-explain`
91+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazyvals6.scala:55:22 ------------------------------------
92+
55 | val _: () -> Unit = z // error
93+
| ^
94+
| Found: (z : () ->{c.io} Unit)
95+
| Required: () -> Unit
96+
|
97+
| Note that capability c.io is not included in capture set {}.
98+
|
99+
| longer explanation available when compiling with `-explain`

0 commit comments

Comments
 (0)