Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make reach refinement shallow #19171

Merged
merged 3 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 37 additions & 12 deletions compiler/src/dotty/tools/dotc/cc/CaptureOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,22 @@ extension (tp: Type)
* (2) all covariant occurrences of cap replaced by `x*`, provided there
* are no occurrences in `T` at other variances. (1) is standard, whereas
* (2) is new.
*
* For (2), multiple-flipped covariant occurrences of cap won't be replaced.
* In other words,
*
* - For xs: List[File^] ==> List[File^{xs*}], the cap is replaced;
* - while f: [R] -> (op: File^ => R) -> R remains unchanged.
*
* Without this restriction, the signature of functions like withFile:
*
* (path: String) -> [R] -> (op: File^ => R) -> R
*
* could be refined to
*
* (path: String) -> [R] -> (op: File^{withFile*} => R) -> R
*
* which is clearly unsound.
*
* Why is this sound? Covariant occurrences of cap must represent capabilities
* that are reachable from `x`, so they are included in the meaning of `{x*}`.
Expand All @@ -245,18 +261,27 @@ extension (tp: Type)
def withReachCaptures(ref: Type)(using Context): Type =
object narrowCaps extends TypeMap:
var ok = true
def apply(t: Type) = t.dealias match
case t1 @ CapturingType(p, cs) if cs.isUniversal =>
if variance > 0 then
t1.derivedCapturingType(apply(p), ref.reach.singletonCaptureSet)
else
ok = false
t
case _ => t match
case t @ CapturingType(p, cs) =>
t.derivedCapturingType(apply(p), cs) // don't map capture set variables
case t =>
mapOver(t)

/** Has the variance been flipped at this point? */
private var isFlipped: Boolean = false

def apply(t: Type) =
val saved = isFlipped
try
if variance <= 0 then isFlipped = true
t.dealias match
case t1 @ CapturingType(p, cs) if cs.isUniversal =>
Copy link
Contributor

@odersky odersky Dec 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the logic can be simplified a bit by this:

Suggested change
case t1 @ CapturingType(p, cs) if cs.isUniversal =>
case t1 @ CapturingType(p, cs) if cs.isUniversal && !isFlipped =>

if variance > 0 then
t1.derivedCapturingType(apply(p), if isFlipped then cs else ref.reach.singletonCaptureSet)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then you can leave things as they were here.

else
ok = false
t
case _ => t match
case t @ CapturingType(p, cs) =>
t.derivedCapturingType(apply(p), cs) // don't map capture set variables
case t =>
mapOver(t)
finally isFlipped = saved
ref match
case ref: CaptureRef if ref.isTrackableRef =>
val tp1 = narrowCaps(tp)
Expand Down
18 changes: 18 additions & 0 deletions tests/neg-custom-args/captures/refine-reach-shallow.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import language.experimental.captureChecking
trait IO
def test1(): Unit =
val f: IO^ => IO^ = x => x
val g: IO^ => IO^{f*} = f // error
def test2(): Unit =
val f: [R] -> (IO^ => R) -> R = ???
val g: [R] -> (IO^{f*} => R) -> R = f // error
def test3(): Unit =
val f: [R] -> (IO^ -> R) -> R = ???
val g: [R] -> (IO^{f*} -> R) -> R = f // error
def test4(): Unit =
val xs: List[IO^] = ???
val ys: List[IO^{xs*}] = xs // ok
def test5(): Unit =
val f: [R] -> (IO^ -> R) -> IO^ = ???
val g: [R] -> (IO^ -> R) -> IO^{f*} = f // ok
val h: [R] -> (IO^{f*} -> R) -> IO^ = f // error
8 changes: 8 additions & 0 deletions tests/neg-custom-args/captures/refine-withFile.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import language.experimental.captureChecking

trait File
val useFile: [R] -> (path: String) -> (op: File^ -> R) -> R = ???
def main(): Unit =
val f: [R] -> (path: String) -> (op: File^ -> R) -> R = useFile
val g: [R] -> (path: String) -> (op: File^{f*} -> R) -> R = f // error
val leaked = g[File^{f*}]("test")(f => f) // boom
Loading