Improve borrowck error message for nested flat_map

Opened by Jon Gjengset at 2020-06-12 00:52:03

Take the following code:

struct Foo { x: String }

impl Foo {
    fn one(&mut self) -> Vec<String> { vec![] }
    fn two(&mut self) {
        let z: Vec<Vec<String>> = Default::default();
        let _: Vec<_> = z.into_iter()
            .flat_map(|xs| xs.into_iter().flat_map(|_| self.one()))
            .collect();
    }
}

This code fails to compile with

error[E0598]: lifetime of `self` is too short to guarantee its contents can be safely reborrowed
 --> src/main.rs:8:52
  |
8 |             .flat_map(|xs| xs.into_iter().flat_map(|_| self.one()))
  |                                                    ^^^
  |
note: `self` would have to be valid for the method call at 7:25...
 --> src/main.rs:7:25
  |
7 |           let _: Vec<_> = z.into_iter()
  |  _________________________^
8 | |             .flat_map(|xs| xs.into_iter().flat_map(|_| self.one()))
9 | |             .collect();
  | |______________________^
note: ...but `self` is only valid for the lifetime  as defined on the body at 8:23
 --> src/main.rs:8:23
  |
8 |             .flat_map(|xs| xs.into_iter().flat_map(|_| self.one()))
  |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Though that is not particularly helpful in terms of identifying the underlying issue, namely that the inner flat map returns a closure whose lifetime is associated with self, preventing self from being used again when the outer closure is called again. I think. I'll let @nikomatsakis explain (reformatted from IRC):

basically you wind up with two ways to access self. easiest fix is certainly to call collect(). you are returning the closure as part of the iterator you return and that closure that you return has captured self, but the outer closure (which exists at the same time as the inner closure) also has captured self (so it can pass it to the inner closure). we have no way to know that the iterator will fully drain and discard the inner closure before re-invoking the outer closure

interesting test case; I wonder how on earth we could give an error that explains it better :P do you suppose you could open an issue with that example and a brief summary (as an A-diagnostics puzzler)

cc @estebank -- I'm not sure how to prioritize it, but this is a nice example of an error message that we could focus on improving.
Niko Matsakis at 2017-09-15 18:47:12

@nikomatsakis, just spitballing here, would this be a reasonable error?

error[E0598]: lifetime of `self` is too short to guarantee its contents can be safely reborrowed
  --> src/main.rs:10:31
   |
10 |                     .flat_map(|_| {
   |                               ^^^
   |
note: as a borrow tied to `self` is being returned on the body at 8:23 of type `std::iter::FlatMap<std::vec::IntoIter<std::string::String>, std::vec::Vec<std::string::String>, [closure@src/main.rs:10:31: 12:22 self:_]>`
  --> src/main.rs:9:17
   |
8  |               .flat_map(|xs| {
9  |                   xs.into_iter()
   |  _________________^
10 | |                     .flat_map(|_| {
11 | |                         self.one()
12 | |                     })
   | |______________________^ `std::iter::FlatMap` borrowing `self` being returned
note: ...but `self` would have to be valid for the method call at 7:25...
  --> src/main.rs:7:25
   |
7  |           let _: Vec<_> = z.into_iter()
   |  _________________________^
8  | |             .flat_map(|xs| {
9  | |                 xs.into_iter()
10 | |                     .flat_map(|_| {
...  |
13 | |             })
14 | |             .collect();
   | |______________________^
help: try to turn the borrow iterable into an owned type by collecting it:
  --> src/main.rs:12:22
   |
12 |                     }).collect()
   |                       ^^^^^^^^^^

I have no idea how feasible it'd be to actually implement all of it.

Esteban Kuber at 2017-09-26 00:44:01

Current output:

error: captured variable cannot escape `FnMut` closure body
 --> src/lib.rs:8:28
  |
8 |             .flat_map(|xs| xs.into_iter().flat_map(|_| self.one()))
  |                          - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returns a reference to a captured variable which escapes the closure body
  |                          |
  |                          inferred to be a `FnMut` closure
  |
  = note: `FnMut` closures only have access to their captured variables while they are executing...
  = note: ...therefore, they cannot allow references to captured variables to escape

Esteban Kuber at 2018-12-13 02:37:35

Triage: no change.
Esteban Kuber at 2020-06-12 00:52:03