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

Fix soundness bug introduced in Pony 0.51.2 #4283

Merged
merged 7 commits into from
Dec 29, 2022
Merged

Fix soundness bug introduced in Pony 0.51.2 #4283

merged 7 commits into from
Dec 29, 2022

Conversation

jasoncarr0
Copy link
Contributor

@jasoncarr0 jasoncarr0 commented Dec 28, 2022 •
edited
Loading

This fixes #4282
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.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Dec 28, 2022
@SeanTAllen SeanTAllen changed the title Fix issue 4282 Fix soundness bug introduced in Pony 0.51.2 Dec 29, 2022
@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Dec 29, 2022
@SeanTAllen
Copy link
Member

I pushed a fix to test formatting.

SeanTAllen
SeanTAllen reviewed Dec 29, 2022
View reviewed changes
Copy link
Member

@SeanTAllen SeanTAllen left a comment

Choose a reason for hiding this comment

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

I've fixed a couple formatting issues in commits I've added to this PR.

@SeanTAllen
Copy link
Member

If this passes CI after my couple of formatting fixes, I'll merge and do a release tomorrow.

@SeanTAllen SeanTAllen merged commit c7746df into main Dec 29, 2022
@SeanTAllen SeanTAllen deleted the fix-4282 branch December 29, 2022 03:46
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Dec 29, 2022
github-actions bot pushed a commit that referenced this pull request Dec 29, 2022
@ponylang-main
Update CHANGELOG for PR #4283
74b5a9e
github-actions bot pushed a commit that referenced this pull request Dec 29, 2022
@ponylang-main
Updates release notes for PR #4283
235d459
SeanTAllen added a commit that referenced this pull request Dec 30, 2022
@SeanTAllen
Fix compiler crash introduced in #4283
0381955 
Fixes a missed case in #4283 (arrow types). Instead of displaying
an error message, the user who was attempting to do an unsafe
mutation of a val was greeted by a compiler crash.

Fixes #4287
@SeanTAllen SeanTAllen mentioned this pull request Dec 30, 2022
Merged
SeanTAllen added a commit that referenced this pull request Dec 30, 2022
@SeanTAllen
Fix compiler crash introduced in #4283 (#4288)
2d33583 
Fixes a missed case in #4283 (arrow types). Instead of displaying
an error message, the user who was attempting to do an unsafe
mutation of a val was greeted by a compiler crash.

Fixes #4287
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SeanTAllen SeanTAllen SeanTAllen left review comments

Assignees
No one assigned
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Direct writing to a field of a val object
3 participants
@jasoncarr0 @SeanTAllen @ponylang-main