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

Lifetimes: Shouldn't the compiler figure out and prolong lifetimes of temporary values where it makes sense? #49118

Closed
axos88 opened this issue Mar 17, 2018 · 9 comments
Labels
A-NLL Area: Non-lexical lifetimes (NLL) C-enhancement Category: An issue proposing an enhancement or a PR with one. NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@axos88
Copy link

axos88 commented Mar 17, 2018

Reading the book currently, and could not really grasp why it's not possible to create a cons list without Box<>, so I tried it...

The results were... not satisfactory for me... Shouldn't the compiler be smart enough and prolong the lifetimes of the temporary values in case of "error"? Or is that a not-as-easy-as-it-sounds problem?

I'm pretty sure there are other real-life scenarios where I wouldn't want to name all the intermediary values...

Btw, shouldn't the first case fail to compile as well, since the lifetime of the shadowed variable is supposed to end when it gets shadowed, doesn't it?

enum List<'a, T: Copy + 'a> {
    Nil,
    Con(T, &'a List<'a, T>)
}

impl<'a, T: Copy + 'a> List<'a, T> {
    fn push(&'a self, value: T ) -> List<'a, T> {
        List::Con(value, self)
    }
}


fn main() {
    let works = List::Nil;
    let works = works.push(7);
    let works = works.push(5);
    let works = works.push(3);

    // temporary value does not live long enough
    let error = List::Nil
        .push(7)
        .push(5)
        .push(3);
}

error[E0597]: borrowed value does not live long enough
  --> src/main.rs:19:17
   |
19 |       let error = List::Nil
   |  _________________^
20 | |         .push(7)
21 | |         .push(5)
   | |________________^ temporary value does not live long enough
22 |           .push(3);
   |                   - temporary value dropped here while still borrowed
23 |   }
   |   - temporary value needs to live until here
   |
   = note: consider using a `let` binding to increase its lifetime
@axos88
Copy link
Author

axos88 commented Mar 17, 2018

Hmm, I have a hunch the problem arises when the value is passed around, since there is no way to keep the values then. Still would like to see this working within the same function though.

@sapphire-arches
Copy link
Contributor

You're right that the compiler should be able to do this. It doesn't work on stable today due to the way AST borrowck works, but your example will happily compile on nightly with non-lexical lifetimes =). I'll add some tags so we can keep track of the issue, but I believe this is considered fixed.

@sapphire-arches sapphire-arches added A-NLL Area: Non-lexical lifetimes (NLL) NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. labels Mar 18, 2018
@sinkuu
Copy link
Contributor

sinkuu commented Mar 19, 2018

@axos88

Btw, shouldn't the first case fail to compile as well, since the lifetime of the shadowed variable is supposed to end when it gets shadowed, doesn't it?

No, shadowed variables are dropped at the end of the enclosing block, as usual. Shadowing syntactically makes the old variable inaccessible, but has no semantic difference if I understand correctly.

@axos88
Copy link
Author

axos88 commented Mar 23, 2018

@bobtwinkles I believe the fix is not working for all cases. I would expect the compiler to also be able to infer that it has to extend the live of the MutexGuard for as long as there is a reference to whatever is inside:

https://play.rust-lang.org/?gist=3af98ef6bf9a631eaf0185f90e8f1183&version=nightly

@pnkfelix
Copy link
Member

pnkfelix commented May 30, 2018

@axos88 asked:

Shouldn't the compiler be smart enough and prolong the lifetimes of the temporary values in case of "error"? Or is that a not-as-easy-as-it-sounds problem?

The compiler could be smart enough to do what you describe, but that does not mean that it should do so.

The main argument that I have against such a change is that I want the user to be able to predict when a value will be dropped based on how the value itself is moved around, and not on how long its borrows survive.

That argument's foundation rests upon my personal hypothesis: when thinking about when destructors run, it is significant easier for developers to reason solely about how values are moved around (and initialized and overwritten) than it would be for developers to also reason about what lifetimes the compiler infers for any borrows of those values.

  • I don't have evidence to present to support this hypothesis. It is largely a gut feeling.

@pnkfelix
Copy link
Member

Having said that, I think @nikomatsakis has at least one, and perhaps a whole series, of blog posts on this topic:

http://smallcultfollowing.com/babysteps/blog/2012/09/15/rvalue-lifetimes/

http://smallcultfollowing.com/babysteps/blog/2014/01/09/rvalue-lifetimes-in-rust/

see also: #3511
and #11585

@axos88
Copy link
Author

axos88 commented Jun 21, 2018

This was quite some time ago, so i'm not 100% sure, but as far as i remember there is no borrowing happening there, everything is moved

@axos88
Copy link
Author

axos88 commented Jun 21, 2018

@pnkfelix

@XAMPPRocky XAMPPRocky added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-lang Relevant to the language team, which will review and decide on the PR/issue. WG-compiler-nll labels Jun 26, 2018
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jul 3, 2018

We do not want to prolong the lifetimes of temporaries arbitarily. The underlying principle is that the runtime semantics of your code should not depend on borrow-check -- that is, you should not need to understand region inference to understand when a destructor will run. It is this precise principle that allows us to increase the precision of the system (e.g., do NLL) without it being a breaking change (since otherwise that could affect when destructors run, etc)

There are some proposals to do better than we currently do when it comes to temporary lifetimes though. One such proposal is RFC #66, which was never fully implemented but would be backwards compatible. That RFC basically needs to be rewritten to be made actionable, I fear: I was in the midst of doing that but never finished. Anyway, the tracking issue for that is #15023.

It might also be nice to revamp some of the current temporary lifetimes in non-backwards-compatible ways, specifically around the tails of expressions. There are some issues relating to some of the wacky cases that arise but I don't have time to hunt them down right now.

In any case, I'm going to close this issue as the precise remedy it describes is not I think the direction we want to go. Thanks @axos88 for the bug report. =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL) C-enhancement Category: An issue proposing an enhancement or a PR with one. NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants