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

retag return place #1330

Merged
merged 3 commits into from
Apr 15, 2020
Merged

retag return place #1330

merged 3 commits into from
Apr 15, 2020

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Apr 13, 2020

@eddyb suggested that return places should be treated like unique references for Stacked Borrows. That is implemented by this patch, but it is unfortunately quite the hack because otherwise we are retagging references, not places.

@eddyb does this roughly correspond to what you had in mind? (Except for whatever it is you think should happen with argument passing, which is a much bigger issue.) Also, do you think there is any way we can test this?

Needs rust-lang/rust#71100 to land.

@RalfJung RalfJung added the S-blocked-on-rust Status: Blocked on landing a Rust PR label Apr 13, 2020
@eddyb
Copy link
Member

eddyb commented Apr 13, 2020

Also, do you think there is any way we can test this?

You want to a raw pointer to some place (so that borrowck doesn't catch it) then place = foo().
But it might not be possible to make the call terminator not use a temporary, without NRVO.

@RalfJung
Copy link
Member Author

Yeah, this creates a temporary:

fn foo(_ptr: *mut i32) -> i32 { 4 }

fn main() {
    let mut ret = 0i32;
    let ptr = &mut ret as *mut _;
    ret = foo(ptr);
}

@eddyb
Copy link
Member

eddyb commented Apr 13, 2020

I'm not entirely sure why we do that, don't we make temporaries for all the non-constant args?
Feels like overkill. cc @nikomatsakis

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 14, 2020
Centril added a commit to Centril/rust that referenced this pull request Apr 15, 2020
Centril added a commit to Centril/rust that referenced this pull request Apr 15, 2020
@RalfJung
Copy link
Member Author

@bors r+

@bors
Copy link
Contributor

bors commented Apr 15, 2020

📌 Commit 3548dcf has been approved by RalfJung

@bors
Copy link
Contributor

bors commented Apr 15, 2020

⌛ Testing commit 3548dcf with merge fbea3e5...

@bors
Copy link
Contributor

bors commented Apr 15, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked-on-rust Status: Blocked on landing a Rust PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants