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

Stacked Borrow: Barriers #553

Merged
merged 14 commits into from
Nov 30, 2018
Merged

Stacked Borrow: Barriers #553

merged 14 commits into from
Nov 30, 2018

Conversation

RalfJung
Copy link
Member

Waiting for a nightly to happen (seems like we skipped one?)

src/stacked_borrows.rs Outdated Show resolved Hide resolved
@@ -0,0 +1,13 @@
fn inner(x: *mut i32, _y: &i32) {
// If `x` and `y` alias, retagging is fine with this... but we really
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this simply invalidate y? Without the function call (so if inlined) this would suddenly be fine?

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't this simply invalidate y?

Yes. But without the barrier, invalidating y would be okay. Nobody is using y again, after all.

Copy link
Contributor

Choose a reason for hiding this comment

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

So why are we disallowing this code? y is unused, even in the main function.

Copy link
Member Author

@RalfJung RalfJung Nov 28, 2018

Choose a reason for hiding this comment

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

Because we want to enforce that a reference passed to a function is not invalidated while the function executes, no matter whether it is used or not. This is an operational version of "the reference outlives the function".

This can help optimizations that want to introduce new uses.

@RalfJung
Copy link
Member Author

Oh dang, this breaks the async-fn test. I will have to investigate, but I may not get to that today.

Co-Authored-By: RalfJung <post@ralfj.de>
@RalfJung RalfJung added the S-waiting-on-author Status: Waiting for the PR author to address review comments label Nov 28, 2018
@RalfJung
Copy link
Member Author

This works now locally.

@oli-obk did I resolve your questions?

@RalfJung
Copy link
Member Author

RalfJung commented Nov 28, 2018

Uh... it fails to build on Windows? Looks like a rustc bug?

EDIT: Reported as rust-lang/rust#56320

@RalfJung
Copy link
Member Author

Probably requires rust-lang/rust#56313 to land

@RalfJung RalfJung added S-blocked-on-rust Status: Blocked on landing a Rust PR S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Nov 28, 2018
@RalfJung RalfJung added S-waiting-on-nightly and removed S-waiting-on-review Status: Waiting for a review to complete S-blocked-on-rust Status: Blocked on landing a Rust PR labels Nov 28, 2018
@RalfJung RalfJung merged commit 8d2bc97 into master Nov 30, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Nov 30, 2018
update miri

This should make miri green again :)
(Includes rust-lang/miri#553)

r? @oli-obk
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Dec 1, 2018
update miri

This should make miri green again :)
(Includes rust-lang/miri#553)

r? @oli-obk
Centril added a commit to Centril/rust that referenced this pull request Dec 1, 2018
update miri

This should make miri green again :)
(Includes rust-lang/miri#553)

r? @oli-obk
Centril added a commit to Centril/rust that referenced this pull request Dec 2, 2018
update miri

This should make miri green again :)
(Includes rust-lang/miri#553)

r? @oli-obk
bors added a commit to rust-lang/rust that referenced this pull request Dec 3, 2018
update miri

This should make miri green again :)
(Includes rust-lang/miri#553)

r? @oli-obk
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.

2 participants