Skip to content

Commit 825fc2d

Browse files
committed
Revise separation checks
- Compute dependencies of later arguments to earlier ones - Don't add to initial footprint references implied by a dependency - Don't add to overlap references implied by a dependency - Make error diagnostics better structured and clearer
1 parent 3cf8e37 commit 825fc2d

File tree

17 files changed

+230
-227
lines changed

17 files changed

+230
-227
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,11 @@ sealed abstract class CaptureSet extends Showable:
8080
assert(!isConst)
8181
asInstanceOf[Var]
8282

83+
/** Convert to Const with current elements unconditionally */
84+
def toConst: Const = this match
85+
case c: Const => c
86+
case v: Var => Const(v.elems)
87+
8388
/** Does this capture set contain the root reference `cap` as element? */
8489
final def isUniversal(using Context) =
8590
elems.exists(_.isCap)
@@ -270,6 +275,9 @@ sealed abstract class CaptureSet extends Showable:
270275
else if this.isConst && that.isConst then Const(this.elems ++ that.elems)
271276
else Union(this, that)
272277

278+
def ++ (that: CaptureSet.Const)(using Context): CaptureSet.Const =
279+
Const(this.elems ++ that.elems)
280+
273281
/** The smallest superset (via <:<) of this capture set that also contains `ref`.
274282
*/
275283
def + (ref: CaptureRef)(using Context): CaptureSet =

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ object CheckCaptures:
8989
tp
9090
case _ =>
9191
mapOver(tp)
92+
override def toString = "SubstParamsMap"
9293
end SubstParamsMap
9394

9495
/** Used for substituting parameters in a special case: when all actual arguments
@@ -108,6 +109,7 @@ object CheckCaptures:
108109
tp
109110
case _ =>
110111
mapOver(tp)
112+
override def toString = "SubstParamsBiMap"
111113

112114
lazy val inverse = new BiTypeMap:
113115
def apply(tp: Type): Type = tp match
@@ -124,6 +126,7 @@ object CheckCaptures:
124126
tp
125127
case _ =>
126128
mapOver(tp)
129+
override def toString = "SubstParamsBiMap.inverse"
127130
def inverse = thisMap
128131
end SubstParamsBiMap
129132

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

Lines changed: 83 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ class SepChecker(checker: CheckCaptures.CheckerAPI) extends tpd.TreeTraverser:
1616
import checker.*
1717

1818
extension (cs: CaptureSet)
19-
def footprint(using Context): CaptureSet =
19+
def footprint(using Context): CaptureSet.Const =
2020
def recur(elems: CaptureSet.Refs, newElems: List[CaptureRef]): CaptureSet.Refs = newElems match
2121
case newElem :: newElems1 =>
2222
val superElems = newElem.captureSetOfInfo.elems.filter: superElem =>
2323
!superElem.isMaxCapability && !elems.contains(superElem)
24-
recur(superElems ++ elems, superElems.toList ++ newElems1)
24+
recur(elems ++ superElems, newElems1 ++ superElems.toList)
2525
case Nil => elems
2626
val elems: CaptureSet.Refs = cs.elems.filter(!_.isMaxCapability)
2727
CaptureSet(recur(elems, elems.toList))
@@ -56,12 +56,23 @@ class SepChecker(checker: CheckCaptures.CheckerAPI) extends tpd.TreeTraverser:
5656
recur(cs)
5757
end hidden
5858

59-
def captures(arg: Tree)(using Context): CaptureSet =
59+
/** The captures of an argument or prefix widened to the formal parameter, if
60+
* the latter contains a cap.
61+
*/
62+
def formalCaptures(arg: Tree)(using Context): CaptureSet.Const =
6063
val argType = arg.nuType
61-
if argType.hasUseAnnot then argType.deepCaptureSet else argType.captureSet
64+
(if argType.hasUseAnnot then argType.deepCaptureSet else argType.captureSet)
65+
.toConst
66+
67+
/** The captures of an argument of prefix. No widening takes place */
68+
def actualCaptures(arg: Tree)(using Context): CaptureSet.Const =
69+
val argType = arg.actualType.orElse(arg.nuType)
70+
(if arg.nuType.hasUseAnnot then argType.deepCaptureSet else argType.captureSet)
71+
.toConst
6272

6373
private def sepError(fn: Tree, args: List[Tree], argIdx: Int,
64-
overlap: Refs, hiddenInArg: CaptureSet, footprints: List[(CaptureSet, Int)])(using Context): Unit =
74+
overlap: Refs, hiddenInArg: CaptureSet, footprints: List[(CaptureSet, Int)],
75+
deps: collection.Map[Tree, List[Tree]])(using Context): Unit =
6576
val arg = args(argIdx)
6677
def paramName(mt: Type, idx: Int): Option[Name] = mt match
6778
case mt @ MethodType(pnames) =>
@@ -73,11 +84,11 @@ class SepChecker(checker: CheckCaptures.CheckerAPI) extends tpd.TreeTraverser:
7384
case _ => ""
7485
def whatStr = if overlap.size == 1 then "this capability is" else "these capabilities are"
7586
def funStr =
76-
if fn.symbol.exists then i"${fn.symbol}"
77-
else "the function"
87+
if fn.symbol.exists then i"${fn.symbol}: ${fn.symbol.info}"
88+
else i"a function of type ${fn.nuType.widen}"
7889
val clashIdx = footprints
7990
.collect:
80-
case (fp, idx) if !hiddenInArg.footprint.overlapWith(fp).isEmpty => idx
91+
case (fp, idx) if !hiddenInArg.overlapWith(fp).isEmpty => idx
8192
.head
8293
def whereStr = clashIdx match
8394
case 0 => "function prefix"
@@ -90,62 +101,98 @@ class SepChecker(checker: CheckCaptures.CheckerAPI) extends tpd.TreeTraverser:
90101
else args(clashIdx - 1)
91102
def clashType =
92103
clashTree.actualType.orElse(clashTree.nuType)
93-
def clashCaptures = captures(clashTree)
94-
def clashHidden = CaptureSet(hidden(clashCaptures))
95-
def hiddenCaptures = CaptureSet(hidden(captures(arg)))
96-
def clashFootprint = clashCaptures.footprint ++ clashHidden.footprint
104+
def clashCaptures = actualCaptures(clashTree)
105+
def clashHidden = CaptureSet(hidden(formalCaptures(clashTree)))
106+
def hiddenCaptures = CaptureSet(hidden(formalCaptures(arg)))
107+
def clashFootprint = clashCaptures.footprint
97108
def hiddenFootprint = hiddenCaptures.footprint
109+
def declaredFootprint = CaptureSet(deps(arg).map(actualCaptures(_).elems).foldLeft(emptySet)(_ ++ _)).footprint
110+
def footprintOverlap = CaptureSet(hiddenFootprint.overlapWith(clashFootprint) -- declaredFootprint.elems)
98111
def clashHiddenStr =
99112
if clashTree.needsSepCheck then
100-
i"\n Hidden set of $whereStr : $clashHidden"
113+
i"\n Hidden set of $whereStr : $clashHidden"
101114
else ""
102115
report.error(
103-
em"""Separation failure: argument of type ${arg.actualType} to capture-polymorphic parameter
104-
|${formalName}of type ${arg.nuType} captures ${CaptureSet(overlap)}, and $whatStr also passed
105-
|separately in the ${whereStr.trim} with type $clashType to $funStr.
116+
em"""Separation failure: argument of type ${arg.actualType}
117+
|to $funStr
118+
|corresponds to capture-polymorphic formal parameter ${formalName}of type ${arg.nuType}
119+
|and captures ${CaptureSet(overlap)}, but $whatStr also passed separately
120+
|in the ${whereStr.trim} with type $clashType.
106121
|
107-
| Capture set of $whereStr : $clashCaptures$clashHiddenStr
108-
| Hidden set of current argument : $hiddenCaptures
109-
| Footprint of $whereStr : $clashFootprint
110-
| Hidden footprint of current argument: $hiddenFootprint
111-
| Overlap of footprints : ${CaptureSet(hiddenFootprint.overlapWith(clashFootprint))}""",
122+
| Capture set of $whereStr : $clashCaptures
123+
| Hidden set of current argument : $hiddenCaptures
124+
| Footprint of $whereStr : $clashFootprint
125+
| Hidden footprint of current argument : $hiddenFootprint
126+
| Declared footprint of current argument: $declaredFootprint
127+
| Undeclared overlap of footprints : $footprintOverlap""",
112128
arg.srcPos)
113129
end sepError
114130

115-
private def checkApply(fn: Tree, args: List[Tree])(using Context): Unit =
116-
val fnCaptures =
117-
if true then
118-
fn.nuType.deepCaptureSet
119-
else methPart(fn) match
120-
case Select(qual, _) => qual.nuType.deepCaptureSet
121-
case _ => CaptureSet.empty
122-
val argCaptures = args.map(captures)
123-
capt.println(i"check separate $fn($args), fnCaptures = $fnCaptures, argCaptures = $argCaptures")
131+
private def checkApply(fn: Tree, args: List[Tree], deps: collection.Map[Tree, List[Tree]])(using Context): Unit =
132+
val fnCaptures = methPart(fn) match
133+
case Select(qual, _) => qual.nuType.captureSet
134+
case _ => CaptureSet.empty
135+
capt.println(i"check separate $fn($args), fnCaptures = $fnCaptures, argCaptures = ${args.map(formalCaptures)}, deps = ${deps.toList}")
124136
var footprint = fnCaptures.footprint
125137
val footprints = mutable.ListBuffer[(CaptureSet, Int)]((footprint, 0))
126138
val indexedArgs = args.zipWithIndex
139+
140+
def subtractDeps(elems: Refs, arg: Tree): Refs =
141+
deps(arg).foldLeft(elems): (elems, dep) =>
142+
elems -- actualCaptures(dep).footprint.elems
143+
127144
for (arg, idx) <- indexedArgs do
128145
if !arg.needsSepCheck then
129-
footprint = footprint ++ captures(arg)
146+
footprint = footprint ++ CaptureSet(subtractDeps(actualCaptures(arg).footprint.elems, arg))
130147
footprints += ((footprint, idx + 1))
131148
for (arg, idx) <- indexedArgs do
132149
if arg.needsSepCheck then
133-
val ac = captures(arg)
150+
val ac = formalCaptures(arg)
134151
val hiddenInArg = CaptureSet(hidden(ac)).footprint
135152
//println(i"check sep $arg: $ac, footprint so far = $footprint, hidden = $hiddenInArg")
136-
val overlap = hiddenInArg.footprint.overlapWith(footprint)
153+
val overlap = subtractDeps(hiddenInArg.overlapWith(footprint), arg)
137154
if !overlap.isEmpty then
138-
sepError(fn, args, idx, overlap, hiddenInArg, footprints.toList)
139-
footprint = footprint ++ hiddenInArg.footprint ++ ac.footprint
155+
sepError(fn, args, idx, overlap, hiddenInArg, footprints.toList, deps)
156+
footprint ++= actualCaptures(arg).footprint
140157
footprints += ((footprint, idx + 1))
141158
end checkApply
142159

