Skip to content

Commit

Permalink
Go back to original no cap in box/unbox restrictions
Browse files Browse the repository at this point in the history 
We go back to the original lifetime restriction that box/unbox cannot
apply to universal capture sets, and drop the later restriction that
type variable instantiations may not deeply capture cap.

The original restriction is proven to be sound and is probably expressive
enough when we add reach capabilities.

This required some changes in tests and also in the standard library.

The original restriction is in place for source <= 3.2 and >= 3.5. Source
3.3 and 3.4 use the alternative restriction on type variable instances.

Some neg tests have not been brought forward to 3.4. They are all in
tests/neg-customargs/captures and start with

//> using options -source 3.4

We need to look at these tests one-by-one and analyze whether the new 3.5 behavior
is correct.
  • Loading branch information
@odersky
odersky committed Jun 7, 2024
1 parent 844537a commit 95d23a4
Show file tree
Hide file tree
Showing 47 changed files with 310 additions and 167 deletions.
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/cc/CaptureOps.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ object ccConfig:
* previous global retriction that `cap` can't be boxed or unboxed.
*/
def allowUniversalInBoxed(using Context) =
Feature.sourceVersion.isAtLeast(SourceVersion.`3.3`)
Feature.sourceVersion.stable == SourceVersion.`3.3`
|| Feature.sourceVersion.stable == SourceVersion.`3.4`
//|| Feature.sourceVersion.stable == SourceVersion.`3.5` // drop `//` if you want to test with the sealed type params strategy
end ccConfig


Expand Down
4 changes: 2 additions & 2 deletions scala2-library-cc/src/scala/collection/Iterator.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1008,7 +1008,7 @@ object Iterator extends IterableFactory[Iterator] {
def newBuilder[A]: Builder[A, Iterator[A]] =
new ImmutableBuilder[A, Iterator[A]](empty[A]) {
override def addOne(elem: A): this.type = { elems = elems ++ single(elem); this }
}
}.asInstanceOf // !!! CC unsafe op

/** Creates iterator that produces the results of some element computation a number of times.
*
Expand Down Expand Up @@ -1160,7 +1160,7 @@ object Iterator extends IterableFactory[Iterator] {
@tailrec def merge(): Unit =
if (current.isInstanceOf[ConcatIterator[_]]) {
val c = current.asInstanceOf[ConcatIterator[A]]
current = c.current
current = c.current.asInstanceOf // !!! CC unsafe op
currentHasNextChecked = c.currentHasNextChecked
if (c.tail != null) {
if (last == null) last = c.last
Expand Down
12 changes: 7 additions & 5 deletions scala2-library-cc/src/scala/collection/SeqView.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,14 @@ object SeqView {
}

@SerialVersionUID(3L)
class Sorted[A, B >: A] private (private[this] var underlying: SomeSeqOps[A]^,
class Sorted[A, B >: A] private (underlying: SomeSeqOps[A]^,
private[this] val len: Int,
ord: Ordering[B])
extends SeqView[A] {
outer: Sorted[A, B]^ =>

private var myUnderlying: SomeSeqOps[A]^{underlying} = underlying

// force evaluation immediately by calling `length` so infinite collections
// hang on `sorted`/`sortWith`/`sortBy` rather than on arbitrary method calls
def this(underlying: SomeSeqOps[A]^, ord: Ordering[B]) = this(underlying, underlying.length, ord)
Expand Down Expand Up @@ -221,10 +223,10 @@ object SeqView {
val res = {
val len = this.len
if (len == 0) Nil
else if (len == 1) List(underlying.head)
else if (len == 1) List(myUnderlying.head)
else {
val arr = new Array[Any](len) // Array[Any] =:= Array[AnyRef]
underlying.copyToArray(arr)
myUnderlying.copyToArray(arr)
java.util.Arrays.sort(arr.asInstanceOf[Array[AnyRef]], ord.asInstanceOf[Ordering[AnyRef]])
// casting the Array[AnyRef] to Array[A] and creating an ArraySeq from it
// is safe because:
Expand All @@ -238,12 +240,12 @@ object SeqView {
}
}
evaluated = true
underlying = null
myUnderlying = null
res
}

private[this] def elems: SomeSeqOps[A]^{this} = {
val orig = underlying
val orig = myUnderlying
if (evaluated) _sorted else orig
}

Expand Down
53 changes: 29 additions & 24 deletions scala2-library-cc/src/scala/collection/immutable/LazyListIterable.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import scala.language.implicitConversions
import scala.runtime.Statics
import language.experimental.captureChecking
import annotation.unchecked.uncheckedCaptures
import caps.untrackedCaptures

/** This class implements an immutable linked list. We call it "lazy"
* because it computes its elements only when they are needed.
Expand Down Expand Up @@ -245,14 +246,16 @@ import annotation.unchecked.uncheckedCaptures
* @define evaluatesAllElements This method evaluates all elements of the collection.
*/
@SerialVersionUID(3L)
final class LazyListIterable[+A] private(private[this] var lazyState: () => LazyListIterable.State[A]^)
final class LazyListIterable[+A] private(@untrackedCaptures lazyState: () => LazyListIterable.State[A]^)
extends AbstractIterable[A]
with Iterable[A]
with IterableOps[A, LazyListIterable, LazyListIterable[A]]
with IterableFactoryDefaults[A, LazyListIterable]
with Serializable {
import LazyListIterable._

private var myLazyState = lazyState

@volatile private[this] var stateEvaluated: Boolean = false
@inline private def stateDefined: Boolean = stateEvaluated
private[this] var midEvaluation = false
Expand All @@ -264,11 +267,11 @@ final class LazyListIterable[+A] private(private[this] var lazyState: () => Lazy
throw new RuntimeException("self-referential LazyListIterable or a derivation thereof has no more elements")
}
midEvaluation = true
val res = try lazyState() finally midEvaluation = false
val res = try myLazyState() finally midEvaluation = false
// if we set it to `true` before evaluating, we may infinite loop
// if something expects `state` to already be evaluated
stateEvaluated = true
lazyState = null // allow GC
myLazyState = null // allow GC
res
}

