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

Argument-position impl Trait requires a named lifetime #49287

Open
cramertj opened this issue Mar 22, 2018 · 11 comments
Open

Argument-position impl Trait requires a named lifetime #49287

cramertj opened this issue Mar 22, 2018 · 11 comments
Labels
A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cramertj
Copy link
Member

This function produces an "expected lifetime parameter" error:

fn foo(_: impl Iterator<Item = &u8>) {}

This code should instead be accepted and bound the impl Trait parameter by the elided lifetime.

cc #34511

@nikomatsakis
Copy link
Contributor

In my opinion, we should accept '_, in-band lifetimes, and friends in where clauses as well. I'm not 100% sure whether this was controversial though, have to check.

@pietroalbini pietroalbini added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. labels Mar 23, 2018
@alexreg

This comment has been minimized.

@cramertj

This comment has been minimized.

@alexreg

This comment has been minimized.

@CAD97
Copy link
Contributor

CAD97 commented May 8, 2019

Ran into this today. Providing the '_ lifetime gives a very fun error:

fn f(_: impl Iterator<Item = &'_ ()>) {}
error[E0106]: missing lifetime specifier
 --> src/lib.rs:1:31
  |
1 | fn f(_: impl Iterator<Item = &'_ ()>) {}
  |                               ^^ expected lifetime parameter

@estebank estebank added D-confusing Diagnostics: Confusing error or lint that should be reworked. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. labels Jan 30, 2020
@estebank
Copy link
Contributor

estebank commented Jan 30, 2020

It's actually gotten slightly worse:

error[E0106]: missing lifetime specifier
 --> src/lib.rs:1:31
  |
1 | fn f(_: impl Iterator<Item = &'_ ()>) {}
  |                               ^^ expected named lifetime parameter
  |
help: consider introducing a named lifetime parameter
  |
1 | fn f(_: 'a, impl Iterator<Item = &'a ()>) {}
  |         ^^                       ^^^

#68583 will address that. After it gets merged the output will be

Screen Shot 2020-01-30 at 11 55 07 AM

I feel that's going to be as good a diagnostic we're gonna have for this.

@runiq
Copy link

runiq commented Jan 31, 2020

I feel that's going to be as good a diagnostic we're gonna have for this.

@estebank Does that mean that this will be the "final" state for this feature? As in, anonymous lifetimes will not be usable in the positions mentioned by @nikomatsakis in #49287 (comment)?

@estebank
Copy link
Contributor

What I mean that until anonymous lifetimes are accepted in impl Trait we're going to have some diagnostic, and the one I posted is likely the best we can do given our constraints.

Centril added a commit to Centril/rust that referenced this issue Feb 5, 2020
Account for HR lifetimes when suggesting introduction of named lifetime

```
error[E0106]: missing lifetime specifier
 --> src/test/ui/suggestions/fn-missing-lifetime-in-item.rs:2:32
  |
2 | struct S2<F: Fn(&i32, &i32) -> &i32>(F);
  |                 ----  ----     ^ 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 argument 1 or argument 2
  = note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
help: consider making the bound lifetime-generic with a new `'a` lifetime
  |
2 | struct S2<F: for<'a> Fn(&'a i32, &'a i32) -> &'a i32>(F);
  |              ^^^^^^^    ^^^^^^^  ^^^^^^^     ^^^
help: consider introducing a named lifetime parameter
  |
2 | struct S2<'a, F: Fn(&'a i32, &'a i32) -> &'a i32>(F);=
  |           ^^^       ^^^^^^^  ^^^^^^^     ^^^
```

Follow up to rust-lang#68267. Addresses the diagnostics part of rust-lang#49287.
estebank added a commit to estebank/rust that referenced this issue Feb 5, 2020
@nikomatsakis
Copy link
Contributor

The reason that lifetimes were not permitted here, as I recall, was some uncertainty about what they should mean (in particular, should impl Foo<'_> mean impl for<'a> Foo<'a> or what). I am actually still a bit unsure as to my opinion here -- I've seen people expect it to mean both things.

@estebank
Copy link
Contributor

estebank commented Feb 5, 2020

In this case, impl for<'a> Foo<'a> wouldn't work, but could you show me a case where it would? I feel that the only case where impl for<'a> X is common is for Fn() and in that case unnamed lifetimes usually work, and in the case where a named lifetime that hasn't been introduced is used produces the following now (not yet in the latest nightly):

error[E0261]: use of undeclared lifetime name `'a`
 --> file4.rs:1:27
  |
1 | fn foo(_compare: impl Fn(&'a u8)) {
  |                           ^^ undeclared lifetime
  |
  = note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
help: consider introducing lifetime `'a` here
  |
1 | fn foo<'a>(_compare: impl Fn(&'a u8)) {
  |       ^^^^
help: consider making the bound lifetime-generic with a new `'a` lifetime
  |
1 | fn foo(_compare: impl for<'a> Fn(&'a u8)) {
  |                       ^^^^^^^

@estebank estebank removed the D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. label Feb 5, 2020
@nikomatsakis
Copy link
Contributor

I'm not sure what you mean by "show you a case where it would work", I guess you mean a realistic example where that is what you would want? I don't really have one off the top of my head, but I agree with you it's probably quite a bit less common -- after all, I almost never write for bounds in practice.

So yeah, I pretty much agree it'd be the wrong meaning, and I think it'd even be somewhat counter-intuitive. We have a pretty strong precedent right now that '_ binds into the "innermost enclosing parentheses", in a sense, and I would be quite surprising to find impl Foo<'_> create a lifetime bound within the scope of impl keyword.

bors added a commit that referenced this issue Feb 6, 2020
Account for HR lifetimes when suggesting introduction of named lifetime

```
error[E0106]: missing lifetime specifier
 --> src/test/ui/suggestions/fn-missing-lifetime-in-item.rs:2:32
  |
2 | struct S2<F: Fn(&i32, &i32) -> &i32>(F);
  |                 ----  ----     ^ 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 argument 1 or argument 2
  = note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
help: consider making the bound lifetime-generic with a new `'a` lifetime
  |
2 | struct S2<F: for<'a> Fn(&'a i32, &'a i32) -> &'a i32>(F);
  |              ^^^^^^^    ^^^^^^^  ^^^^^^^     ^^^
help: consider introducing a named lifetime parameter
  |
2 | struct S2<'a, F: Fn(&'a i32, &'a i32) -> &'a i32>(F);=
  |           ^^^       ^^^^^^^  ^^^^^^^     ^^^
```

Follow up to #68267. Addresses the diagnostics part of #49287.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. 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

8 participants
@alexreg @nikomatsakis @runiq @estebank @pietroalbini @cramertj @CAD97 and others