160+
private def collectMethodTypes(tp: Type): List[TermLambda] = tp match
161+
case tp: MethodType => tp :: collectMethodTypes(tp.resType)
162+
case tp: PolyType => collectMethodTypes(tp.resType)
163+
case _ => Nil
164+
165+
private def dependencies(fn: Tree, argss: List[List[Tree]])(using Context): collection.Map[Tree, List[Tree]] =
166+
val mtpe =
167+
if fn.symbol.exists then fn.symbol.info
168+
else fn.tpe.widen // happens for PolyFunction applies
169+
val mtps = collectMethodTypes(mtpe)
170+
assert(mtps.hasSameLengthAs(argss), i"diff for $fn: ${fn.symbol} /// $mtps /// $argss")
171+
val mtpsWithArgs = mtps.zip(argss)
172+
val argMap = mtpsWithArgs.toMap
173+
val deps = mutable.HashMap[Tree, List[Tree]]().withDefaultValue(Nil)
174+
for
175+
(mt, args) <- mtpsWithArgs
176+
(formal, arg) <- mt.paramInfos.zip(args)
177+
dep <- formal.captureSet.elems.toList
178+
do
179+
val referred = dep match
180+
case dep: TermParamRef =>
181+
argMap(dep.binder)(dep.paramNum) :: Nil
182+
case dep: ThisType if dep.cls == fn.symbol.owner =>
183+
val Select(qual, _) = fn: @unchecked
184+
qual :: Nil
185+
case _ =>
186+
Nil
187+
deps(arg) ++= referred
188+
deps
189+
143190
private def traverseApply(tree: Tree, argss: List[List[Tree]])(using Context): Unit = tree match
144191
case Apply(fn, args) => traverseApply(fn, args :: argss)
145192
case TypeApply(fn, args) => traverseApply(fn, argss) // skip type arguments
146193
case _ =>
147194
if argss.nestedExists(_.needsSepCheck) then
148-
checkApply(tree, argss.flatten)
195+
checkApply(tree, argss.flatten, dependencies(tree, argss))
149196

150197
def traverse(tree: Tree)(using Context): Unit =
151198
tree match

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,9 @@ object Synthetics:
132132
val (pt: PolyType) = info: @unchecked
133133
val (mt: MethodType) = pt.resType: @unchecked
134134
val (enclThis: ThisType) = owner.thisType: @unchecked
135+
val paramCaptures = CaptureSet(enclThis, defn.captureRoot.termRef)
135136
pt.derivedLambdaType(resType = MethodType(mt.paramNames)(
136-
mt1 => mt.paramInfos.map(_.capturing(CaptureSet.universal)),
137+
mt1 => mt.paramInfos.map(_.capturing(paramCaptures)),
137138
mt1 => CapturingType(mt.resType, CaptureSet(enclThis, mt1.paramRefs.head))))
138139

139140
def transformCurriedTupledCaptures(info: Type, owner: Symbol) =
@@ -148,7 +149,10 @@ object Synthetics:
148149
ExprType(mapFinalResult(et.resType, CapturingType(_, CaptureSet(enclThis))))
149150

150151
def transformCompareCaptures =
151-
MethodType(defn.ObjectType.capturing(CaptureSet.universal) :: Nil, defn.BooleanType)
152+
val (enclThis: ThisType) = symd.owner.thisType: @unchecked
153+
MethodType(
154+
defn.ObjectType.capturing(CaptureSet(defn.captureRoot.termRef, enclThis)) :: Nil,
155+
defn.BooleanType)
152156

153157
symd.copySymDenotation(info = symd.name match
154158
case DefaultGetterName(nme.copy, n) =>

scala2-library-cc/src/scala/collection/IterableOnce.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -805,7 +805,7 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A]^ =>
805805
case _ => Some(reduceLeft(op))
806806
}
807807
private final def reduceLeftOptionIterator[B >: A](op: (B, A) => B): Option[B] = reduceOptionIterator[A, B](iterator)(op)
808-
private final def reduceOptionIterator[X >: A, B >: X](it: Iterator[X]^)(op: (B, X) => B): Option[B] = {
808+
private final def reduceOptionIterator[X >: A, B >: X](it: Iterator[X]^{this, caps.cap})(op: (B, X) => B): Option[B] = {
809809
if (it.hasNext) {
810810
var acc: B = it.next()
811811
while (it.hasNext)

tests/neg-custom-args/captures/caseclass/Test_2.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ def test(c: C) =
55
val mixed: () ->{c} Unit = pure
66
val x = Ref(impure)
77
val _: Ref = x // error
8-
val y = x.copy()
8+
val y = caps.unsafe.unsafeAssumeSeparate(x.copy()) // TODO remove
99
val yc: Ref = y // error
1010
val y0 = x.copy(pure)
1111
val yc0: Ref = y0

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
11
-- Error: tests/neg-custom-args/captures/filevar-expanded.scala:34:19 --------------------------------------------------
22
34 | withFile(io3): f => // error: separation failure
33
| ^
4-
| Separation failure: argument of type (f: test2.File^{io3}) ->{io3} Unit to capture-polymorphic parameter
5-
| op of type (f: test2.File^{io3}) => Unit captures {io3}, and this capability is also passed
6-
| separately in the first argument with type (io3 : test2.IO^) to method withFile.
4+
| Separation failure: argument of type (f: test2.File^{io3}) ->{io3} Unit
5+
| to method withFile: [T](io2: test2.IO^)(op: (f: test2.File^{io2}) => T): T
6+
| corresponds to capture-polymorphic formal parameter op of type (f: test2.File^{io3}) => Unit
7+
| and captures {io3}, but this capability is also passed separately
8+
| in the first argument with type (io3 : test2.IO^).
79
|
8-
| Capture set of first argument : {cap}
9-
| Hidden set of first argument : {io3}
10-
| Hidden set of current argument : {io3}
11-
| Footprint of first argument : {io3}
12-
| Hidden footprint of current argument: {io3}
13-
| Overlap of footprints : {io3}
10+
| Capture set of first argument : {io3}
11+
| Hidden set of current argument : {io3}
12+
| Footprint of first argument : {io3}
13+
| Hidden footprint of current argument : {io3}
14+
| Declared footprint of current argument: {}
15+
| Undeclared overlap of footprints : {io3}
1416
35 | val o = Service(io3)
1517
36 | o.file = f // this is a bit dubious. It's legal since we treat class refinements
1618
37 | // as capture set variables that can be made to include refs coming from outside.

tests/neg-custom-args/captures/function-combinators.check

Lines changed: 0 additions & 24 deletions
This file was deleted.

tests/neg-custom-args/captures/function-combinators.scala

Lines changed: 0 additions & 31 deletions
This file was deleted.

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,15 @@
2929
-- Error: tests/neg-custom-args/captures/lazyref.scala:24:55 -----------------------------------------------------------
3030
24 | val ref4 = (if cap1 == cap2 then ref1 else ref2).map(g) // error: separation failure
3131
| ^
32-
|Separation failure: argument of type (x: Int) ->{cap2} Int to capture-polymorphic parameter
33-
|f of type Int => Int captures {cap2}, and this capability is also passed
34-
|separately in the function prefix with type (LazyRef[Int]{val elem: () ->{ref2*} Int} | (ref1 : LazyRef[Int]{val elem: () ->{cap1} Int}^{cap1}))^{ref2} to method map.
32+
|Separation failure: argument of type (x: Int) ->{cap2} Int
33+
|to method map: [U](f: T => U): LazyRef[U]^{f, LazyRef.this}
34+
|corresponds to capture-polymorphic formal parameter f of type Int => Int
35+
|and captures {cap2}, but this capability is also passed separately
36+
|in the function prefix with type (LazyRef[Int]{val elem: () ->{ref2*} Int} | (ref1 : LazyRef[Int]{val elem: () ->{cap1} Int}^{cap1}))^{ref2}.
3537
|
36-
| Capture set of function prefix : {ref1, ref2}
37-
| Hidden set of current argument : {cap2}
38-
| Footprint of function prefix : {cap2, cap1, ref1, ref2}
39-
| Hidden footprint of current argument: {cap2}
40-
| Overlap of footprints : {cap2}
38+
| Capture set of function prefix : {ref1, ref2}
39+
| Hidden set of current argument : {cap2}
40+
| Footprint of function prefix : {ref1, ref2, cap1, cap2}
41+
| Hidden footprint of current argument : {cap2}
42+
| Declared footprint of current argument: {}
43+
| Undeclared overlap of footprints : {cap2}

0 commit comments

Comments
 (0)