Skip to content

Commit

Permalink
Two fixes to capture checking (scala#17524)
Browse files Browse the repository at this point in the history
- It changes `isParamDependent` to consider `CaptureDependency` as well,
so that when applying a capture dependent function the capture sets in
the types of the remaining parameters will be correctly substituted. For
example:
```scala
def foo[X](x: Foo[X]^, op: () ->{x} Unit): Unit = ???
foo(a, useA)
```
After rechecking `a` in the argument list, the expected type of the
second argument, `useA`, should become `() -> {a} Unit`. This example
previously does not work.

- It fixes scala#17517. It fixes the healing of capture sets, which happens
at the end of the capture checking phase, in which we traverse the
capture checked tree and try to heal the ill-formed capture sets in type
arguments. Here we say a capture set `C` in a type argument is
ill-formed if it contains `TermParamRef` bound by other lambdas. For
example:
```scala
def usingLogger[sealed T](f: OutputStream^)(op: Logger^{f} => T): T = ???
usingLogger[() ->?1 Unit](file)(l => () => l.log("test"))
```
`l` will be propagated to `?1` as a result of capture checking, which
makes `?1` ill-formed and we should widen `l` to `file`. The problem is
that we heal the capture sets one-after-another but the healing of a
later capture set may propagate more `TermParamRef`s to a healed one.
For example:
```
usingFile[() ->?2 Unit](  // should be error
  "foo",
  file => {
    usingLogger[() ->?1 Unit](file)(l => () => l.log("test"))
  }
)
```
After capture checking both `?1` and `?2` will be `{l}`. When traversing
the tree we first heal `?2`, wherein we widen `l` to `file`, but `file`
is a `TermRef` here and we do nothing. Then, only later when we heal
`?1`, we also widen `l` to `file` but this will end up propagating a
`TermParamRef` of `file` to `?2`, which we should have widened as well.
But `?2` is already healed and is not inspected again.

This PR solves this issue by installing a handler when healing the
capture sets which will try to heal the new elements propagated later in
the healing process.
  • Loading branch information
odersky authored May 22, 2023
2 parents c205a89 + 6870649 commit f2f1156
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 12 deletions.
15 changes: 14 additions & 1 deletion compiler/src/dotty/tools/dotc/cc/CaptureSet.scala
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,11 @@ sealed abstract class CaptureSet extends Showable:
if isUniversal then handler()
this

/** Invoke handler on the elements to check wellformedness of the capture set */
def ensureWellformed(handler: List[CaptureRef] => Context ?=> Unit)(using Context): this.type =
handler(elems.toList)
this

/** An upper approximation of this capture set, i.e. a constant set that is
* subcaptured by this set. If the current set is a variable
* it is the intersection of all upper approximations of known supersets
Expand Down Expand Up @@ -375,6 +380,9 @@ object CaptureSet:
/** A handler to be invoked if the root reference `cap` is added to this set */
var rootAddedHandler: () => Context ?=> Unit = () => ()

/** A handler to be invoked when new elems are added to this set */
var newElemAddedHandler: List[CaptureRef] => Context ?=> Unit = _ => ()

var description: String = ""

/** Record current elements in given VarState provided it does not yet
Expand Down Expand Up @@ -405,7 +413,8 @@ object CaptureSet:
if !isConst && recordElemsState() then
elems ++= newElems
if isUniversal then rootAddedHandler()
// assert(id != 2 || elems.size != 2, this)
newElemAddedHandler(newElems.toList)
// assert(id != 5 || elems.size != 3, this)
(CompareResult.OK /: deps) { (r, dep) =>
r.andAlso(dep.tryInclude(newElems, this))
}
Expand All @@ -425,6 +434,10 @@ object CaptureSet:
rootAddedHandler = handler
super.disallowRootCapability(handler)

override def ensureWellformed(handler: List[CaptureRef] => (Context) ?=> Unit)(using Context): this.type =
newElemAddedHandler = handler
super.ensureWellformed(handler)

private var computingApprox = false

/** Roughly: the intersection of all constant known supersets of this set.
Expand Down
7 changes: 5 additions & 2 deletions compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala
Original file line number Diff line number Diff line change
Expand Up @@ -976,8 +976,11 @@ class CheckCaptures extends Recheck, SymTransformer:
recur(refs, Nil)

private def healCaptureSet(cs: CaptureSet): Unit =
val toInclude = widenParamRefs(cs.elems.toList.filter(!isAllowed(_)).asInstanceOf)
toInclude.foreach(checkSubset(_, cs, tree.srcPos))
def avoidance(elems: List[CaptureRef])(using Context): Unit =
val toInclude = widenParamRefs(elems.filter(!isAllowed(_)).asInstanceOf)
//println(i"HEAL $cs by widening to $toInclude")
toInclude.foreach(checkSubset(_, cs, tree.srcPos))
cs.ensureWellformed(avoidance)

private var allowed: SimpleIdentitySet[TermParamRef] = SimpleIdentitySet.empty

Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3880,7 +3880,8 @@ object Types {
/** Does one of the parameter types contain references to earlier parameters
* of this method type which cannot be eliminated by de-aliasing?
*/
def isParamDependent(using Context): Boolean = paramDependencyStatus == TrueDeps
def isParamDependent(using Context): Boolean =
paramDependencyStatus == TrueDeps || paramDependencyStatus == CaptureDeps

/** Is there a dependency involving a reference in a capture set, but
* otherwise no true result dependency?
Expand Down
7 changes: 7 additions & 0 deletions tests/neg-custom-args/captures/usingLogFile-alt.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- Error: tests/neg-custom-args/captures/usingLogFile-alt.scala:18:2 ---------------------------------------------------
18 | usingFile( // error
| ^^^^^^^^^
| Sealed type variable T cannot be instantiated to box () => Unit since
| that type captures the root capability `cap`.
| This is often caused by a local capability in the body of method usingFile
| leaking as part of its result.
23 changes: 23 additions & 0 deletions tests/neg-custom-args/captures/usingLogFile-alt.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Reported in issue #17517

import language.experimental.captureChecking
import java.io.*

object Test:
class Logger(f: OutputStream^):
def log(msg: String): Unit = ???

def usingFile[sealed T](name: String, op: OutputStream^ => T): T =
val f = new FileOutputStream(name)
val result = op(f)
f.close()
result

def usingLogger[sealed T](f: OutputStream^)(op: Logger^{f} => T): T = ???

usingFile( // error
"foo",
file => {
usingLogger(file)(l => () => l.log("test"))
}
)
7 changes: 0 additions & 7 deletions tests/neg-custom-args/captures/usingLogFile.check
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,3 @@
| that type captures the root capability `cap`.
| This is often caused by a local capability in the body of method usingFile
| leaking as part of its result.
-- Error: tests/neg-custom-args/captures/usingLogFile.scala:72:6 -------------------------------------------------------
72 | usingLogger(_, l => () => l.log("test"))) // error
| ^^^^^^^^^^^
| Sealed type variable T cannot be instantiated to box () => Unit since
| that type captures the root capability `cap`.
| This is often caused by a local capability in the body of method usingLogger
| leaking as part of its result.
2 changes: 1 addition & 1 deletion tests/neg-custom-args/captures/usingLogFile.scala
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,5 @@ object Test4:

def test =
val later = usingFile("logfile", // error
usingLogger(_, l => () => l.log("test"))) // error
usingLogger(_, l => () => l.log("test"))) // ok, since we can widen `l` to `file` instead of to `cap`
later()
8 changes: 8 additions & 0 deletions tests/pos-custom-args/captures/cc-dep-param.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import language.experimental.captureChecking

trait Foo[T]
def test(): Unit =
val a: Foo[Int]^ = ???
val useA: () ->{a} Unit = ???
def foo[X](x: Foo[X]^, op: () ->{x} Unit): Unit = ???
foo(a, useA)

0 comments on commit f2f1156

Please sign in to comment.