Temporary lifetimes sometimes yield surprising errors

[stephaneyfx]

UPDATE: The main remaining issue seems to be that the NLL checker is correctly identifying cases where temporary lifetimes are acting in surprising ways -- some of these were overlooked by the borrow checker. The only fix is to adjust the temporary lifetime rules, which would be technically backwards incompatible, but maybe something we could get away with. See this comment and follow-ups for more details. -- @nikomatsakis

---

See the second code snippet in comment below.

---

Old text

In the following code, f compiles but g does not. It looks like the borrow of a gets a different lifetime between these 2 functions.
Playground

fn f() {
    let a = vec![()];
    let b = (0 .. 1).map(|_| (|| a.iter()));
    let _c: Vec<_> = b.collect();
}

fn g() {
    let a = vec![()];
    let _c: Vec<_> = (0 .. 1).map(|_| (|| a.iter())).collect();
}

fn main() {}

Compiling playground v0.0.1 (file:///playground)
error[E0597]: `**a` does not live long enough
  --> src/main.rs:9:43
   |
9  |     let _c: Vec<_> = (0 .. 1).map(|_| (|| a.iter())).collect();
   |                                        -- ^                  - borrowed value only lives until here
   |                                        |  |
   |                                        |  does not live long enough
   |                                        capture occurs here
10 | }
   | - borrowed value needs to live until here

error: aborting due to previous error

error: Could not compile `playground`.

To learn more, run the command again with --verbose.

[stephaneyfx]

I'm not sure if this is related. Someone on IRC got a borrow error I couldn't explain and this is a minimized reproduction of the error: playground

pub struct Statement;

pub struct Rows<'stmt>(&'stmt Statement);

impl<'stmt> Drop for Rows<'stmt> {
    fn drop(&mut self) {}
}

impl<'stmt> Iterator for Rows<'stmt> {
    type Item = String;

    fn next(&mut self) -> Option<Self::Item> {
        None
    }
}

fn get_names() -> Option<String> {
    let stmt = Statement;
    let mut rows = Rows(&stmt);
    rows.map(|row| row).next()
    // Replacing the previous line with the following works:
    // let x = rows.map(|row| row).next();
    // x
    //
    // Removing the map works too as does removing the Drop impl.
}

fn main() {}

Dubious error:

   Compiling playground v0.0.1 (file:///playground)
error[E0597]: `stmt` does not live long enough
  --> src/main.rs:26:1
   |
19 |     let mut rows = Rows(&stmt);
   |                          ---- borrow occurs here
...
26 | }
   | ^ `stmt` dropped here while still borrowed
   |
   = note: values in a scope are dropped in the opposite order they are created

[pietroalbini]

This should be fixed with non-lexical lifetimes (playground link).

I don't think it's worth fixing the issue separately, since (if I understood correctly) NLL are planned to be stabilized this year. Closing this.

[stephaneyfx]

@pietroalbini : Thank you for looking into it. The second example still does not compile with NLL. Maybe they are unrelated and I should open another issue? See playground:

#![feature(nll)]

pub struct Statement;

pub struct Rows<'stmt>(&'stmt Statement);

impl<'stmt> Drop for Rows<'stmt> {
    fn drop(&mut self) {}
}

impl<'stmt> Iterator for Rows<'stmt> {
    type Item = String;

    fn next(&mut self) -> Option<Self::Item> {
        None
    }
}

fn get_names() -> Option<String> {
    let stmt = Statement;
    let rows = Rows(&stmt);
    rows.map(|row| row).next()
    // Replacing the previous line with the following works:
    // let x = rows.map(|row| row).next();
    // x
    //
    // Removing the map works too as does removing the Drop impl.
}

fn main() {}

[pietroalbini]

Sorry, I didn't see the other snippet :(
Reopening.

[pietroalbini]

cc @nikomatsakis?

[stephaneyfx]

@pietroalbini : No problem ☺️. I should probably have opened a separate issue as they may not be related after all. Thank you again!

[nikomatsakis]

Seems like the second issue is a bug. I'll file it and dig in a bit soon.

[nikomatsakis]

Ok so I dug into this error a bit. The cause is the ordering of this "tail bit" in the MIR:

    | Live variables on entry to bb10: [_6 (drop)]
    bb10: {
        StorageDead(_2);
        StorageDead(_1);
        drop(_6) -> [return: bb11, unwind: bb1];
    }

Here, _1 represents the stmts variable. _6 represents the temporary from calling map, I think:

    let mut _6: std::iter::Map<Rows<'_>, [closure@/home/nmatsakis/tmp/issue-46413.rs:22:14: 22:23]>;

So, MIR borrowck is not wrong to complain -- the destructor is indeed running with access to memory whose storage is dead. The question is more why the data is getting generated in this way. At first glance, the order looks wrong there, but the lifetime of temporaries in the tail expression of a block has some subtle points, and I'm fairly sure this is intentional (though I wonder if we could alter it)

cc @pnkfelix -- do you remember?