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

Disallow non-explicit elided lifetimes in async fn #60388

Merged
merged 1 commit into from
May 3, 2019

Conversation

cramertj
Copy link
Member

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 30, 2019
src/librustc/hir/lowering.rs Outdated Show resolved Hide resolved
src/test/ui/async-fn-path-elision.rs Show resolved Hide resolved
@rust-highfive

This comment has been minimized.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally positive, but the error wording is a bit confusing. If we don't want to do the proper fix, then perhaps something like "help: use '_ to represent the elided lifetime explicitly"?

LL | async fn error(lt: HasLifetime) {
| ^^^^^^^^^^^
|
= help: consider using an explicit elided lifetime: `'_`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggestion seems a bit confusing. It would be nicer, of course, to suggest HasLifetime<'_>, but this is also complicated to do. @zackmdavis put a quite a bit of work into these sorts of things as part of the lint, I wonder if we can reuse that code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've put up a change to reuse the logic from the rust_2018_idioms elided lifetime parameters in types lint-- this gives a nice improvement for the impl headers case as well-- thanks for the recommendation! As a drive-by, I did notice that the error prints somewhat surprisingly:

#![deny(rust_2018_idioms)]

struct Foo<'a>(&'a u8);

fn foo(_: Foo) {}

gives

error: hidden lifetime parameters in types are deprecated
 --> src/main.rs:5:11
  |
5 | fn foo(_: Foo) {}
  |           ^^^- help: indicate the anonymous lifetime: `<'_>`

Strangely, adding a type parameter makes it spell out the whole type:

#![deny(rust_2018_idioms)]

struct Foo<'a, T>(&'a T);

fn foo<T>(_: Foo<T>) {}
error: hidden lifetime parameters in types are deprecated
 --> src/main.rs:5:14
  |
5 | fn foo<T>(_: Foo<T>) {}
  |              ^^^^^^ help: indicate the anonymous lifetime: `Foo<'_, T>`

Not sure if that's intended, but I found : <'_> a bit odd.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 2, 2019

📌 Commit c6e13bc has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 2, 2019
Centril added a commit to Centril/rust that referenced this pull request May 3, 2019
…ikomatsakis

Disallow non-explicit elided lifetimes in async fn

Fix rust-lang#60203

r? @nikomatsakis
bors added a commit that referenced this pull request May 3, 2019
Rollup of 12 pull requests

Successful merges:

 - #59928 (Make deprecation lint `ambiguous_associated_items` deny-by-default)
 - #60220 (report fatal errors during doctest parsing)
 - #60373 (Tidy: ensure lang features are sorted by since)
 - #60388 (Disallow non-explicit elided lifetimes in async fn)
 - #60393 ( Do not suggest incorrect syntax on pattern type error due to borrow)
 - #60401 (Rename `RUST_LOG` to `RUSTC_LOG`)
 - #60409 (Require a trait in the bounds of existential types)
 - #60455 (Resolve match arm ty when arms diverge)
 - #60457 (Const prop refactoring)
 - #60467 (Avoid repeated interning of static strings.)
 - #60478 (minor compiler doc tweaks)
 - #60501 (Propagate mutability from arguments to local bindings in async fn)

Failed merges:

r? @ghost
@bors bors merged commit c6e13bc into rust-lang:master May 3, 2019
@cramertj cramertj deleted the elided-lifetime-async branch May 3, 2019 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Async fns that take structs with elided lifetime parameters are broken.
6 participants