NLL reports "borrow used here in later iteration of loop" in cases outside of loop

[pnkfelix]

Spawned off of #53220 (comment)

In this code (play), we report "borrow used here in later iteration of loop" for the members.len() call that is not within the loop.

#![feature(nll)]

struct Archive;
struct ArchiveIterator<'a> { x: &'a Archive }
struct ArchiveChild<'a> { x: &'a Archive }

struct A { raw: &'static mut Archive }
struct Iter<'a> { raw: &'a mut ArchiveIterator<'a> }
struct C<'a> { raw: &'a mut ArchiveChild<'a> }

impl A { pub fn iter(&self) -> Iter<'_> { panic!() } }
impl Drop for A { fn drop(&mut self) { } }
impl<'a> Drop for C<'a> { fn drop(&mut self) { } }

impl<'a> Iterator for Iter<'a> {
    type Item = C<'a>;
    fn next(&mut self) -> Option<C<'a>> { panic!() }
}


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

(I assume this is due to some bug in how we are attempting to reverse-engineer the existence of a loop by analyzing the basic-block control-flow of MIR alone.)

[estebank]

If you comment out members.len(); the label is more appropriate (same text points at members in members.push), but it's not helpful enough.

[pnkfelix]

@estebank wait: the point of this particular ticket is solely the issue with members.len().

I think the broader issue with the diagnostic quality for this example overall is probably being covered by #53220, right?

[estebank]

@pnkfelix It seems that would be the relevant issue. I was pointing out what happens when removing the line as it hints at where the problem might be introduced in rustc.

[nikomatsakis]

I believe that the change I suggested in #54015 would also fix this.

[nikomatsakis]

I'm sort of tempted to close one or the other as a duplicate, in fact.

[nikomatsakis]

Adding to RC 2 milestone as a "nice to have", though I think we should merge with #54015

[pnkfelix]

This is fixed, presumably by PR #54343

[pnkfelix]

I spoke too soon.

The current output, according to play, is:

error[E0713]: borrow may still be in use when destructor runs
  --> src/main.rs:24:22
   |
24 |         members.push(child.raw);
   |                      ^^^^^^^^^
25 |     }
   |     - here, drop of `child` needs exclusive access to `*child.raw`, because the type `C<'_>` implements the `Drop` trait
26 |     members.len();
   |     ------- borrow used here, in later iteration of loop

error: aborting due to previous error

so, we still have the "in later iteration of loop" in the diagnostic.

[nikomatsakis]

Removing from the Rust 2018 Release milestone.

[nikomatsakis]

Created a topic for this on zulip (link) and assigned @spastorino

[arielb1]

So the problem here is that the CFG is very similar to that of:

    let mut members: Vec<&mut ArchiveChild<'_>> = vec![];
    let archive_iter = archive.iter();
    loop { match archive_iter.next() {
        Some(child) => {
            members.push(child.raw);
        }
        None => {
            members.len();
            return;
        }
    }}

In which case the error message would be correct. One way to solve this would be to add a StatementKind::LoopHeader and follow the CFG while being blocked by loop headers that are a parent of both of the uses. If we can't find such a path, then we are in the "later iteration of loop" case.

[pnkfelix]

NLL triage. P-medium.