Expand Down Expand Up @@ -755,7 +758,7 @@ final class LazyListIterable[+A] private(private[this] var lazyState: () => Lazy
* The iterator returned by this method mostly preserves laziness;
* a single element ahead of the iterator is evaluated.
*/
override def grouped(size: Int): Iterator[LazyListIterable[A]] = {
override def grouped(size: Int): Iterator[LazyListIterable[A]]^{this} = {
require(size > 0, "size must be positive, but was " + size)
slidingImpl(size = size, step = size)
}
Expand All @@ -765,12 +768,12 @@ final class LazyListIterable[+A] private(private[this] var lazyState: () => Lazy
* The iterator returned by this method mostly preserves laziness;
* `size - step max 1` elements ahead of the iterator are evaluated.
*/
override def sliding(size: Int, step: Int): Iterator[LazyListIterable[A]] = {
override def sliding(size: Int, step: Int): Iterator[LazyListIterable[A]]^{this} = {
require(size > 0 && step > 0, s"size=$size and step=$step, but both must be positive")
slidingImpl(size = size, step = step)
}

@inline private def slidingImpl(size: Int, step: Int): Iterator[LazyListIterable[A]] =
@inline private def slidingImpl(size: Int, step: Int): Iterator[LazyListIterable[A]]^{this} =
if (knownIsEmpty) Iterator.empty
else new SlidingIterator[A](this, size = size, step = step)

Expand Down Expand Up @@ -996,7 +999,7 @@ object LazyListIterable extends IterableFactory[LazyListIterable] {

private def filterImpl[A](ll: LazyListIterable[A]^, p: A => Boolean, isFlipped: Boolean): LazyListIterable[A]^{ll, p} = {
// DO NOT REFERENCE `ll` ANYWHERE ELSE, OR IT WILL LEAK THE HEAD
var restRef: LazyListIterable[A]^{ll*} = ll // restRef is captured by closure arg to newLL, so A is not recognized as parametric
var restRef: LazyListIterable[A]^{ll} = ll // restRef is captured by closure arg to newLL, so A is not recognized as parametric
newLL {
var elem: A = null.asInstanceOf[A]
var found = false
Expand All @@ -1013,7 +1016,7 @@ object LazyListIterable extends IterableFactory[LazyListIterable] {

private def collectImpl[A, B](ll: LazyListIterable[A]^, pf: PartialFunction[A, B]^): LazyListIterable[B]^{ll, pf} = {
// DO NOT REFERENCE `ll` ANYWHERE ELSE, OR IT WILL LEAK THE HEAD
var restRef: LazyListIterable[A]^{ll*} = ll // restRef is captured by closure arg to newLL, so A is not recognized as parametric
var restRef: LazyListIterable[A]^{ll} = ll // restRef is captured by closure arg to newLL, so A is not recognized as parametric
newLL {
val marker = Statics.pfMarker
val toMarker = anyToMarker.asInstanceOf[A => B] // safe because Function1 is erased
Expand All @@ -1032,7 +1035,7 @@ object LazyListIterable extends IterableFactory[LazyListIterable] {

private def flatMapImpl[A, B](ll: LazyListIterable[A]^, f: A => IterableOnce[B]^): LazyListIterable[B]^{ll, f} = {
// DO NOT REFERENCE `ll` ANYWHERE ELSE, OR IT WILL LEAK THE HEAD
var restRef: LazyListIterable[A]^{ll*} = ll // restRef is captured by closure arg to newLL, so A is not recognized as parametric
var restRef: LazyListIterable[A]^{ll} = ll // restRef is captured by closure arg to newLL, so A is not recognized as parametric
newLL {
var it: Iterator[B]^{ll, f} = null
var itHasNext = false
Expand All @@ -1056,7 +1059,7 @@ object LazyListIterable extends IterableFactory[LazyListIterable] {

private def dropImpl[A](ll: LazyListIterable[A]^, n: Int): LazyListIterable[A]^{ll} = {
// DO NOT REFERENCE `ll` ANYWHERE ELSE, OR IT WILL LEAK THE HEAD
var restRef: LazyListIterable[A]^{ll*} = ll // restRef is captured by closure arg to newLL, so A is not recognized as parametric
var restRef: LazyListIterable[A]^{ll} = ll // restRef is captured by closure arg to newLL, so A is not recognized as parametric
var iRef = n // val iRef = new IntRef(n)
newLL {
var rest = restRef // var rest = restRef.elem
Expand All @@ -1073,7 +1076,7 @@ object LazyListIterable extends IterableFactory[LazyListIterable] {

private def dropWhileImpl[A](ll: LazyListIterable[A]^, p: A => Boolean): LazyListIterable[A]^{ll, p} = {
// DO NOT REFERENCE `ll` ANYWHERE ELSE, OR IT WILL LEAK THE HEAD
var restRef: LazyListIterable[A]^{ll*} = ll // restRef is captured by closure arg to newLL, so A is not recognized as parametric
var restRef: LazyListIterable[A]^{ll} = ll // restRef is captured by closure arg to newLL, so A is not recognized as parametric
newLL {
var rest = restRef // var rest = restRef.elem
while (!rest.isEmpty && p(rest.head)) {
Expand All @@ -1086,8 +1089,8 @@ object LazyListIterable extends IterableFactory[LazyListIterable] {

private def takeRightImpl[A](ll: LazyListIterable[A]^, n: Int): LazyListIterable[A]^{ll} = {
// DO NOT REFERENCE `ll` ANYWHERE ELSE, OR IT WILL LEAK THE HEAD
var restRef: LazyListIterable[A]^{ll*} = ll // restRef is captured by closure arg to newLL, so A is not recognized as parametric
var scoutRef: LazyListIterable[A]^{ll*} = ll // same situation
var restRef: LazyListIterable[A]^{ll} = ll // restRef is captured by closure arg to newLL, so A is not recognized as parametric
var scoutRef: LazyListIterable[A]^{ll} = ll // same situation
var remainingRef = n // val remainingRef = new IntRef(n)
newLL {
var scout = scoutRef // var scout = scoutRef.elem
Expand Down Expand Up @@ -1236,33 +1239,35 @@ object LazyListIterable extends IterableFactory[LazyListIterable] {
*/
def newBuilder[A]: Builder[A, LazyListIterable[A]] = new LazyBuilder[A]

private class LazyIterator[+A](private[this] var lazyList: LazyListIterable[A]^) extends AbstractIterator[A] {
override def hasNext: Boolean = !lazyList.isEmpty
private class LazyIterator[+A](lazyList: LazyListIterable[A]^) extends AbstractIterator[A] {
private var myLazyList = lazyList
override def hasNext: Boolean = !myLazyList.isEmpty

override def next(): A =
if (lazyList.isEmpty) Iterator.empty.next()
if (myLazyList.isEmpty) Iterator.empty.next()
else {
val res = lazyList.head
lazyList = lazyList.tail
val res = myLazyList.head
myLazyList = myLazyList.tail
res
}
}

private class SlidingIterator[A](private[this] var lazyList: LazyListIterable[A]^, size: Int, step: Int)
private class SlidingIterator[A](lazyList: LazyListIterable[A]^, size: Int, step: Int)
extends AbstractIterator[LazyListIterable[A]] {
private var myLazyList = lazyList
private val minLen = size - step max 0
private var first = true

def hasNext: Boolean =
if (first) !lazyList.isEmpty
else lazyList.lengthGt(minLen)
if (first) !myLazyList.isEmpty
else myLazyList.lengthGt(minLen)

def next(): LazyListIterable[A] = {
if (!hasNext) Iterator.empty.next()
else {
first = false
val list = lazyList
lazyList = list.drop(step)
val list = myLazyList
myLazyList = list.drop(step)
list.take(size)
}
}
Expand All @@ -1281,7 +1286,7 @@ object LazyListIterable extends IterableFactory[LazyListIterable] {
import LazyBuilder._

private[this] var next: DeferredState[A] = _
private[this] var list: LazyListIterable[A] = _
@uncheckedCaptures private[this] var list: LazyListIterable[A]^ = _

clear()

Expand Down
2 changes: 1 addition & 1 deletion tests/neg-custom-args/captures/box-adapt-cases.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ def test1(): Unit = {
type Id[X] = [T] -> (op: X => T) -> T

val x: Id[Cap^] = ???
x(cap => cap.use()) // was error, now OK
x(cap => cap.use()) // error, OK under sealed
}

def test2(io: Cap^): Unit = {
Expand Down
4 changes: 2 additions & 2 deletions tests/neg-custom-args/captures/capt-test.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ def handle[E <: Exception, R <: Top](op: (CT[E] @retains(caps.cap)) => R)(handl
catch case ex: E => handler(ex)

def test: Unit =
val b = handle[Exception, () => Nothing] { // error
val b = handle[Exception, () => Nothing] {
(x: CanThrow[Exception]) => () => raise(new Exception)(using x)
} {
} { // error
(ex: Exception) => ???
}
40 changes: 20 additions & 20 deletions tests/neg-custom-args/captures/capt1.check
View file
Original file line number Diff line number Diff line change
@@ -1,52 +1,52 @@
-- Error: tests/neg-custom-args/captures/capt1.scala:4:11 --------------------------------------------------------------
4 | () => if x == null then y else y // error
-- Error: tests/neg-custom-args/captures/capt1.scala:6:11 --------------------------------------------------------------
6 | () => if x == null then y else y // error
| ^
| (x : C^) cannot be referenced here; it is not included in the allowed capture set {}
| of an enclosing function literal with expected type () -> C
-- Error: tests/neg-custom-args/captures/capt1.scala:7:11 --------------------------------------------------------------
7 | () => if x == null then y else y // error
-- Error: tests/neg-custom-args/captures/capt1.scala:9:11 --------------------------------------------------------------
9 | () => if x == null then y else y // error
| ^
| (x : C^) cannot be referenced here; it is not included in the allowed capture set {}
| of an enclosing function literal with expected type Matchable
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/capt1.scala:14:2 -----------------------------------------
14 | def f(y: Int) = if x == null then y else y // error
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/capt1.scala:16:2 -----------------------------------------
16 | def f(y: Int) = if x == null then y else y // error
| ^
| Found: (y: Int) ->{x} Int
| Required: Matchable
15 | f
17 | f
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/capt1.scala:21:2 -----------------------------------------
21 | class F(y: Int) extends A: // error
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/capt1.scala:23:2 -----------------------------------------
23 | class F(y: Int) extends A: // error
| ^
| Found: A^{x}
| Required: A
22 | def m() = if x == null then y else y
23 | F(22)
24 | def m() = if x == null then y else y
25 | F(22)
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/capt1.scala:26:2 -----------------------------------------
26 | new A: // error
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/capt1.scala:28:2 -----------------------------------------
28 | new A: // error
| ^
| Found: A^{x}
| Required: A
27 | def m() = if x == null then y else y
29 | def m() = if x == null then y else y
|
| longer explanation available when compiling with `-explain`
-- Error: tests/neg-custom-args/captures/capt1.scala:32:12 -------------------------------------------------------------
32 | val z2 = h[() -> Cap](() => x) // error // error
-- Error: tests/neg-custom-args/captures/capt1.scala:34:12 -------------------------------------------------------------
34 | val z2 = h[() -> Cap](() => x) // error // error
| ^^^^^^^^^^^^
| Sealed type variable X cannot be instantiated to () -> box C^ since
| the part box C^ of that type captures the root capability `cap`.
| This is often caused by a local capability in an argument of method h
| leaking as part of its result.
-- Error: tests/neg-custom-args/captures/capt1.scala:32:30 -------------------------------------------------------------
32 | val z2 = h[() -> Cap](() => x) // error // error
-- Error: tests/neg-custom-args/captures/capt1.scala:34:30 -------------------------------------------------------------
34 | val z2 = h[() -> Cap](() => x) // error // error
| ^
| (x : C^) cannot be referenced here; it is not included in the allowed capture set {}
| of an enclosing function literal with expected type () -> box C^
-- Error: tests/neg-custom-args/captures/capt1.scala:34:12 -------------------------------------------------------------
34 | val z3 = h[(() -> Cap) @retains(x)](() => x)(() => C()) // error
-- Error: tests/neg-custom-args/captures/capt1.scala:36:12 -------------------------------------------------------------
36 | val z3 = h[(() -> Cap) @retains(x)](() => x)(() => C()) // error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
| Sealed type variable X cannot be instantiated to box () ->{x} Cap since
| the part Cap of that type captures the root capability `cap`.
Expand Down
2 changes: 2 additions & 0 deletions tests/neg-custom-args/captures/capt1.scala
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//> using options -source 3.4
// (to make sure we use the sealed policy)
import annotation.retains
class C
def f(x: C @retains(caps.cap), y: C): () -> C =
Expand Down
22 changes: 11 additions & 11 deletions tests/neg-custom-args/captures/effect-swaps-explicit.check
View file
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/effect-swaps-explicit.scala:62:8 -------------------------
61 | Result:
62 | Future: // error, type mismatch
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/effect-swaps-explicit.scala:64:8 -------------------------
63 | Result:
64 | Future: // error, type mismatch
| ^
| Found: Result.Ok[box Future[box T^?]^{fr, contextual$1}]
| Required: Result[Future[T], Nothing]
63 | fr.await.ok
65 | fr.await.ok
|--------------------------------------------------------------------------------------------------------------------
|Inline stack trace
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
|This location contains code that was inlined from effect-swaps-explicit.scala:39
39 | boundary(Ok(body))
|This location contains code that was inlined from effect-swaps-explicit.scala:41
41 | boundary(Ok(body))
| ^^^^^^^^
--------------------------------------------------------------------------------------------------------------------
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/effect-swaps-explicit.scala:72:10 ------------------------
72 | Future: fut ?=> // error: type mismatch
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/effect-swaps-explicit.scala:74:10 ------------------------
74 | Future: fut ?=> // error: type mismatch
| ^
| Found: Future[box T^?]^{fr, lbl}
| Required: Future[box T^?]^?
73 | fr.await.ok
75 | fr.await.ok
|
| longer explanation available when compiling with `-explain`
-- Error: tests/neg-custom-args/captures/effect-swaps-explicit.scala:66:15 ---------------------------------------------
66 | Result.make: //lbl ?=> // error, escaping label from Result
-- Error: tests/neg-custom-args/captures/effect-swaps-explicit.scala:68:15 ---------------------------------------------
68 | Result.make: //lbl ?=> // error, escaping label from Result
| ^^^^^^^^^^^
|local reference contextual$9 from (using contextual$9: boundary.Label[Result[box Future[box T^?]^{fr, contextual$9}, box E^?]]^):
| box Future[box T^?]^{fr, contextual$9} leaks into outer capture set of type parameter T of method make in object Result
Loading

0 comments on commit 95d23a4

Please sign in to comment.