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

Add a scheme to find the place where an id was destroyed #1080

Merged
merged 8 commits into from
Dec 10, 2019

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Nov 28, 2019

cc #974

I'm not too happy with it, but since stacked borrows don't have access to the current call stack, I can't just report a warning as per #797

We could add some global mutex that we can throw strings at and step will clear out that mutex and report warnings before moving the statement_id or the block_id, not sure how well that would work. For now I think this is sufficient

@bors
Copy link
Contributor

bors commented Nov 29, 2019

☔ The latest upstream changes (presumably #1083) made this pull request unmergeable. Please resolve the merge conflicts.

src/eval.rs Outdated Show resolved Hide resolved
src/bin/miri.rs Outdated Show resolved Hide resolved
src/stacked_borrows.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member

What a hack! But it's not like I have a better idea that's even remotely as simple, so, the basic approach seems fine to me.

README.md Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member

RalfJung commented Dec 8, 2019

Uh... that's really nasty. And you're aborting in the middle of a Stacked Borrows operation. I don't think it is a good idea to go on executing -- as you said, that's basically a corrupt state, so any follow-up errors might be completely bogus.

I prefer your first scheme.

@RalfJung
Copy link
Member

RalfJung commented Dec 9, 2019

LGTM! r=me with the open suggestion applied.

Co-Authored-By: Ralf Jung <post@ralfj.de>
@oli-obk
Copy link
Contributor Author

oli-obk commented Dec 10, 2019

@bors r=RalfJung

@bors
Copy link
Contributor

bors commented Dec 10, 2019

📌 Commit 8d409a7 has been approved by RalfJung

bors added a commit that referenced this pull request Dec 10, 2019
Add a scheme to find the place where an id was destroyed

cc #974

I'm not too happy with it, but since stacked borrows don't have access to the current call stack, I can't just report a warning as per #797

We could add some global mutex that we can throw strings at and `step` will clear out that mutex and report warnings before moving the `statement_id` or the `block_id`, not sure how well that would work. For now I think this is sufficient
@bors
Copy link
Contributor

bors commented Dec 10, 2019

⌛ Testing commit 8d409a7 with merge f94bc71...

@bors
Copy link
Contributor

bors commented Dec 10, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: RalfJung
Pushing f94bc71 to master...

@bors bors merged commit 8d409a7 into master Dec 10, 2019
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.

4 participants