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

Ephemeral RefCap shouldn't be an assignable refcap type #3931

Closed
redvers opened this issue Dec 12, 2021 · 2 comments · Fixed by #3953 or #4018
Closed

Ephemeral RefCap shouldn't be an assignable refcap type #3931

redvers opened this issue Dec 12, 2021 · 2 comments · Fixed by #3953 or #4018
Assignees
Labels
bug Something isn't working triggers release Major issue that when fixed, results in an "emergency" release

Comments

@redvers
Copy link
Contributor

redvers commented Dec 12, 2021

This allows modification of val and sending of mutable data to actors.

Example:

use "debug"
use "files"
use "collections"

actor Main
  var env: Env

  new create(env': Env) =>
    env = env'

    let map: Map[String, Array[String]] trn^ = recover trn Map[String, Array[String]] end
    map.insert("start", ["A" ; "b"])
    map.insert("A", ["end"])

    let romap: Map[String, Array[String]] val = map
    let foo: Foo = Foo(env, map)
    foo.test()

//    map.insert("X", ["c"; "b"; "end"; "start"])

actor Foo
  let env: Env
  let map: Map[String, Array[String]] val
  new create(env': Env, map': Map[String, Array[String]] val) =>
    env = env'
    map = map'

  be test() =>
    env.out.print("Size of map in actor: " + map.size().string())
@SeanTAllen SeanTAllen added bug Something isn't working help wanted Extra attention is needed triggers release Major issue that when fixed, results in an "emergency" release labels Dec 13, 2021
@jasoncarr0
Copy link
Contributor

jasoncarr0 commented Dec 13, 2021

A few extra cases for the fix:

  • Unions: (Example iso^ | None)

  • Intersections: A ref & A val already has a more aggressive check, but there could be other cases.
    (Side note: If this check is implemented, there's an argument that we should technically only enforce the ref & val check for variable bindings (as both A trn^ or A iso^ are already subtypes of A val & A ref by definition))

  • Tuples (A iso^, B ref)

In other words, the test for capabilities has to be deep, not just checking a nominal cap

Some day also generics, but that has its own issue
#1931

@SeanTAllen SeanTAllen removed the help wanted Extra attention is needed label Dec 14, 2021
SeanTAllen pushed a commit that referenced this issue Feb 15, 2022
This PR introduces checks around variables and other unaliasing points, to reject programs which attempt to create variables whose capabilities are inherently unsound. The checks about compatability of intersections have been moved to this check after investigation of the origin, as it is not needed for soundness, but this can be reinstated in the original location if helpful for engineering purposes.

Fixes #3931

Some generic code may still produce unsoundness after instantiating, but this change is hopefully a step-forwarding to introducing the necessary constraints.
@SeanTAllen SeanTAllen reopened this Feb 16, 2022
@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Feb 16, 2022
@SeanTAllen
Copy link
Member

I reopened this because the PR that closed it needs to be reverted for now.

SeanTAllen added a commit that referenced this issue Mar 8, 2022
This PR introduces checks around variables and other unaliasing points, to reject programs which attempt to create variables whose capabilities are inherently unsound. The checks about compatibility of intersections have been moved to this check after investigation of the origin, as it is not needed for soundness, but this can be reinstated in the original location if helpful for engineering purposes.

Fixes #3931

Some generic code may still produce unsoundness after instantiating, but this change is hopefully a step-forwarding to introducing the necessary constraints.
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triggers release Major issue that when fixed, results in an "emergency" release
Projects
None yet
4 participants