"Doesn't live long enough" error can be further improved

[sophiajt]

From @nikomatsakis (based on a chat with @wycats) - we can further improve the "doesn't live long enough" error by talking a bit more about how the values flow through the error. Something like:

error: `x` does not live long enough
 --> <anon>:4:10
  |
4 |     p = &x;
  |          - borrow occurs here
5 | }
  | ^ `x` dropped here while still borrowed
  |
= note: values in a scope are dropped in the opposite order they are created

[mikhail-m1]

@jonathandturner

I'm looking for test case, I've found some

error: `x` does not live long enough
  --> src/test/compile-fail/regions-escape-loop-via-variable.rs:21:14
   |
21 |         p = &x; //~ ERROR `x` does not live long enough
   |              ^ does not live long enough
22 |     }
   |     - borrowed value only lives until here
23 | }
   | - borrowed value needs to live until here

just need to change text for line 21 and 22 and add note?

[sophiajt]

These are slightly different examples, but I think you have the right idea. Your example would look like:

error: `x` does not live long enough
  --> src/test/compile-fail/regions-escape-loop-via-variable.rs:21:14
   |
21 |         p = &x;
   |              - borrow occurs here
22 |     }
   |     ^ `x` dropped here while still borrowed
23 | }
   | - borrowed value needs to live until here

Notice also that the primary span (the ^^^ part) is on line 22 instead of 21. I don't think there's a note on this one since the note is shown if both the borrowed value and the borrower are dropped in the same scope.

[mikhail-m1]

Is it example for first output?

fn main() {
    let p;
    let a = 42;
    p = &a;
}

[sophiajt]

@mikhail-m1 - that looks like it. Want to add it to the test suite, if it's not already there?

[mikhail-m1]

@jonathandturner I did not find it in repo. I doubt about '^', I far as i know both '^' and '-' adds by by span_label, so need to change error place to change mark types, but error in line 4 not in 5 (in first output)

[sophiajt]

Great, please add the test if you're creating a PR for this.

The ^ and - are created by the labels. Labels that have the same span as the error message itself get ^, all other labels get -

[mikhail-m1]

@jonathandturner Let me explain in another way:
for my sample I have next calls

struct_span_error(line 4 at 'a', ....)
span_label(line 4 at 'a', "does not live long enough")
span_label(line 5 at '}', "a dropped before borrower")

for achieve target output it will be

struct_span_error(line 5 at '}, ....)
span_label(line 4 at 'a', "borrow occurs here")
span_label(line 5 at '}',  "a dropped here while still borrowed")

so error will be in line 5, is it OK?

[sophiajt]

Yup, you got it.

[mikhail-m1]

@jonathandturner I've explored code borrow checker, and found several cases for "does not live long enough":

1. closures,
2. with same scope, like my last and your first sample
3. with different scope, like my first sample

Should I change only second case or third too?

and I've found some uncommon construction in closure case

let db = struct_span_error(...)
...
db.span = MultiSpan::from_span(...) 

Can I use it?

[sophiajt]

Have you tried the closure case to do a before/after and see what it looks like?

I don't remember using that functionality much. Does it let you set labels?

[mikhail-m1]

@jonathandturner In closure case It works before but in other logical branch.
It works, but reset all already performed span_label calls. So it was unexpected for me. Span_label works well after it.

It my case it allows minimize changes but not makes code harder to read.

What about my first question?

[mikhail-m1]

@jonathandturner error with different lifetime ranges like

fn main() {
  let x;
  {
     let y = 42;
     x = &y;
  }
}

remain unchanged

error: `y` does not live long enough
 --> 1.rs:5:11
  |
5 |      x = &y;
  |           ^ does not live long enough
6 |    }
  |    - borrowed value only lives until here
7 | }
  | - borrowed value needs to live until here

should I fix it too?

[sophiajt]

Yes, please. I thought that's what this PR fixed?

[mikhail-m1]

@jonathandturner it's different logic brunches of function, actually the function is used for output the error in next cases

1. temporary value or borrowed value in closures
2. temporary value or borrowed value with same lifetime
3. temporary value or borrowed value with different lifetime

the PR fixes just 2 and only for borrowed values

[sophiajt]

If you want to tackle the other cases, definitely feel free.

[sophiajt]

Looks like this was improved by #37405 and others