Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't suggest let bindings if they don't help with borrows #52049

Closed
oli-obk opened this issue Jul 4, 2018 · 3 comments
Closed

Don't suggest let bindings if they don't help with borrows #52049

oli-obk opened this issue Jul 4, 2018 · 3 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Jul 4, 2018

fn foo(_: &'static u32) {}

fn unpromotable<T>(t: T) -> T { t }

fn main() {
    foo(&unpromotable(5u32));
}

suggests

error[E0597]: borrowed value does not live long enough
 --> src/main.rs:6:10
  |
6 |     foo(&unpromotable(5u32));
  |          ^^^^^^^^^^^^^^^^^^ - temporary value only lives until here
  |          |
  |          temporary value does not live long enough
  |
  = note: borrowed value must be valid for the static lifetime...
  = note: consider using a `let` binding to increase its lifetime

but

    let x = unpromotable(5u32);
    foo(&x);

still won't give us a 'static lifetime.

We should not emit the note if the required lifetime is 'static

Instructions

  1. search for the note inside the source
  2. I presume there's some code already checking for producing the "borrowed value must be valid for the static lifetime" message, find that code and bubble up the knowledge about that until you have it in the scope of the "consider using a let binding to increase its lifetime" message
  3. Don't emit the message in that case
  4. This'll already fail a bunch of tests, I don't think you need any new tests. Running ./x.py test --stage 1 src/test/{ui,compile-fail} --bless should succeed and give you a bunch of changes to commit. Check that all changes make sense and just commit them.
@oli-obk oli-obk added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-diagnostics Area: Messages for errors, warnings, and lints labels Jul 4, 2018
@PramodBisht
Copy link
Contributor

@oli-obk can I take this on if nobody is working on this?

@oli-obk
Copy link
Contributor Author

oli-obk commented Jul 4, 2018

All yours!

@PramodBisht PramodBisht self-assigned this Jul 6, 2018
PramodBisht added a commit to PramodBisht/rust that referenced this issue Jul 7, 2018
PramodBisht added a commit to PramodBisht/rust that referenced this issue Jul 7, 2018
bors added a commit that referenced this issue Jul 8, 2018
Don't suggest `let` bindings if they don't help with borrows

@oli-obk I have added a condition to address #52049, right now, this is on WIP because I think code change is also required on `error_reporting.rs`. Plus I need to check if any test cases fail.
I will ping you again if everything passes

r? @oli-obk
@PramodBisht
Copy link
Contributor

closing it, as required code change has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

2 participants