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

Separation checking for product types #22539

Open
wants to merge 93 commits into
base: main
Choose a base branch
from

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Feb 6, 2025

This is based on #22588 and contains changes to make product types containing fresh entities work (at least in simple cases). There's a neat combination with both path capabilities and reach capabilities which makes this work.

In short, if we have

class Ref { var current: Int }
case class Pair[+A, +B](fst: A, snd: B)

then we can form a Pair[Ref^, Ref^]:

val r1 = Ref()
val r2 = Ref()
val p_exact: Pair[Ref^{r1}, Ref^{r2}] = Pair(r1, r2)
val p:  Pair[Ref^, Ref^] = p_exact

And if p has this type, then we can also take it apart:

val x = p.fst
val y = p.snd

With types added, this is

val x: Ref^{p.fst*} = p.fst
val y: Ref^{p.snd*} = p.snd

That is, we can get at the position of the boxed first element with p.fst and we can access what's in the box via p.fst*.
Furthermore, p.fst* is known to be separate from p.snd*, so this means we can form another pair from them:

val p2: Pair[Ref^, Ref^] = Pair(x, y)

In short, separation is preserved by forming products and taking them apart. It took quite a bit of work to get there.

@odersky odersky changed the title Sepchecks data handling Separation checking for product types Feb 6, 2025
@hamzaremmal
Copy link
Member

@odersky CI will fail because of the timeouts here too. Revert seems the fastest solution to unblock this situation.

@odersky odersky force-pushed the sepchecks-data-handling branch 6 times, most recently from 4e04b19 to a4b91bd Compare February 13, 2025 19:00
@odersky
Copy link
Contributor Author

odersky commented Feb 16, 2025

This is built on top of #22588. So CI failures due to timeout are the same.

@odersky odersky force-pushed the sepchecks-data-handling branch from 32b6c37 to 40a1bd3 Compare February 16, 2025 12:26
@Linyxus
Copy link
Contributor

Linyxus commented Feb 19, 2025

Some fresh soundness holes!

import language.experimental.captureChecking
import caps.*

class Ref[T](init: T) extends Mutable:
  private var value: T = init
  def get: T = value
  mut def set(newValue: T): Unit = value = newValue

// a library function that assumes that a and b MUST BE separate
def swap[T](a: Ref[Int]^, b: Ref[Int]^): Unit = ???

def test1(): Unit =
  val a: Ref[Int]^ = Ref(0)
  val foo: (x: Ref[Int]^) -> (y: Ref[Int]^{a}) ->{x} Unit =
    x => y => swap(x, y)
  foo(a)(a)  // OH NO

def test2(): Unit =
  val a: Ref[Int]^ = Ref(0)
  val f: (Ref[Int]^, Ref[Int]^) -> Unit = swap
  val g: (Ref[Int]^{a}, Ref[Int]^{a}) -> Unit = f
  g(a, a)  // OH NO

@Linyxus
Copy link
Contributor

Linyxus commented Feb 19, 2025

One more unsound snippet:

import language.experimental.captureChecking
import caps.*

class Ref[T](init: T) extends Mutable:
  private var value: T = init
  def get: T = value
  mut def set(newValue: T): Unit = value = newValue

// a library function that assumes that a and b MUST BE separate
def swap(a: Ref[Int]^, b: Ref[Int]^): Unit = ???

case class Foo[T](bar: Ref[T]^)

def foo(x: Foo[Int]^): Unit =
  val t1: Ref[Int]^{x.bar} = x.bar
  swap(t1, t1)  // no error, OH NO

  // sanity check
  val t2: Ref[Int]^ = Ref(0)
  val t3: Ref[Int]^{t2} = t2
  swap(t3, t3)  // error, as expected

@odersky
Copy link
Contributor Author

odersky commented Feb 20, 2025

@Linyxus The first hole is plugged now.

@odersky odersky force-pushed the sepchecks-data-handling branch from 2cdc7ce to 9e34caa Compare February 20, 2025 18:12
@Linyxus
Copy link
Contributor

Linyxus commented Feb 25, 2025

One soundness issue related to @consume:

import language.experimental.captureChecking
import caps.*

class Ref extends Mutable:
  private var _data = 0
  def get: Int = _data
  mut def put(x: Int): Unit = _data = x

// require f and g to be non-interfering
def par(f: () => Unit, g: () => Unit): Unit = ???

def bad(@consume op: () ->{cap.rd} Unit): () => Unit = op

def test2(@consume x: Ref^): Unit =
  val f: () ->{x.rd} Unit = () => x.get
  val rx: () => Unit = bad(f)  // hides x.rd in the resulting `cap`
  par(rx, () => x.put(42))  // data race!

This compiles but should not. And it has data races.
I was expecting @consume has some sort of "killing" effect. For instance, the function call on the last but two line bad(f) hides x.rd in a consume parameter. This should disable x.rd (and therefore x) afterwards when the function call finishes.

When printing a type `C^` where `C` extends `Capability`, don't show the `^`.
This is overridden under -Yprint-debug.
 - Add Mutable trait and mut modifier.
 - Add dedicated tests `isMutableVar` and `isMutableVarOrAccessor`
   so that update methods can share the same flag `Mutable` with mutable vars.
 - Disallow update methods overriding normal methods
 - Disallow update methods which are not members of classes extending Mutable
 - Add design document from papers repo to docs/internals
 - Add readOnly capabilities
 - Implement raeadOnly access
 - Check that update methods are only called on references with exclusive capture sets.
 - Use cap.rd as default capture set of Capability subtypes
 - Make Mutable a Capability, this means Mutable class references get {cap.rd} as
   default capture set.
 - Use {cap} as captu
….toCap

If existentials are mapped to fresh, it matters where they are opened. Pure or not
arguments don't have anything to do with that.
These are represented as Fresh.Cap(hidden) where hidden is the set of
capabilities subsumed by a fresh. The underlying representation is as
an annotated type `T @annotation.internal.freshCapability`.

Require -source `3.7` for caps to be converted to Fresh.Cap

Also:

 - Refacture and document CaputureSet
 - Make SimpleIdentitySets showable
 - Refactor VarState
    - Drop Frozen enum
    - Make VarState subclasses inner classes of companion object
    - Rename them
    - Give implicit parameter VarState of subCapture method a default value
 - Fix printing of capturesets containing cap and some other capability
 - Revise handing of @uncheckedAnnotation
Check separation from source 3.7 on.

We currently only check applications, other areas of separation checking
are still to be implemented.
Check that a capability that gets hidden in the (result-)type of some
definition is not used afterwards in the same or a nested scope.
When checking whether two items overlap we should always check their deep capture sets.
Buried aliases should count as well.
This is necessary since capability sets are IdentitySets.
TODO:

 - check that only @consume parameters flow to @consume parameters
odersky added 2 commits March 5, 2025 15:53
Also, implement infrastructure to add a note to a type mismatch when a failure
to subsume occurs.
Allows some simplifications in the zoo of classification methods.
odersky added 7 commits March 7, 2025 18:33
Allows some simplifications in the zoo of classification methods.
Exceptions are bracketed in `withCapAsRoot` calls.
TODO: Check that the new error in box-adapt-contra makes sense
TODO: error messages need to be improved
@odersky
Copy link
Contributor Author

odersky commented Mar 7, 2025

Second soundness hold related to contravariant cap is now also fixed

@bracevac bracevac self-requested a review March 8, 2025 11:33
 1. Strip readonly modifier before checking o=for overlaps
 2. Don't reset consume to empty before entering defs
@odersky
Copy link
Contributor Author

odersky commented Mar 8, 2025

Soundness issue related to @consume is fixed now.

odersky added 3 commits March 9, 2025 12:53
I believe the timeout was simply that we have to many pos tests, so theior compilation takes more than the
limit of 20 minutes. Testing this hypothesis by moving pos-custumargs/captures tests into a separate
top-level test posCC
Deduct explicit refs of actual as opposed to formal argument type.
Move extension methods to SepChecks object, drop unused ones.
 - Make it peak-based instead of footprint-based
 - Don't flag rd/rd conflicts between consumed and re-used
@odersky odersky force-pushed the sepchecks-data-handling branch from e2234bf to 48b7c09 Compare March 10, 2025 22:06
@odersky odersky force-pushed the sepchecks-data-handling branch from 48b7c09 to 6b5d900 Compare March 10, 2025 22:29
@odersky odersky force-pushed the sepchecks-data-handling branch from cce8509 to c1f44c4 Compare March 13, 2025 09:58
Intersections used the heuristic mightAccountFor instead of the precise accountsFor. Thsi
can lead to a loss of precision and (if unchecked afterwards) also soundness. The fix caused
some tests to fail, which involved tracked parameters.

We now deal with tracked parameters in the same way as parameters that carry a @refineOverride annotation.
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