Two cases of confusing borrow over-extension

[yuriks]

Using rustc nightly 2018-10-13 4699283 (from playpen).

I found two cases where lifetimes appear to be extended where they shouldn't:

use std::cell::RefCell;
use std::collections::binary_heap::PeekMut;
use std::collections::BinaryHeap;

fn main() {
    let heap: RefCell<BinaryHeap<u32>> = RefCell::new(BinaryHeap::new());

    let mut heap_ref = heap.borrow_mut();
    let entry = if let Some(entry) = heap_ref.peek_mut() {
        entry
    } else {
        panic!();
    };
    let task = PeekMut::pop(entry);

    //drop(entry);

    drop(heap_ref);
    println!("We made it {:?}, {:?}", heap.borrow_mut(), task);
}

Playpen link

Which gives this error:

error[E0505]: cannot move out of `heap_ref` because it is borrowed
  --> src/main.rs:18:10
   |
9  |     let entry = if let Some(entry) = heap_ref.peek_mut() {
   |                                      -------- borrow of `heap_ref` occurs here
...
18 |     drop(heap_ref);
   |          ^^^^^^^^ move out of `heap_ref` occurs here

Even though the only thing that could be borrowing heap_ref is entry, which has already been moved into the pop() call.

---

use std::cell::RefCell;
use std::collections::binary_heap::PeekMut;
use std::collections::BinaryHeap;

fn main() {
    let heap: RefCell<BinaryHeap<u32>> = RefCell::new(BinaryHeap::new());

    let task = {
        let mut heap_ref = heap.borrow_mut();
        if let Some(entry) = heap_ref.peek_mut() {
            PeekMut::pop(entry)
        } else {
            panic!();
        }
    };

    println!("We made it {:?}, {:?}", heap.borrow_mut(), task);
}

Playpen link

Which gives this error:

error[E0597]: `heap_ref` does not live long enough
  --> src/main.rs:10:30
   |
10 |         if let Some(entry) = heap_ref.peek_mut() {
   |                              ^^^^^^^^ borrowed value does not live long enough
...
15 |     };
   |     -- borrowed value needs to live until here
   |     |
   |     `heap_ref` dropped here while still borrowed

Where it's complaining that a borrow needs to live until the same point where it's dropped.

[memoryruins]

The first case successfully compiles with #![feature(nll)] or 2018 edition enabled, and the second case gives a more thorough diagnostic when enabled.

15 |     };
   |     -- ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `std::option::Option<std::collections::binary_heap::PeekMut<'_, u32>>`
   |     |
   |     `heap_ref` dropped here while still borrowed
   |
   = note: The temporary is part of an expression at the end of a block. Consider forcing this temporary to be dropped sooner, before the block's local variables are dropped. For example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block.

[pnkfelix]

There is something about the second diagnostic that is true with and without NLL, and its something I'd like to try to figure out a way to fix:

The presentation of the two different adjacent annotations looks like a single point when you see:

15 |     };
   |     -- borrowed value needs to live until here
   |     |
   |     `heap_ref` dropped here while still borrowed

The diagnostic becomes (IMO) much clearer if you add some spaces between the } and the ;:

15 |     }  ;
   |     -  - borrowed value needs to live until here
   |     |
   |     `heap_ref` dropped here while still borrowed

since now it is much clearer that the usage point comes after the point where it is dropped.

[pnkfelix]

There is also a problem in the presentation of this diagnostic (this time from NLL):

10 |         if let Some(entry) = heap_ref.peek_mut() {
   |                              ^^^^^^^^-----------
   |                              |
   |                              borrowed value does not live long enough
   |                              a temporary with access to the borrow is created here ...

At least to my eye, it is not sufficiently clear which of the two messages one is meant to apply to each of the two overlapping ranges (the ^^^ vs the -----)

[pnkfelix]

Having said that, I am mostly tempted to tag this with NLL-fixed-by-NLL.

I'll double check with others, perhaps at the meeting tonight.