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.

Sign up for GitHub

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

Jump to bottom

Direct writing to a field of a val object #4282

Closed
redvers opened this issue Dec 28, 2022 · 2 comments · Fixed by #4283
Closed

Direct writing to a field of a val object #4282

redvers opened this issue Dec 28, 2022 · 2 comments · Fixed by #4283
Labels
help wanted Extra attention is needed needs investigation This needs to be looked into before its "ready for work" triggers release Major issue that when fixed, results in an "emergency" release

Comments

@redvers
Copy link
Contributor

redvers commented Dec 28, 2022

This is a "wicked bad bug"™

actor Main
  let env: Env
  new create(env': Env) =>
    env = env'
    let foo: Foo val = Foo
    env.out.print("Field: " + foo.field.string())
    foo.field = 42
    env.out.print("Field: " + foo.field.string())
    
    
class Foo
  var field: USize = 0
@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Dec 28, 2022
@SeanTAllen
Copy link
Member

This was broken in the 0.51.2 release. I'm not sure of the exact change but suspect from the CHANGELOG that #4124 would be the most likely culprit.

@SeanTAllen SeanTAllen added triggers release Major issue that when fixed, results in an "emergency" release help wanted Extra attention is needed needs investigation This needs to be looked into before its "ready for work" labels Dec 28, 2022
@SeanTAllen
Copy link
Member

When this is fixed, tests should be added to make sure this basic safety feature doesn't get broken again.

@jasoncarr0 jasoncarr0 mentioned this issue Dec 28, 2022
Merged
SeanTAllen pushed a commit that referenced this issue Dec 29, 2022
@jasoncarr0
Fix soundness bug introduced in Pony 0.51.2 (#4283)
c7746df 
This commit includes a fix for the soundness bug introduced in 0.51.2 and  adds a set of simple "obviously-correct" tests of soundness / some completeness.

The bug was introduced because the change to safe_to_move in c5113e7 was correct from an isolation standpoint, but not from the standpoint of checking mutability, which is what the operator code was using it for. I.e. the change was valid only for the case of auto-recovering a constructor, and not mutating an existing object.

Now, mutability is checked before checking safety. This allows a better error message for the user in addition to fixing the bug.

Fixes #4282
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Extra attention is needed needs investigation This needs to be looked into before its "ready for work" triggers release Major issue that when fixed, results in an "emergency" release
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix soundness bug introduced in Pony 0.51.2 ponylang/ponyc
3 participants
@SeanTAllen @redvers @ponylang-main