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

Suggest named lifetime on trait functions like we do for free functions #73931

Closed
estebank opened this issue Jul 1, 2020 · 18 comments
Closed
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

estebank commented Jul 1, 2020

The following fn foo provides a structured suggestion to add a new named lifetime:

fn foo(x: &i32, y: &i32) -> Option<&i32> {
    Some(y)
}
error[E0106]: missing lifetime specifier
 --> src/lib.rs:1:36
  |
1 | fn foo(x: &i32, y: &i32) -> Option<&i32> {
  |           ----     ----            ^ expected named lifetime parameter
  |
  = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from `x` or `y`
help: consider introducing a named lifetime parameter
  |
1 | fn foo<'a>(x: &'a i32, y: &'a i32) -> Option<&'a i32> {
  |       ^^^^    ^^^^^^^     ^^^^^^^            ^^^

But similar code in a trait, doesn't:

trait T {
    fn foo(&self, x: &i32, y: &i32) -> Option<&i32> {
        Some(y)
    }
}
error[E0623]: lifetime mismatch
 --> src/lib.rs:3:9
  |
2 |     fn foo(&self, x: &i32, y: &i32) -> Option<&i32> {
  |                               ----     ------------
  |                               |
  |                               this parameter and the return type are declared with different lifetimes...
3 |         Some(y)
  |         ^^^^^^^ ...but data from `y` is returned here

The method add_missing_lifetime_specifiers_label is the one responsible for the former suggestion and can probably be easily reused where the method lifetime issue is emitted.

Thanks to https://users.rust-lang.org/t/this-parameter-and-the-return-type-are-declared-with-different-lifetimes/45200 for making me notice this discrepancy.

@estebank estebank 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 T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. labels Jul 1, 2020
@JohnTitor JohnTitor added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 1, 2020
@zachschuermann

This comment has been minimized.

@rustbot rustbot self-assigned this Jul 2, 2020
@theryangeary

This comment has been minimized.

@zachschuermann

This comment has been minimized.

@theryangeary

This comment has been minimized.

@p3achyjr

This comment has been minimized.

@zachschuermann

This comment has been minimized.

@p3achyjr

This comment has been minimized.

@p3achyjr

This comment has been minimized.

@zachschuermann

This comment has been minimized.

@camelid camelid changed the title Suggest named lifetime on methods like we do for functions Suggest named lifetime on trait functions like we do for free functions Oct 12, 2020
@camelid
Copy link
Member

camelid commented Oct 12, 2020

Changed issue title to be clearer:

Suggest named lifetime on trait functions like we do for free functions

@p3achyjr
Copy link

Hey @estebank, @schuermannator and I both looked into this, but found that the error reporting for traits and free functions live in different modules. Moreover, add_missing_lifetime_specifiers_label is an instance method and doesn't seem to fit in naturally in different_lifetimes.rs. Do you have any suggestions on this?

@dtolnay
Copy link
Member

dtolnay commented Jan 27, 2022

@rustbot release-assignment

@rustbot rustbot removed their assignment Jan 27, 2022
@kckeiks
Copy link
Contributor

kckeiks commented Feb 23, 2022

Hi,
I've been taking a high level look at the parts that are involved in try_report_anon_anon_conflict and add_missing_lifetime_specifiers_label. As @p3achyjr said, the error reporting for traits and free functions are in diff modules and are methods of different objects but it looks like try_report_anon_anon_conflict or NiceRegionError might have the parts needed (and that add_missing_lifetime_specifiers_label uses) to solve the problem. @estebank What do you think? I could take a crack at it. I'm looking to make my first contribution. If you think this is still an E-easy issue, I'll claim it.

@estebank
Copy link
Contributor Author

I think the first approach might be to try and move the relevant methods to a place that can be called from both modules. If that is not possible maybe creating a PR that duplicates the code could be useful to see the extent of the needs/impact. Feel free to take this for a spin and ping me on zulip or here for help!

@kckeiks
Copy link
Contributor

kckeiks commented Feb 23, 2022

@rustbot claim

@kckeiks
Copy link
Contributor

kckeiks commented Feb 23, 2022

I think the first approach might be to try and move the relevant methods to a place that can be called from both modules. If that is not possible maybe creating a PR that duplicates the code could be useful to see the extent of the needs/impact. Feel free to take this for a spin and ping me on zulip or here for help!

Sounds good. I will try the first approach. If it's not possible, I may be able to duplicate the code and have a PR but it depends on my bandwidth. I'll keep in touch.

@kckeiks
Copy link
Contributor

kckeiks commented Mar 1, 2022

@estebank I found some inconsistencies with the hints being given in different_lifetimes.rs so I made a small change #94464. The description of the issue is in #94462. I think the changes in that PR could provide a useful hint for the case in this issue, although the hint is not exactly as you had proposed here. There are some challenges (at least for me because I am a beginner) to getting the hint requested in this issue. I'd be happy to leave some of my notes if you want. I think the change in #94464 is a good compromise and could close this issue. Let me know what you think.

Given:

trait T {
    fn foo(&self, x: &i32, y: &i32) -> Option<&i32> {
        Some(y)
    }
}

Output:

error[E0623]: lifetime mismatch
 --> src/main.rs:7:9
  |
6 |     fn foo(&self, x: &i32, y: &i32) -> Option<&i32> {
  |                               ----     ------------
  |                               |
  |                               this parameter and the return type are declared with different lifetimes...
7 |         Some(y)
  |         ^^^^^^^ ...but data from `y` is returned here
  |
  = note: each elided lifetime in input position becomes a distinct lifetime
help: consider introducing a named lifetime parameter
  |
6 |     fn foo<'a>(&'a self, x: &i32, y: &'a i32) -> Option<&i32> {
  |           ++++  ++                    ++

error: aborting due to previous error

For more information about this error, try `rustc --explain E0623`.

@kckeiks
Copy link
Contributor

kckeiks commented Mar 2, 2022

@estebank Can this one be closed now that #94464 was merged?

@estebank estebank closed this as completed Mar 2, 2022
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 A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants