Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Mar 21, 2018

You can now do things like:

impl MyTrait<'_> for &u32 { ... }

Each '_ or elided lifetime is a fresh parameter. '_ and elision are still not permitted in associated type values. (Plausibly we could support that if there is a single input lifetime.) The original lifetime elision RFC was a bit unclear on this point: as documented here, I think this is the correct interpretation, both because it fits existing impls and it's most analogous to the behavior in fns.

We do not support elision with deprecated forms:

impl MyTrait for std::cell::Ref<u32> { } // ERROR

Builds on the in-band lifetime stuff.

r? @cramertj

Fixes #15872

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 21, 2018
@nikomatsakis
Copy link
Contributor Author

Oh, still running tests locally.

@nikomatsakis nikomatsakis force-pushed the issue-15872-elision-impl-header branch 4 times, most recently from 1873a9f to b149319 Compare March 22, 2018 00:04
@nikomatsakis
Copy link
Contributor Author

Looks good!

@bors
Copy link
Collaborator

bors commented Mar 22, 2018

☔ The latest upstream changes (presumably #49264) made this pull request unmergeable. Please resolve the merge conflicts.

@nikomatsakis nikomatsakis force-pushed the issue-15872-elision-impl-header branch from b149319 to 94468da Compare March 22, 2018 20:55
lt_defs: &[LifetimeDef],
f: F
) -> T where F: FnOnce(&mut LoweringContext) -> T
lt_defs: impl Iterator<Item = &'l LifetimeDef>,
Copy link
Member

Choose a reason for hiding this comment

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

Huh-- this is interesting. Seems like this would probably trigger #44752, but it's an error to omit the lifetime. We should probably be able to make argument-position impl Trait elided lifetimes "just work", but it's backcompat to fix, so I'm not super concerned about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should probably be able to make argument-position impl Trait elided lifetimes "just work", but it's backcompat to fix, so I'm not super concerned about it.

Yeah, that was curious, but I came to the same conclusion. I think clearly they should work as a fresh lifetime (but I feel the same about '_ in where clauses).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should probably file an issue about it.

Copy link
Member

Choose a reason for hiding this comment

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

Filed #49287

@cramertj
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 22, 2018

📌 Commit 94468da has been approved by cramertj

@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 Mar 22, 2018
@kennytm
Copy link
Member

kennytm commented Mar 24, 2018

@bors p=1

@bors
Copy link
Collaborator

bors commented Mar 24, 2018

⌛ Testing commit 94468da with merge b4aa80d...

bors added a commit that referenced this pull request Mar 24, 2018
…r=cramertj

support elision in impl headers

You can now do things like:

```
impl MyTrait<'_> for &u32 { ... }
```

Each `'_` or elided lifetime is a fresh parameter. `'_` and elision are still not permitted in associated type values. (Plausibly we could support that if there is a single input lifetime.) The original lifetime elision RFC was a bit unclear on this point: [as documented here, I think this is the correct interpretation, both because it fits existing impls and it's most analogous to the behavior in fns](#15872 (comment)).

We do not support elision with deprecated forms:

```
impl MyTrait for std::cell::Ref<u32> { } // ERROR
```

Builds on the in-band lifetime stuff.

r? @cramertj

Fixes #15872
@bors
Copy link
Collaborator

bors commented Mar 24, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: cramertj
Pushing b4aa80d to master...

@bors bors merged commit 94468da into rust-lang:master Mar 24, 2018
This was referenced Mar 24, 2018
bors added a commit that referenced this pull request Aug 6, 2018
…atsakis

Extract impl_header_lifetime_elision out of in_band_lifetimes

This way we can experiment with `impl Debug for &MyType` separately from `impl Debug for &'a MyType`.

I can't say I know what the code in here is doing, so please let me know if there's a better way 🙂

I marked this as enabled in 2018 so that edition code continues to work without another flag.

Actual feature PR #49251; Tracking Issue #15872; In-band lifetimes tracking issue #44524.

cc @aturon, per discussion on discord earlier
cc @cramertj & @nikomatsakis, who actually wrote these features
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.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.