[nll] help user trace lifetimes through error messages

[nikomatsakis]

I am archiving this error (playground) which arose while porting rustc to use NLL. It is a non-trivial case of tracing lifetimes. I believe the error is legit, but it'd be nice if we could help the user see how the lifetime winds up over-extended:

#![feature(nll)]

struct RawArchive;
struct RawArchiveIterator<'a> { x: &'a RawArchive }
struct RawArchiveChild<'a> { x: &'a RawArchive }

struct ArchiveRO {
    raw: &'static mut RawArchive
}

struct Iter<'a> {
    raw: &'a mut RawArchiveIterator<'a>,
}

struct Child<'a> {
    raw: &'a mut RawArchiveChild<'a>,
}

impl ArchiveRO {
    pub fn iter(&self) -> Iter<'_> {
        panic!()
    }
}

impl Drop for ArchiveRO {
    fn drop(&mut self) { }
}

impl<'a> Iterator for Iter<'a> {
    type Item = Result<Child<'a>, String>;

    fn next(&mut self) -> Option<Result<Child<'a>, String>> {
        panic!()
    }
}

impl<'a> Drop for Iter<'a> {
    fn drop(&mut self) { }
}

impl<'a> Drop for Child<'a> {
    fn drop(&mut self) { }
}

fn error(archive: &ArchiveRO) {
    let mut members: Vec<&mut RawArchiveChild<'_>> = vec![];
    for child in archive.iter() {
        match child {
            Ok(child) => members.push(child.raw),
            Err(_) => ()
        }
    }
    members.len();
}
  
fn main() { }

Currently gives:

error[E0597]: `*child.raw` does not live long enough
  --> src/main.rs:49:39
   |
49 |             Ok(child) => members.push(child.raw),
   |                                       ^^^^^^^^^ borrowed value does not live long enough
50 |             Err(_) => ()
51 |         }
   |         - `*child.raw` dropped here while still borrowed
52 |     }
53 |     members.len();
   |     ------- borrow used here in later iteration of loop

[nikomatsakis]

Oh, and the "used in later iteration of loop" part of the message is not correct.

[nikomatsakis]

Interestingly, changing this to self.raw() tells us a slightly different error message (though not "on purpose"):

error[E0597]: `child` does not live long enough
  --> src/main.rs:55:43
   |
55 |             Ok(mut child) => members.push(child.raw()),
   |                                           ^^^^^ borrowed value does not live long enough
56 |             Err(_) => ()
57 |         }
   |         - `child` dropped here while still borrowed
58 |     }
59 |     members.len();
   |     ------- borrow used here in later iteration of loop

Notably, we talk about child, which is strictly better I think and more correct.

[nikomatsakis]

After a long talk with @wycats we came to a few conclusions. In part, that we should be consistent about the terminology we use to talk about destructors, and also that it would be helpful to highlight the Drop impl. Also that a key point is that the destructor may access child.raw.

Something like:

error[E0597]: `child` does not live long enough
  --> src/main.rs:55:43
   |
49 |             Ok(child) => members.push(child.raw),
   |                                       ^^^^^^^^^ borrowed value does not live long enough
50 |             Err(_) => ()
57 |         }
   |         - the destructor for `child` executes here, and it may access `child.raw`
58 |     }
59 |     members.len();
   |     ------- borrow used here in later iteration of loop
   |
  = note: the drop impl appears here
   |    impl Drop for ... { .. }

[pnkfelix]

Oh, and the "used in later iteration of loop" part of the message is not correct.

Filed as #53773

[estebank]

Small wording suggestion:

error[E0597]: `child` does not live long enough
  --> src/main.rs:55:43
   |
49 |             Ok(child) => members.push(child.raw),
   |                                       ^^^^^^^^^ borrowed value does not live long enough
50 |             Err(_) => ()
57 |         }
   |         - the destructor for `child` executes here
58 |     }
59 |     members.len();
   |     ------- borrow of `child.raw` continues to exist here
   |
note: `child` has a destructor implementation that might access `child.raw`
   |
15 | / impl<'a> Drop for Child<'a> {
16 | |     fn drop(&mut self) { }
17 | | }
   | |_^
   = note: <some further explanation or link to explain `Drop`>

---

Would it be worth it to spend the time checking wether a drop impl touches a given field and not complain if not? It seems like it would be an ergonomic win, but not big enough to be worth it.

[matthewjasper]

Current message

error[E0713]: borrow may still be in use when destructor runs
  --> <source>:50:39
   |
50 |             Ok(child) => members.push(child.raw),
   |                                       ^^^^^^^^^
51 |             Err(_) => ()
52 |         }
   |         - here, drop of `child` needs exclusive access to `*child.raw`, because the type `Child<'_>` implements the `Drop` trait
53 |     }
54 |     members.len();
   |     ------- borrow used here in later iteration of loop
error: aborting due to previous error
For more information about this error, try `rustc --explain E0713`.
Compiler returned: 1

[nikomatsakis]

I'm going to call this bug closed for now, the error seems pretty decent, even if I might quibble with the wording here and there.