Skip to content

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Sep 5, 2025

  • Propagate expected type into blocks

    The level checking should do the proper avoidance so that local symbols
    would not leak into type variables of the expected type.

  • Don't automatically add a FreshCap owner to its hidden set

  • Don't use span capture sets for function types as the underlying set

Based on #23874

These problems were discovered once we started recording uses of this references.
The recording is not yet part of this commit because it requires downstream bugfixes
in the compiler.
These fell through the cracks before since we only considered named outer refs.
But inner classes can have outer this references check needd to be tracked in
use sets.
Previously there was a boundary condition where this could be the case for outermost
classes.
A constructor should never capture `this`, the object it constructs.
Was shockingly missing before.
 - Charge the use set of the initializer to the class constructor
 - Charge the declared capture set to the class
The level checking should do the proper avoidance so that local symbols
would not leak into type variables of the expected type.
@odersky odersky requested a review from a team as a code owner September 5, 2025 17:00
This can lead to unsoundness. For instance:

    class C extends SharedCapability
    val b: C => C

Then `b` is the owner of the FreshCap implicitly added to the result C,
yet it should not be added to the hidden set, since `b` itself is not
a SharedCapability.
What is the underlying capture set of a function? CC says the capture set
on the first arrow but we experimented with the capture of the span of the
function instead. This however causes a lot of complications if we want to push
it to a logical conclusion. So we now make it configurable and by default
span capture set is not used.
@odersky odersky requested review from noti0na1 and Linyxus and removed request for noti0na1 September 6, 2025 18:08
@odersky odersky assigned Linyxus and unassigned noti0na1 Sep 6, 2025
@odersky odersky changed the title CC: Changes around improveVAR and the apply rule CC: Various fixes and simplifications Sep 6, 2025
Copy link
Contributor

@Linyxus Linyxus left a comment

Choose a reason for hiding this comment

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

The changes looks great to me!

Though I believe the original issue remains unsound. A mini example:

import language.experimental.captureChecking
import caps.*
class Console extends SharedCapability:
  def println(msg: Any): Unit = ()
class C1:
  this: C1^ =>
  val console: Console^ = Console()
  val foo: () ->{this.console} Unit = () =>
    console.println(42)
def expect(x: C1^)(op: () ->{x.console} Unit): () ->{x} Unit = op
def test(whatever: () => Unit): Unit =
  val c1: C1^{} = C1()
  val pureFoo: () -> Unit = expect(c1)(c1.foo)
  // ^^^ the same console effect, but pure!

The problem is that although this.console is properly tracked inside the class, but the class that creates and uses the console capability (class C1) remains pure. Then, with a simple trick, we can treat an operation with I/O effect as pure (the pureFoo line).

But maybe this can be addressed in a follow-up PR? I am approving this one for now.

@odersky
Copy link
Contributor Author

odersky commented Sep 8, 2025

Yes, we are not there yet. There will be a follow-up PR to address the following:

  • Currently markFree treats FreshCaps generated by fields as non-visible since they are contained in the class symbol. So they are dropped from the capset of the class. We need to treat them as visible, except for FreshCaps in parameters, which are already handled in augmentConstructor.
  • We further need to convert field FreshCaps to ResultCaps in the return type of the constructor, so that they are freshly instantiated on each call.

@odersky odersky merged commit 7cbadac into scala:main Sep 8, 2025
44 checks passed
@odersky odersky deleted the change-improve branch September 8, 2025 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants