Skip to content

Commit bc818a9

Browse files
committed
Fix special capture set handling in recheckApply, Step 2
Step 2: Change the logic. The previous one was unsound. The new logic is a bot too conservative. I left comments in tests where it could be improved.
1 parent 90749ea commit bc818a9

File tree

7 files changed

+33
-20
lines changed

7 files changed

+33
-20
lines changed

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

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -554,30 +554,26 @@ class CheckCaptures extends Recheck, SymTransformer:
554554

555555
/** A specialized implementation of the apply rule.
556556
*
557-
* E |- f: Ra ->Cf Rr^Cr
558-
* E |- a: Ra^Ca
557+
* E |- q: Tq^Cq
558+
* E |- q.f: Ta ->Cf Tr^Cr
559+
* E |- a: Ta
559560
* ---------------------
560-
* E |- f a: Rr^C
561+
* E |- f(a): Tr^C
561562
*
562-
* The implementation picks as `C` one of `{f, a}` or `Cr`, depending on the
563-
* outcome of a `mightSubcapture` test. It picks `{f, a}` if this might subcapture Cr
564-
* and Cr otherwise.
563+
* The implementation picks `C` as `Cq` instead of `Cr`, if
564+
* 1. The argument(s) Ta are always pure
565+
* 2. `Cq` might subcapture `Cr`.
565566
*/
566567
protected override
567568
def recheckApplication(tree: Apply, qualType: Type, funType: MethodType, argTypes: List[Type])(using Context): Type =
568569
Existential.toCap(super.recheckApplication(tree, qualType, funType, argTypes)) match
569570
case appType @ CapturingType(appType1, refs)
570571
if qualType.exists
571572
&& !tree.fun.symbol.isConstructor
572-
&& !qualType.isBoxedCapturing // TODO: This is not strng enough, we also have
573-
// to exclude existentials in function results
574-
&& !argTypes.exists(_.isBoxedCapturing)
573+
&& argTypes.forall(_.isAlwaysPure)
575574
&& qualType.captureSet.mightSubcapture(refs)
576-
&& argTypes.forall(_.captureSet.mightSubcapture(refs))
577575
=>
578-
val callCaptures = tree.args.foldLeft(qualType.captureSet): (cs, arg) =>
579-
cs ++ arg.tpe.captureSet
580-
appType.derivedCapturingType(appType1, callCaptures)
576+
appType.derivedCapturingType(appType1, qualType.captureSet)
581577
.showing(i"narrow $tree: $appType, refs = $refs, qual-cs = ${qualType.captureSet} = $result", capt)
582578
case appType =>
583579
appType

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import scala.annotation.tailrec
1717
import scala.annotation.unchecked.uncheckedVariance
1818
import scala.runtime.Statics
1919
import language.experimental.captureChecking
20-
20+
import caps.unsafe.unsafeAssumePure
2121

2222
/** Iterators are data structures that allow to iterate over a sequence
2323
* of elements. They have a `hasNext` method for checking
@@ -595,7 +595,8 @@ trait Iterator[+A] extends IterableOnce[A] with IterableOnceOps[A, Iterator, Ite
595595

596596
private[this] def nextCur(): Unit = {
597597
cur = null
598-
cur = f(self.next()).iterator
598+
cur = f(self.next()).iterator.unsafeAssumePure
599+
// !!! see explanation in colltest5.CollectionStrawManCC5_1.flatMap why the unsafeAssumePure is needed
599600
_hasNext = -1
600601
}
601602

scala2-library-cc/src/scala/collection/immutable/LazyListIterable.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import scala.runtime.Statics
2525
import language.experimental.captureChecking
2626
import annotation.unchecked.uncheckedCaptures
2727
import caps.untrackedCaptures
28+
import caps.unsafe.unsafeAssumePure
2829

2930
/** This class implements an immutable linked list. We call it "lazy"
3031
* because it computes its elements only when they are needed.
@@ -1041,7 +1042,8 @@ object LazyListIterable extends IterableFactory[LazyListIterable] {
10411042
var itHasNext = false
10421043
var rest = restRef // var rest = restRef.elem
10431044
while (!itHasNext && !rest.isEmpty) {
1044-
it = f(rest.head).iterator
1045+
it = f(rest.head).iterator.unsafeAssumePure
1046+
// !!! see explanation in colltest5.CollectionStrawManCC5_1.flatMap why the unsafeAssumePure is needed
10451047
itHasNext = it.hasNext
10461048
if (!itHasNext) { // wait to advance `rest` because `it.next()` can throw
10471049
rest = rest.tail

scala2-library-cc/src/scala/collection/mutable/Buffer.scala

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package mutable
1616
import scala.annotation.nowarn
1717
import language.experimental.captureChecking
1818
import caps.unboxed
19+
import caps.unsafe.unsafeAssumePure
1920

2021
/** A `Buffer` is a growable and shrinkable `Seq`. */
2122
trait Buffer[A]
@@ -185,7 +186,10 @@ trait IndexedBuffer[A] extends IndexedSeq[A]
185186
var i = 0
186187
val s = size
187188
val newElems = new Array[(IterableOnce[A]^{f*})](s)
188-
while (i < s) { newElems(i) = f(this(i)); i += 1 }
189+
while i < s do
190+
newElems(i) = f(this(i)).unsafeAssumePure
191+
// !!! see explanation in colltest5.CollectionStrawManCC5_1.flatMap why the unsafeAssumePure is needed
192+
i += 1
189193
clear()
190194
i = 0
191195
while (i < s) { ++=(newElems(i)); i += 1 }

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/reaches.scala:64:27 --------------------------------------
4848
64 | val f1: File^{id*} = id(f) // error, since now id(f): File^
4949
| ^^^^^
50-
| Found: File^{id, f}
50+
| Found: File^
5151
| Required: File^{id*}
5252
|
5353
| longer explanation available when compiling with `-explain`

tests/pos-custom-args/captures/nested-classes-2.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,5 @@ def test2(x1: (() => Unit), x2: (() => Unit) => Unit) =
2020
def test3(y1: (() => Unit), y2: (() => Unit) => Unit) =
2121
val cc1: C1^{y1, y2} = C1(y1, y2)
2222
val cc2 = cc1.c2(x1, x2)
23-
val cc3: cc1.C2^{cc1, x1, x2} = cc2
23+
val cc3: cc1.C2^{cc2} = cc2
2424

tests/run-custom-args/captures/colltest5/CollectionStrawManCC5_1.scala

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import Predef.{augmentString as _, wrapString as _, *}
55
import scala.reflect.ClassTag
66
import annotation.unchecked.{uncheckedVariance, uncheckedCaptures}
77
import annotation.tailrec
8+
import caps.unsafe.unsafeAssumePure
89

910
/** A strawman architecture for new collections. It contains some
1011
* example collection classes and methods with the intent to expose
@@ -555,7 +556,16 @@ object CollectionStrawMan5 {
555556
private var myCurrent: Iterator[B]^{this, f} = Iterator.empty
556557
private def current = {
557558
while (!myCurrent.hasNext && self.hasNext)
558-
myCurrent = f(self.next()).iterator
559+
myCurrent = f(self.next()).iterator.unsafeAssumePure
560+
// !!! This is unsafe since the iterator's result could return a capability
561+
// depending on the argument self.next() of type A. To exclude that we'd have
562+
// to define f to be of type EX c. A ->{c} IterableOnce[B]^{c}, i.e. `c` may
563+
// not depend on A. But to get there we have to
564+
// - improve the way we express existentials using `^`
565+
// - rework the recheckApplication code to cater for this. Right now
566+
// we cannot do anything since `A` is not always pure. But if we took
567+
// the existential scope of the result into account, this could provide
568+
// a solution.
559569
myCurrent
560570
}
561571
def hasNext = current.hasNext

0 commit comments

Comments
 (0)