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

Async fns that take structs with elided lifetime parameters are broken. #60203

Closed
Lymia opened this issue Apr 23, 2019 · 2 comments · Fixed by #60388
Closed

Async fns that take structs with elided lifetime parameters are broken. #60203

Lymia opened this issue Apr 23, 2019 · 2 comments · Fixed by #60388
Labels
A-async-await Area: Async & Await A-lifetimes Area: lifetime related AsyncAwait-Polish Async-await issues that are part of the "polish" area C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Lymia
Copy link
Contributor

Lymia commented Apr 23, 2019

This code fails to compile with a very strange error:

#![feature(test, futures_api, async_await, await_macro)]
struct HasLifetime<'a>(&'a bool);
async fn bug(lifetime: HasLifetime) { 
    if *lifetime.0 { }
}

Error given:

error: cannot infer an appropriate lifetime
 --> src/lib.rs:3:37
  |
3 |   async fn bug(lifetime: HasLifetime) { 
  |                                       ^
  |                                       |
  |  _____________________________________this return type evaluates to the `'static` lifetime...
  | |
4 | |     if *lifetime.0 { }
5 | | }
  | |_^ ...but this borrow...
  |
note: ...can't outlive the anonymous lifetime #1 defined on the function body at 3:1
 --> src/lib.rs:3:1
  |
3 | / async fn bug(lifetime: HasLifetime) { 
4 | |     if *lifetime.0 { }
5 | | }
  | |_^
help: you can add a constraint to the return type to make it last less than `'static` and match the anonymous lifetime #1 defined on the function body at 3:1
  |
3 | async fn bug(lifetime: HasLifetime)  + '_{ 
  |                                      ^^^^

It seems the compiler fails to generate a lifetime constraint on the impl Future<...> the function compiles down to because there is no apparent lifetime on HasLifetime. Replacing it with HasLifetime<'_> fixes the problem.

@jonas-schievink jonas-schievink added A-async-await Area: Async & Await A-lifetimes Area: lifetime related T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Apr 23, 2019
@cramertj cramertj added the AsyncAwait-Polish Async-await issues that are part of the "polish" area label Apr 23, 2019
@cramertj
Copy link
Member

Today the elided-lifetime-in-async-fn implementation shares most of its logic with the elision in impl headers feature, where non-explicit (without '_) lifetimes are explicitly banned. From a purely implementation-oriented standpoint, I think I'd prefer to have the two continue to match behavior, so my plan is to introduce a hard error for this case similar to the one already present in impl headers. If we decide it's desirable, we can always relax this constraint in the future.

@Centril Centril added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Apr 23, 2019
@nikomatsakis
Copy link
Contributor

We discussed this in the @rust-lang/lang meeting today and reached the following conclusions (however, we did not have a large quorum, so perhaps these conclusions can be considered tentative):

We are content to go ahead and make hidden parameters a hard error in async fn for now; we did have a rough consensus to deprecate this pattern and hence we are not required to support it in new forms. Also, it would be backwards compatible to extend to permit hidden parameters in the future if desired. There was a vague concern that this might make async fn appear to be less "full featured" than regular fn, but this can be side-stepped by using the "deprecated" terminology in the error message etc.

(In general, we should be working on the rust-2018-idioms lints, but that's another story.)

I'm going to tentatively clear the I-Nominated flag, but if any member of @rust-lang/lang feels they want to discuss this again, feel free to set it once more. =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-lifetimes Area: lifetime related AsyncAwait-Polish Async-await issues that are part of the "polish" area C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants