Try to tailor error messages for common scenarios

[nikomatsakis]

This is a vague bug but here is my intention:

1. Look at common bugs (returning a closure, etc) and see if we can improve the error messages for those
2. Try to improve the heuristics that suggest how to report a region inference failure, which frequently give nonsense results

[nikomatsakis]

triage: P-high (1.0)

[nikomatsakis]

I've been reading through this thread http://internals.rust-lang.org/t/confused-by-lifetime-error-messages-tell-me-about-it/358/ and trying to extract some specific examples I'd like to target. I will add them as comments here for now.

[nikomatsakis]

Outliving temporary lifetimes, particularly with the newer rules:

use std::collections::HashMap;
use std::io::BufRead;

fn main() {
    let map = (|| {
        let mut map = HashMap::new();
        map.insert("imo", "in my opinion");
        map
    })();

    for line in std::io::stdin().lock().lines().map(|l| l.unwrap()) {
        println!("{}", line.split(' ')
                 .map(|seg| if map.contains_key(seg) {
                     map[seg.as_slice()].to_string()
                 } else {
                     seg.to_string()
                 }).collect::<Vec<String>>().connect(" "));
    }
}

compiled yields an error (see below). Some thoughts:

1. I think part of the issue that the for loop expansion is yielding particularly poor explanations of the lifetimes involved.
2. A temporary value is being returned by std::io::stdin()
3. This temporary value is not valid long enough, but the lifetime mismatch here is somewhat subtle.

We do make a helpful suggestion, but we could make it more concrete perhaps, by indicating what specifically should be moved into a let, and indicating that earlier.

(In general, my impression is that indicating the precise scopes seems to (usually) not be that helpful, particularly in cases like these.)

/home/nmatsakis/tmp/lifetime-test-1.rs:11:17: 11:33 error: borrowed value does not live long enough
/home/nmatsakis/tmp/lifetime-test-1.rs:11     for line in std::io::stdin().lock().lines().map(|l| l.unwrap()) {
                                                          ^~~~~~~~~~~~~~~~
/home/nmatsakis/tmp/lifetime-test-1.rs:11:5: 19:2 note: reference must be valid for the destruction scope surrounding statement at 11:4...
/home/nmatsakis/tmp/lifetime-test-1.rs:11     for line in std::io::stdin().lock().lines().map(|l| l.unwrap()) {
/home/nmatsakis/tmp/lifetime-test-1.rs:12         println!("{}", line.split(' ')
/home/nmatsakis/tmp/lifetime-test-1.rs:13                  .map(|seg| if map.contains_key(seg) {
/home/nmatsakis/tmp/lifetime-test-1.rs:14                      map[seg.as_slice()].to_string()
/home/nmatsakis/tmp/lifetime-test-1.rs:15                  } else {
/home/nmatsakis/tmp/lifetime-test-1.rs:16                      seg.to_string()
                                          ...
/home/nmatsakis/tmp/lifetime-test-1.rs:11:5: 19:2 note: ...but borrowed value is only valid for the statement at 11:4
/home/nmatsakis/tmp/lifetime-test-1.rs:11     for line in std::io::stdin().lock().lines().map(|l| l.unwrap()) {
/home/nmatsakis/tmp/lifetime-test-1.rs:12         println!("{}", line.split(' ')
/home/nmatsakis/tmp/lifetime-test-1.rs:13                  .map(|seg| if map.contains_key(seg) {
/home/nmatsakis/tmp/lifetime-test-1.rs:14                      map[seg.as_slice()].to_string()
/home/nmatsakis/tmp/lifetime-test-1.rs:15                  } else {
/home/nmatsakis/tmp/lifetime-test-1.rs:16                      seg.to_string()
                                          ...
/home/nmatsakis/tmp/lifetime-test-1.rs:11:5: 19:2 help: consider using a `let` binding to increase its lifetime
/home/nmatsakis/tmp/lifetime-test-1.rs:11     for line in std::io::stdin().lock().lines().map(|l| l.unwrap()) {
/home/nmatsakis/tmp/lifetime-test-1.rs:12         println!("{}", line.split(' ')
/home/nmatsakis/tmp/lifetime-test-1.rs:13                  .map(|seg| if map.contains_key(seg) {
/home/nmatsakis/tmp/lifetime-test-1.rs:14                      map[seg.as_slice()].to_string()
/home/nmatsakis/tmp/lifetime-test-1.rs:15                  } else {
/home/nmatsakis/tmp/lifetime-test-1.rs:16                      seg.to_string()
                                          ...

[nikomatsakis]

see #23581

[nikomatsakis]

Suggesting move from closures would help with this example from @wycats:

<anon>:7:9: 7:41 error: `book1` does not live long enough
<anon>:7   spawn(|| println!("{}", &book1.title));
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: reference must be valid for the static lifetime...
<anon>:4:91: 8:2 note: ...but borrowed value is only valid for the block suffix following statement 0 at 4:90
<anon>:4   let mut book1 = Book { title: "Hello".to_string(), chapters: vec!["world".to_string()] };
<anon>:5   let mut book2 = Book { title: "Hello".to_string(), chapters: vec!["world".to_string()] };
<anon>:6 
<anon>:7   spawn(|| println!("{}", &book1.title));
<anon>:8 }

[nikomatsakis]

More complete example:

use std::thread::spawn;

fn main() {
    let mut books = vec![1,2,3];
    spawn(|| books.push(4));
}

[nikomatsakis]

Similar to that case is this one http://is.gd/ExwsE4

[wycats]

@nikomatsakis I also don't think that this message is very useful:

note: reference must be valid for the static lifetime...
<anon>:4:91: 8:2 note: ...but borrowed value is only valid for the block suffix following statement 0 at 4:90

If you already know roughly how things work, the basic message says what you need to know, and if you don't, it's totally obtuse.

[nikomatsakis]

PR for the escaping closure case: #24242

[nikomatsakis]

From Carl: https://gist.github.com/carllerche/d058aff63c9f9ae7a8a8

Things that would have helped:

• Report errors for callee first
• Not reporting duplicate Sized error for Option<Foo> and Foo