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

Improve lifetime error messages re: block suffixes and dropck #21875

Closed
main-- opened this issue Feb 2, 2015 · 13 comments
Closed

Improve lifetime error messages re: block suffixes and dropck #21875

main-- opened this issue Feb 2, 2015 · 13 comments
Labels
A-type-system Area: Type system

Comments

@main--
Copy link
Contributor

main-- commented Feb 2, 2015

fn main() {
    let foo;
    let bar = 1337;
    foo = &bar;
}
$ rustc lifetime.rs 
lifetime.rs:4:12: 4:15 error: `bar` does not live long enough
lifetime.rs:4     foo = &bar;
                         ^~~
lifetime.rs:2:12: 5:2 note: reference must be valid for the block suffix following statement 0 at 2:11...
lifetime.rs:2     let foo;
lifetime.rs:3     let bar = 1337;
lifetime.rs:4     foo = &bar;
lifetime.rs:5 }
lifetime.rs:3:19: 5:2 note: ...but borrowed value is only valid for the block suffix following statement 1 at 3:18
lifetime.rs:3     let bar = 1337;
lifetime.rs:4     foo = &bar;
lifetime.rs:5 }
error: aborting due to previous error

foo and bar go out of scope simultaneously, so the error can only refer to the fact that bar's lifetime begins after foo's lifetime. But that still doesn't make sense, as bar is alive when the reference to it is assigned to foo.

I'm using the nightly builds from the Ubuntu PPA:

$ rustc --version --verbose
rustc 1.0.0-dev
binary: rustc
commit-hash: unknown
commit-date: unknown
host: x86_64-unknown-linux-gnu
release: 1.0.0-dev
@sinistersnare
Copy link
Contributor

playpen link

The error shows exactly what is wrong in black and white...

They may go out of scope at the same time, but the lifetime of bar is shorter than foo, and that is exactly what the error says. Maybe in the future this could work, but I feel like the error is perfectly clear in its message?

I think this has to do with the fact that lifetimes are lexical, which is not always what we want, but it is the way it is currently.

@kmcallister kmcallister added the A-type-system Area: Type system label Feb 3, 2015
@kmcallister kmcallister changed the title Confusing lifetime error Reference lifetime requirement should only extend back to first initialization Feb 3, 2015
@jimblandy
Copy link
Contributor

The problem here is that the old code used to compile, as of a few weeks ago - the set of permitted programs shrank. It certainly would be nice if programs like this were allowed.

@aidanhs
Copy link
Member

aidanhs commented Feb 11, 2015

the lifetime of bar is shorter than foo, and that is exactly what the error says

The message "does not live long enough" (from an English-language point of view) strictly means "the lifetime of the variable must be extended to cover more of the program", which is fairly non-specific. Having used rust a little since mid-Dec, my interpretation is "the extension of the lifetime should probably happen at the end, rust only complains when things don't live long enough".

I've honestly no idea how I'd pick up that foo starting earlier than bar is a deal breaker without googling the error and finding this issue and http://users.rust-lang.org/t/why-how-lifetime-works-has-changed/256

@jimblandy
Copy link
Contributor

Okay; it seems like this is a consequence of #21657, as aidanhs's link suggests. I no longer think this is a bug; it seems that this is the way Rust must behave.

Issue #21657 changes the compiler to track lifetimes more carefully. For example, in code like this:

{
    let x: i32;
    let y: i32;
    let z: i32;
}

the pre-change compiler would treat these all as having the same scope: the block that contains them. Post-change, Rust treats z as having a scope nested within that of y, which is in turn nested within the scope of x.

In the case of i32 values, this doesn't matter, but for Drop types, it matters a lot. Let's make things a bit more complicated:

struct Grr<'a> {
    r: &'a Box<i32>
}

impl<'a> Drop for Grr<'a> {
    fn drop(&mut self) {
        println!("dropping {}", *self.r);
    }
}

#[test]
fn good_borrow01() {
    let mut r;
    let i = box 10i32;
    r = Grr { r: &i };
    assert_eq!(**r.r, 10);
}

In this program, i and its box go out of scope first, and then r's drop will try to use the box's contents.

Note that you must sprinkle #[unsafe_destructor] all over the place to make this code compile, which seems exactly right: if this behavior is permitted, the language is not memory-safe.

@main--
Copy link
Contributor Author

main-- commented Feb 12, 2015

Yes, this is definitely caused by #21657. The HashMap example in there is how I initially stumbled upon this issue.

If I understood it correctly, my premise is already wrong:

foo and bar go out of scope simultaneously

Because as @jimblandy mentions, they really don't. Instead, the code compiles like this:

fn main() {
    let foo;
    {
        let bar = 1337;
        {
            foo = &bar;
        }
    }
}

And suddenly it all makes sense: Of course the &bar inside of foo outlives bar!

