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

rooting of boxes in borrowck fails to take moves into account #3053

Closed
nikomatsakis opened this issue Jul 30, 2012 · 10 comments
Closed

rooting of boxes in borrowck fails to take moves into account #3053

nikomatsakis opened this issue Jul 30, 2012 · 10 comments
Assignees
Labels
A-lifetimes Area: lifetime related I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.

Comments

@nikomatsakis
Copy link
Contributor

The following code:

// exec-env:RUST_POISON_ON_FREE=1

fn free<T>(-x: @T) {}

fn lend(+x: @{f: @{g: int}}) -> int {
    let y = &x.f.g;
    free(x);
    *y
}

fn main() {
    assert lend(@{f: @{g: 22}}) == 22;
}

is handled incorrectly by borrowck right now. It will opt not to root the x.f box even though it ought to. This is because it sees that x.f is found in immutable data which will outlive the lifetime of the borrow: but it doesn't consider moves. To be correct, it should either (1) know which variables might be moved (liveness can tell us) and conservatively root boxes that are found in those variables or (2) take out a loan on x so that an error would be reported. But (2) seems suboptimal since this is not really an error, just a failure of borrowck to make the right call on whether to root x.f.

@ghost ghost assigned nikomatsakis Aug 2, 2012
@nikomatsakis
Copy link
Contributor Author

I have a fix for this but it will not make it to 0.6

@Aatch
Copy link
Contributor

Aatch commented Jun 5, 2013

@nikomatsakis is this fixed? There have been a lot of borrowck changes recently.

Also, that example is so old I'm not even sure how to modernize it...

@catamorphism
Copy link
Contributor

@nikomatsakis , what's the status of this?

@nikomatsakis
Copy link
Contributor Author

I think I fixed this :) let me see if I added a test

nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Nov 12, 2013
@nikomatsakis
Copy link
Contributor Author

Just added a PR with a test, since I didn't see one. I violated rule #1 of test writing though in that I did not observe a test failure; it'd be a pain to do so, but I did inspect code and generated LLVM to make sure that we're creating the right pattern here.

@nikomatsakis
Copy link
Contributor Author

for reference, here is the code that attempts to fix this scenario: https://github.com/mozilla/rust/blob/master/src/librustc/middle/borrowck/gather_loans/lifetime.rs#L96

@huonw
Copy link
Member

huonw commented Nov 12, 2013

Could @cmr use one of his many old rustc's to check it fails? (Does he only need to go back before #10153?)

@emberian
Copy link
Member

I don't see a failure with d2cf295

@huonw
Copy link
Member

huonw commented Nov 13, 2013

Blame suggests this was fixed by a896440 (2013-03-15), I guess you don't go back that far, @cmr?

@emberian
Copy link
Member

I don't have back that far, unfortunately.

On Tue, Nov 12, 2013 at 8:06 PM, Huon Wilson notifications@git.luolix.topwrote:

Blame suggests this was fixed by a896440a896440c(2013-03-15), I guess you don't go back that far,
@cmr https://github.com/cmr?


Reply to this email directly or view it on GitHubhttps://github.com//issues/3053#issuecomment-28351795
.

RalfJung pushed a commit to RalfJung/rust that referenced this issue Sep 12, 2023
celinval pushed a commit to celinval/rust-dev that referenced this issue Jun 4, 2024
Use dependabot to update the `s2n-quic` submodule weekly (every Monday).
This is to avoid having to do this effort manually, and to guarantee
that none of the `s2n-quic` proofs break due to changes in Kani.

I've tested this in my own fork, and it seemed to work correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: lifetime related I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Projects
None yet
Development

No branches or pull requests

5 participants