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 adding undeclared lifetime parameters to struct #46135

Closed
cramertj opened this issue Nov 20, 2017 · 9 comments
Closed

Suggest adding undeclared lifetime parameters to struct #46135

cramertj opened this issue Nov 20, 2017 · 9 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-diagnostics Working group: Diagnostics

Comments

@cramertj
Copy link
Member

The following code:

struct Foo {
    x: &'a u8,
}

Generates this error:

error[E0261]: use of undeclared lifetime name `'a`
 --> src/main.rs:2:9
  |
2 |     x: &'a u8,
  |         ^^ undeclared lifetime

error: aborting due to previous error

error: Could not compile `playground`.

To learn more, run the command again with --verbose.

It'd be nice to provide a suggestion to add a lifetime parameter to the struct definition.

Originally suggested here.

@TimNN TimNN added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Nov 21, 2017
@estebank estebank added E-needs-mentor WG-diagnostics Working group: Diagnostics labels Dec 7, 2017
@estebank
Copy link
Contributor

estebank commented Dec 7, 2017

In the following code:

struct_span_err!(self.sess, lifetime_ref.span, E0261,
"use of undeclared lifetime name `{}`", lifetime_ref.name.name())
.span_label(lifetime_ref.span, "undeclared lifetime")
.emit();

suggest adding the undeclared lifetime to the appropriate parent scope(s) (fn or impl blocks).

Bonus points to perform fuzzy search of the undeclared lifetime against all the parent binders to provide typo suggestion.

@estebank estebank added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. and removed E-needs-mentor labels Dec 7, 2017
@typesanitizer
Copy link

I can work on this. :)

@estebank
Copy link
Contributor

@theindigamer feel free to reach out in gitter, internals, IRC or here if you need any help!

@typesanitizer
Copy link

Thinking about using lifetime parameters that haven't been initialized generically (as opposed to purely in the context of struct definitions), the two simplest cases are

  1. If inside a struct definition, suggest adding the lifetime parameter after the struct's name.
  2. If inside a free-standing function, suggest adding the lifetime parameter after the function's name.

A slightly more complex case is:

  1. If inside a function inside an impl, depending on the situation, add the lifetime parameter after the function's name or after the impl... - I'm not sure what heuristic should be used here for the suggestion. It might make more sense to either add the parameter to either the fn or to the impl.

One could create more complicated cases by arbitrarily nesting fn and impl (a fn can contain a struct and an impl, and an impl can contain a fn). I'm not entirely sure if we could cook up a case where the lifetime must be introduced in some arbitrary ancestor (not just the parent or grandparent), but maybe that is just my lack of imagination.

Another minor issue is that (ideally) we should "unify" errors in the sense that

impl Foo<'a> {
    fn x(&self) -> &'a u8
}

will give two errors

error[E0261]: use of undeclared lifetime name `'a`
 --> foo.rs:5:10
  |
5 | impl Foo<'a> {
  |          ^^ undeclared lifetime

error[E0261]: use of undeclared lifetime name `'a`
 --> foo.rs:6:21
  |
6 |     fn x(&self) -> &'a str { self.x }
  |                     ^^ undeclared lifetime

error: aborting due to 2 previous errors

but it should give only one suggestion, which is replace impl with impl<'a>. What would be a good approach for doing so?

@estebank
Copy link
Contributor

You have adequately expressed the nuance when encountering nested scopes.

For the case where you have nested impls/fns/structs I would provide a label to all the places where the lifetime could be declared:

struct X;

impl X {
    fn bar(bar_arg: &'a usize) {
        fn foo(foo_arg: &'a usize) {
            struct Y {
                y: &'a usize,
            }
        }
    }
}
error[E0261]: use of undeclared lifetime name `'a`
 --> src/main.rs:4:22
  |
3 | impl X {
  | ------ if you mean to declare lifetime `'a` in the `impl`, use `impl<'a> X<'a>`
4 |     fn bar(bar_arg: &'a usize) {
  |     ------           ^^ undeclared lifetime
  |     |
  |     if you meant to declare lifetime `'a` in the `fn`, use `fn bar<'a>`

error[E0261]: use of undeclared lifetime name `'a`
 --> src/main.rs:5:26
  |
5 |         fn foo(foo_arg: &'a usize) {
  |            ---              ^^ undeclared lifetime
  |            |
  |            help: declare lifetime `'a`: `foo<'a>`

error[E0261]: use of undeclared lifetime name `'a`
 --> src/main.rs:7:21
  |
6 |            struct Y {
  |                   - help: declare lifetime `'a`: `Y<'a>`
7 |                 y: &'a usize,
  |                     ^^ undeclared lifetime

error: aborting due to 3 previous errors

For the code example you provided, what I would do is to still emit the first error, recommend the suggestion to add the undeclared lifetime to the impl but accept it at the parser level as a valid lifetime binding. This way the second error won't be emitted (unless the 'a binding is removed, then it will trigger), but the compilation won't be successful due to the first error. In the end the output would look something like:

error[E0261]: use of undeclared lifetime name `'a`
 --> foo.rs:5:10
  |
5 | impl Foo<'a> {
  | ----     ^^ undeclared lifetime
  | |
  | help: declare lifetime `'a` here: `impl<'a>`

@typesanitizer
Copy link

| ------ if you mean to declare lifetime 'a in the impl, use `impl<'a> X<'a>

Do you mean impl<'a> X here, since X doesn't have a lifetime parameter in the example? Also, I'm slightly surprised that you need to declare the lifetime in 3 places and (for example) this doesn't compile.

struct X;

impl<'a> X {
    fn bar(bar_arg: &'a usize) {
        fn foo(foo_arg: &'a usize) {
            struct Y<'a> {
                y: &'a usize,
            }
        }
    }
}

fn main() {}

Are lifetimes parameters only allowed to be used in the children and not in further descendants? That makes things much simpler.

accept it at the parser level as a valid lifetime binding

Got it. Just to make sure that I'm understanding correctly, the way to do it would be to call self.insert_lifetime with the appropriate arguments, correct?

@estebank
Copy link
Contributor

Do you mean impl<'a> X here, since X doesn't have a lifetime parameter in the example?
You're right.

Are lifetimes parameters only allowed to be used in the children and not in further descendants? That makes things much simpler.

I believe lifetimes only get passed from impls to a nested fns, but not from an fn to a nested one or between nested impls.

Just to make sure that I'm understanding correctly, the way to do it would be to call self.insert_lifetime with the appropriate arguments, correct?

I believe that is the case.

@steveklabnik
Copy link
Member

Triage: this error is the same today.

@estebank
Copy link
Contributor

estebank commented Jan 21, 2020

I believe #68267 partially addresses this.

Edit: confirmed that that PR addresses this case.

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 C-enhancement Category: An issue proposing an enhancement or a PR with one. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-diagnostics Working group: Diagnostics
Projects
None yet
Development

No branches or pull requests

5 participants