I can think of several solutions:

  • Because foo and bar are (in the original code) in the same scope, the compiler could be allowed to reorder when they go out of scope. However, that would obviously mean that the order of destruction is now undefined (but safe). Since you can always create scopes manually if you really need determinism here, that doesn't feel like a huge sacrifice to me right now, but as not even C++ has this kind of undefined behavior, there's probably some additional badness associated with it that I can't think of right now.
  • There are no drop calls involved in this example, so reordering when foo and bar go out of scope can't have observable consequences. Therefore, the compiler could be allowed to reorder at least these trivial cases (and maybe additionally calls to "pure destructors" as proposed over at Add CodeExtent::Remainder variant; pre-req for new scoping/drop rules. #21657).
  • At least provide a more helpful error message. I imagine this can be quite hard to track down in bigger functions, especially because the compiler shows you two identical block suffixes that differ only by their start. This is really confusing, as it seems to suggest that the error is due to bar not yet being alive when foo is declared. It doesn't make sense at all on its own, but in the end, this really is what causes the error because Add CodeExtent::Remainder variant; pre-req for new scoping/drop rules. #21657 forces foo's scope to enclose bar's scope. So the block suffixes are indeed different, but rustc doesn't show that.

@main--
Copy link
Contributor Author

main-- commented Feb 12, 2015

Now things get weird:

fn main() {
    let foo: Box<&i32>;
    let bar = 1337;
    foo = Box::new(&bar);
    { foo; } // kill foo
    foo; // error expected, foo was moved
    bar; // no error, bar is still alive
}

As expected, bar is still alive in the last line and as expected, foo can no longer be accessed after the move. But I strongly doubt that the other error message should be there:

<anon>:4:21: 4:24 error: `bar` does not live long enough
<anon>:4     foo = Box::new(&bar);
                             ^~~
<anon>:2:23: 8:2 note: reference must be valid for the block suffix following statement 0 at 2:22...
<anon>:2     let foo: Box<&i32>;
<anon>:3     let bar = 1337;
<anon>:4     foo = Box::new(&bar);
<anon>:5     { foo; } // kill foo
<anon>:6     foo;
<anon>:7     bar;
         ...
<anon>:3:19: 8:2 note: ...but borrowed value is only valid for the block suffix following statement 1 at 3:18
<anon>:3     let bar = 1337;
<anon>:4     foo = Box::new(&bar);
<anon>:5     { foo; } // kill foo
<anon>:6     foo;
<anon>:7     bar;
<anon>:8 }
<anon>:6:5: 6:8 error: use of moved value: `foo`
<anon>:6     foo;
             ^~~
<anon>:5:7: 5:10 note: `foo` moved here because it has type `Box<&i32>`, which is non-copyable
<anon>:5     { foo; } // kill foo
               ^~~
error: aborting due to 2 previous errors

It's the same error again! However, the last two lines show that bar is alive when foo is no longer. With its output, rustc apparently contradicts itself.

@jimblandy
Copy link
Contributor

A variable being moved from is not the same as its going out of scope, though.

In any case, it looks to me as if this issue should be closed; the original behavior described is not a bug.

@main--
Copy link
Contributor Author

main-- commented Feb 13, 2015

It might not be a bug, but it's still far from perfect and could (should, in my opinion) be improved, e.g. in one of the ways I suggested above.

@pnkfelix
Copy link
Member

Your example in #21875 (comment) might get addressed eventually as part of #6393 ; at least, that is the only way I can see us encoding the knowledge that foo does not actually survive to the end of its declared scope.

And that change (#6393) may well happen at some point; but for now, we are coping with our more simplistic model of lifetimes.


I don't think we're going to automatically reorder bindings to satisfy dropck any time soon, maybe ever.

Pure destructors might happen, but I don't think they would actually address the example of #21875 (comment)

In the end, I suspect the best way to approach this ticket is via improvements to the error messages, especially when it comes to the fallout from #21657 (but there are other places where borrowck error messages can be improved).

@pnkfelix pnkfelix changed the title Reference lifetime requirement should only extend back to first initialization Improve lifetime error messages re: block suffixes and dropck Feb 14, 2015
@pnkfelix
Copy link
Member

cc #22321

@lastorset
Copy link
Contributor

It might be best to leave the precise error messages there: hearing about "block suffixes" gives an opportunity to learn the theory behind the borrow checking. But I agree more help is needed: for newcomers to Rust it's easy to misread the compiler's error, since they might not be thinking correctly about lifetimes. So I'd suggest adding a help: message tailored to the situation:

lifetime.rs:2 help: bind `bar` before `foo`

I'm only a beginner, so I might be overlooking cases of incorrect lifetimes where the solution is different.

The compiler already outputs similar messages for temporary variables, which is nice:

<anon>:4:5: 4:26 help: consider using a `let` binding to increase its lifetime
<anon>:4     foo = &String::new();
             ^~~~~~~~~~~~~~~~~~~~~

@Nemo157
Copy link
Member

Nemo157 commented Dec 22, 2016

This appears to have been improved at some point, trying the original code on the playground gives

rustc 1.13.0 (2c6933acc 2016-11-07)
error: `bar` does not live long enough
 --> <anon>:4:12
  |
4 |     foo = &bar;
  |            ^^^ does not live long enough
5 | }
  | - borrowed value dropped before borrower
  |
  = note: values in a scope are dropped in the opposite order they are created

error: aborting due to previous error

the block suffix part of the message is gone and a note is added explaining why it does not live long enough in this specific case.

@Mark-Simulacrum
Copy link
Member

I think the note is close enough that we can close this. Lifetime errors are always non-ideal and we have a lot of issues about improving them (I think, at least) and this one is specific enough that the note (values ... dropped in opposite order) solves it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system
Projects
None yet
Development

No branches or pull requests

9 participants