Skip to content

How subcapturing works for this is unsound #19315

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

Closed
Linyxus opened this issue Dec 20, 2023 · 0 comments · Fixed by #19323
Closed

How subcapturing works for this is unsound #19315

Linyxus opened this issue Dec 20, 2023 · 0 comments · Fixed by #19323
Assignees
Labels
cc-experiment Intended to be merged with cc-experiment branch on origin itype:bug
Milestone

Comments

@Linyxus
Copy link
Contributor

Linyxus commented Dec 20, 2023

Compiler version

main

Minimized code

import language.experimental.captureChecking
trait Logger
case class Boxed[T](unbox: T)

// a horrible function, but it typechecks
def magic(l: Logger^): Logger =
  class Foo:
    def foo: Boxed[Logger^{this}] =
      // for the following line to typecheck
      //   the capture checker assumes {l} <: {this}
      Boxed[Logger^{this}](l)
  val x = new Foo
  val y = x.foo.unbox  // y: Logger^{x}
  val z: Logger = y  // now the capability becomes pure
  z

Output

It typechecks.

Expectation

It should not typecheck. With magic one may make any capability pure.

Why it typechecks:

  • When typechecking Foo, we assume l being a subcapture of this.
  • The capture set of Foo is empty, as the reference to l is boxed.
  • The type of the method foo is Boxed[Logger^{this}].
  • When selecting x.foo, it is typed as Boxed[Logger^{x}].
  • We unbox it and get Logger^{x}, which is a subtype of Logger^{}.

/cc @odersky

@Linyxus Linyxus added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label cc-experiment Intended to be merged with cc-experiment branch on origin and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Dec 20, 2023
odersky added a commit that referenced this issue Jan 11, 2024
…adapted (#19323)

Fixes #19315.

The unsound code:
```scala
import language.experimental.captureChecking
trait Logger
case class Boxed[T](unbox: T)

// a horrible function, but it typechecks
def magic(l: Logger^): Logger =
  class Foo:
    def foo: Boxed[Logger^{this}] =
      // for the following line to typecheck
      //   the capture checker assumes {l} <: {this}
      Boxed[Logger^{this}](l)
  val x = new Foo
  val y = x.foo.unbox  // y: Logger^{x}
  val z: Logger = y  // now the capability becomes pure
  z
```

The `magic` function typechecks before this fix. It casts a capability
to a pure value, which is clearly unsound. The crux of the problem is
`Boxed[Logger^{this}](l)`, which relies on the subcapturing relation
`{l} <: {this}` enabled by the trick implemented `addOuterRefs`.

`addOuterRefs` augment a capture set that contains a self reference
(like `{this}`) with all outer references reachable from `this`. In this
case, the augmented set is `{this, l}`. The reasoning is that, any
captured outer references in the class can be rewritten into a field,
thus being a subcapture of `{this}`:
```
class Foo:
  val this_l: Logger^{l} = l
  def foo: Boxed[Logger^{this}] = Boxed(this.this_l)
```

But the problem with this unsound example is that, the reference to `l`
is boxed, so `l` is not captured by the class. Therefore, the above
rewriting mismatches with the actual situation.

This PR proposes a simple fix, which simply disable `addOuterRefs` when
box adaptation happens. This is of course imprecise, but all tests pass
with this fix.
@Kordyjan Kordyjan added this to the 3.4.1 milestone Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cc-experiment Intended to be merged with cc-experiment branch on origin itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants