-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Orphan parameter while Ychecking with CC library #19661
Comments
nicolasstucki
added
itype:bug
cc-experiment
Intended to be merged with cc-experiment branch on origin
area:experimental:cc
Capture checking related
and removed
cc-experiment
Intended to be merged with cc-experiment branch on origin
labels
Feb 9, 2024
nicolasstucki
changed the title
Orphan parameter while Ychecking with CC library.
Orphan parameter while Ychecking with CC library
Feb 9, 2024
A minimisation that is independent of the capture checked library: import language.experimental.captureChecking
trait MySet[A]:
def collect[B](pf: PartialFunction[A, B]^): MySet[B]^{this, pf}
class Test:
def f(xs: MySet[Int]) = xs collect { case x => x }
def g(xs: MySet[Int]): MySet[Int] = f(xs) |
odersky
added a commit
that referenced
this issue
Feb 15, 2024
…19684) Fixes #19661. ## Cause of the issue As reported in #19661, the following code triggers an assertation failure during Ycheck: ```scala import language.experimental.captureChecking trait MySet[A]: def collect[B](pf: PartialFunction[A, B]^): MySet[B]^{this, pf} class Test: def f(xs: MySet[Int]) = xs collect { case x => x } def g(xs: MySet[Int]): MySet[Int] = f(xs) ``` The failure happens when checking the tree `f(xs)`, whose type is `MySet[Int]^{this, PartialFunction[Int, Int]}`. The `checkNoOrphans` function is invoked on `this`, whose type turns out to be an orphan parameter reference (`xs`). We first inspect the tree outputed by `typer`: ```scala class Test() extends Object() { def f(xs: MySet[Int]): MySet[Int]^{this, PartialFunction[Int, Int]} = xs.collect[Int]( { def $anonfun(x$1: Int): Int = x$1 match { case x @ _ => x:Int } closure($anonfun:PartialFunction[Int, Int]) } ) def g(xs: MySet[Int]): MySet[Int] = this.f(xs) } ``` The problem roots in the signature of the method `f`: in the capture set of its result type, the `this` reference is dangling. How come? It turns out that the `asSeenFrom` map is not working correctly for the typing of `xs.collect`: ``` (xs.collect : [B](pf: PartialFunction[Int, B]^): MySet[B]^{this, pf}) ``` Instead of replacing `this` with `xs`, `asSeenFrom` keeps `this` untouched. This is what happened: - When mapping `asSeenFrom` on the method type, the `asSeenFrom` map recurses and applies on the annotated type. - When mapping the annotation (`@retains(this, pf)`), the `asSeenFrom` map derives a `TreeTypeMap` from itself and uses it to map the `tree` of the annotation. - During that, the type of `this` is properly mapped to `xs.type` but the tree `this` is never changed (since the `TreeTypeMap` is an identity on the structure of trees). To solve this issue, there are (at least) two possibilities: - Refactor the `TypeMap` machineries on annotations to enable it to properly handle these cases. But it is hard: when mapping the capture annotation, we are at a pre-CC phase, so tools for manipulating capture sets are not available. And it is unnecessary: even if we compute these references properly, it gets discarded during CC. - During Ycheck, ignore orphan parameter references inside a normal `@retains` annotation (as opposed to an optimised `CaptureAnnotation`). This feels like a dangerous fix but these `@retains` annotations, even if they are ill-formed, is already treated as being unreliable in CC and get rechecked. Also, CC turns these concrete annotations into optimised `CaptureAnnotation`s, which are not ignored by Ycheck. ## Fix So this PR implements the second option: - Ignore orphan parameter errors inside a normal `@retains` annotation during Ycheck. - The check for `CaptureAnnotation`s will not be bypassed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Minimization of
tests/pos/t6925.scala
Originally posted by @nicolasstucki in #19652 (comment)
The text was updated successfully, but these errors were encountered: