-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Stabilize anonymous_lifetime_in_impl_trait
#107378
base: master
Are you sure you want to change the base?
Conversation
r? @estebank (rustbot has picked a reviewer for you, use r? to override) |
edit: I retract the review switch (I had missed the comments in the review) @rustbot ready |
Well, I am actually waiting for a FCP that should be started by the lang or compile team but didn't receive any feedback yet 🤷 |
Submitting to lang team consideration for stabilization. |
We discussed this in today's @rust-lang/lang meeting, and we think this is ready for an FCP to merge: @rfcbot merge We'd also like to make sure that future work on type-alias impl Trait (TAIT) doesn't automatically assume anonymous lifetimes will work there, and thinks carefully about how or if that should work. |
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
Just to confirm, the following snippet that is currently allowed will be denied due to possible future compatibility issues. #![feature(anonymous_lifetime_in_impl_trait, type_alias_impl_trait)]
type Foo<'a> = impl IntoIterator<Item = &'a i32>;
// TAIT won't work without this function
pub fn type_resolution<'a>(slice: &'a [i32]) -> Foo<'a> {
slice
}
pub fn bar<'a>(slice: &'a [i32]) {
foo([type_resolution(slice)]);
}
// Implies foo<'a>(_: impl IntoIterator<Item = Foo<'a>) {}
pub fn foo(_: impl IntoIterator<Item = Foo>) {} If that is really the case, then I will create a PR to address such concern. |
☔ The latest upstream changes (presumably #108145) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me on the changes after t-lang approval and rebase
Lifetime elision seems to ignore the anonymous lifetime parameter introduced. async fn does_not_compile(mut iter: impl Iterator<Item = &()>) -> &() {
iter.next().unwrap()
}
async fn compiles<'a>(mut iter: impl Iterator<Item = &'a ()>) -> &'a () {
iter.next().unwrap()
}
async fn wrong_error(iter: &mut impl Iterator<Item = &()>) -> &() {
iter.next().unwrap()
} Error outputerror[[E0106]](https://doc.rust-lang.org/stable/error_codes/E0106.html): missing lifetime specifier
--> src/lib.rs:1:67
|
1 | async fn does_not_compile(mut iter: impl Iterator<Item = &()>) -> &() {
| ^ expected named lifetime parameter
|
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
help: consider using the `'static` lifetime
|
1 | async fn does_not_compile(mut iter: impl Iterator<Item = &()>) -> &'static () {
| +++++++
error: lifetime may not live long enough
--> src/lib.rs:2:5
|
1 | async fn does_not_compile(mut iter: impl Iterator<Item = &()>) -> &() {
| --- return type `impl Future<Output = &'static ()>` contains a lifetime `'1`
2 | iter.next().unwrap()
| ^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'static`
error: lifetime may not live long enough
--> src/lib.rs:10:5
|
9 | async fn wrong_error(iter: &mut impl Iterator<Item = &()>) -> &() {
| - --- return type `impl Future<Output = &()>` contains a lifetime `'1`
| |
| let's call the lifetime of this reference `'2`
10 | iter.next().unwrap()
| ^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1`
For more information about this error, try `rustc --explain E0106`.
error: could not compile `playground` due to 3 previous errors And similarly for non- #![feature(anonymous_lifetime_in_impl_trait)]
trait Trait<'a> {
fn foo(&self) -> &'a str { "" }
}
// Comment this to see the other two errors
pub fn f(t: impl Trait<'_>) -> &str {
t.foo()
}
pub fn g(t: &impl Trait<'_>) -> &str {
t.foo()
}
pub fn parse_reg(it: &mut impl Iterator<Item=&Token>) -> Result<&str, String> {
let c = it.next().unwrap();
match c {
Token::Text(text) => Ok(text.as_str()),
_ => Err(format!("Expected register got {:?}", c))
}
} Error outputerror[[E0106]](https://doc.rust-lang.org/nightly/error_codes/E0106.html): missing lifetime specifier
--> src/lib.rs:8:32
|
8 | pub fn f(t: impl Trait<'_>) -> &str {
| ^ expected named lifetime parameter
|
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
help: consider using the `'static` lifetime
|
8 | pub fn f(t: impl Trait<'_>) -> &'static str {
| +++++++
For more information about this error, try `rustc --explain E0106`. error: lifetime may not live long enough
--> src/lib.rs:15:5
|
14 | pub fn g(t: &impl Trait<'_>) -> &str {
| - - let's call the lifetime of this reference `'2`
| |
| has type `t`
15 | t.foo()
| ^^^^^^^ function was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1`
error: lifetime may not live long enough
--> src/lib.rs:27:30
|
24 | pub fn parse_reg(it: &mut impl Iterator<Item=&Token>) -> Result<&str, String> {
| -- - let's call the lifetime of this reference `'2`
| |
| has type `it`
...
27 | Token::Text(text) => Ok(text.as_str()),
| ^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1`
error: could not compile `playground` (lib) due to 2 previous errors The feature should IMO treat the introduced anonymous lifetime like one introduced by a reference.
|
This was discussed in the lang team meeting yesterday (notes). We felt that the PR description needed more detail before we can decide on stabilizing this. As in the Zulip discussion, it seems there are multiple ways of interpreting this elision. fn _example(_: impl Iterator<Item = &()>) {}
There needs to be an argument why (1) is the correct behavior to stabilize, if we go with that. @rfcbot concern why-not-higher-rank We'd also like the PR to lay out how anonymous lifetimes will behave in the following cases (and other interesting cases if we didn't think of them):
as well as whether/how we might extend this behavior to where clauses. @rfcbot concern elaborate-cases-and-future-directions |
Well, it is a surprising statement because my understanding is that all rules governing anonymous lifetimes in non-async functions should equal their async counterparts that is currently stable. Any difference between both will probably induce unnecessary confusion for users. Anyway, I will try to elaborate a report taking into consideration the above assumption as well as the team's concerns in the following days/weeks. |
@nikomatsakis Following the same line of reasoning from my comment above, GATs are clearly different in that their parameters can't be determined ahead of time by the caller. Take the example fn fun(x: impl LendingIterator<Item = &u32>) { // or, if you prefer: Item<'_> = &'_ u32
for _ in x {}
} By construction, our callee It seems like our options for GATs are:
I personally don't see a need to allow elided lifetimes in arguments to GATs right away, so I would propose going with option 1 for now. I think the positions we've discussed all interact differently with
The reasoning about trait parameters in my post above rests on a lack of common examples where trait parameters are not used in the |
I'm thinking here -- I think I actually have two concerns:
That said, looking purely at types, our rules are that
Agreed! Though I'd like to see the patterns where people do want the I added your I think one thing that might be useful -- I've tried to do this in the past but I never got it to quite work out -- is to describe the "underlying logic" for elision in a way that naturally expands to other locations. I would be curious to get a sense for what "your average experienced Rust user" expects things to mean. Maybe we can design an experiment. :) |
I did some grepping around the compiler and found that:
Also, in addition to accidentally stabilizing
|
I've been thinking about tmandry's point -- in other words, why do people put lifetime parameters on traits anyway. @tmandry is correct that the role of such a parameter is to capture a lifetime that (may) appear in the self type (or some other input type) and which we wish to name in the interface. I think part of the rub is that often this lifetime may or may not appear, and it's useful to distinguish that (this is the Serde Looking at e.g. the pub trait DecorateLint<'a, G: EmissionGuarantee> {
/// Decorate and emit a lint.
fn decorate_lint<'b>(
self,
diag: &'b mut DiagnosticBuilder<'a, G>,
) -> &'b mut DiagnosticBuilder<'a, G>;
fn msg(&self) -> DiagnosticMessage;
} Presumably, the reason that this trait has the impl<'a> DecorateLint<'a, ()> for BuiltinMissingDebugImpl<'_> (And this is presumably why we see (I'm assuming clippy or external code bases may be different? If not, this trait just seems more complex than it has to be.) |
OK, I'm feeling quite effectively nerdswiped by this issue. In theory I'm on vacation today but I am still thinking about it. :) Here are some more discombulated thoughts that I don't have time to knit into a coherent story. There are really a few issues at play here. One of them is elided lifetimes the value of an associated type, which may well reference lifetimes that appear in the trait input types. This is relatively clear cut for a trait like It's less clear when you have GATs or lifetime parameters on the trait. In cases like The other case is when you have a lifetime parameter on a trait. That is relatively unusual and (I think) it is sort of ill understood when it makes sense. The basic reason to have a lifetime parameter in a trait is if you want it to appear in a function signature (e.g., the Reason 1a: The lifetime appears in your interface and is tied to one of your input types. This is the Reason 1b: Same as above, but the lifetime is only sometimes necessary. This corresponds to Reason 2: The lifetime appears in your function inputs and also in the value of an associated type. This is what happens with the All of this is definitely making me want to do a bit of a "study" of common patterns across where-clauses, associated types, etc. I'm also finding myself grasping a bit towards some design tenets or principles, though I don't know the ordering yet...
...as I was thinking before, it may be that the best answer to get past |
In the interest of incremental progress: I wonder about stabilizing i.e., fn foo(x: impl Iterator<Item = &u32>) // equivalent to `'a` in `fn foo<'a>`
fn foo(&self) -> impl Iterator<Item = &u32> // equivalent to `'a` in `fn foo<'a>(&'a self)`
fn foo<'b>(x: impl Iterator<'b, Item = &u32>) // equivalent to `'a` in `fn foo<'a>` (NOT `'b`)
// NOT STABILIZED
fn foo(x: impl LendingIterator<Item<'_> = &u32>)
fn foo(x: impl FooBar<_, Item = &u32> Also, ideally, async fn and fn would behave the same. This definitely commits us to a certain sort of principle I'd like to articulate but I really have to stop leaving comments. :) But it seems like a principle I feel pretty comfortable with and reasonably sure we are going to want. |
I would be happy to start with the subset that @nikomatsakis proposed. @c410-f3r are you interested in making those changes? |
efad34f
to
23d76b8
Compare
Personally, I need more time to digest all the information, but at first glance it seems that the current behaviour matches the desired subset. However, if it is necessary to perform more than a feature removal and/or test management, then I probably won't have the time to empirically hack the compiler to achieve the goals in the near future. |
This comment has been minimized.
This comment has been minimized.
This proposal is to only allow elided lifetimes in equality bounds of non-generic associated types. I think the changes from the current behavior would be:
|
☔ The latest upstream changes (presumably #118823) made this pull request unmergeable. Please resolve the merge conflicts. |
@c410-f3r any updates on this? thanks |
I will try to get back to this PR next week |
In regards to not stabilizing elided lifetimes in trait generics, the following is already allowed today. trait FooBar<'a> {
type Item;
}
async fn foo(_: impl FooBar<'_, Item = &u32>) {} So leaving this case out for non-async functions does not help with the lack of parity. If the elaboration of elided rules is not trivial and needs more investigation, then the accidental stabilization of elided lifetimes for asynchronous functions should probably be re-considered and possibly partially-reverted. |
70abdc8
to
5717003
Compare
5717003
to
6a202f5
Compare
@rustbot labels +I-lang-nominated We unnominated this back in October 2023 as more analysis seemed to be needed. Since then, @nikomatsakis and @tmandry have posted substantive analysis that it seems we should discuss. |
☔ The latest upstream changes (presumably #129841) made this pull request unmergeable. Please resolve the merge conflicts. |
@c410-f3r |
Six months ago the suggestions of #107378 (comment) were properly addressed with one exception as commented in #107378 (comment). The following allowed signatures are tested in the fn foo(x: impl Iterator<Item = &u32>) // equivalent to `'a` in `fn foo<'a>`
fn foo(&self) -> impl Iterator<Item = &u32> // equivalent to `'a` in `fn foo<'a>(&'a self)` GAT-related signatures are tested and hopefully correctly ignored in the So, if This PR is waiting for feedback, which is similar to the situation of #122808 (comment) |
Stabilization proposal
This PR proposes the stabilization of
#![feature(anonymous_lifetime_in_impl_trait)]
.Version: 1.69 (beta => 2023-03-09, stable => 2023-04-20).
What is stabilized
For non-asynchronous functions, allows the usage of anonymous lifetimes in APITs.
Motivation
In addition to ergonomics, the lack of parity between asynchronous and non-asynchronous functions can be confusing as the former is already allowed on stable toolchains.
History
anonymous_lifetime_in_impl_trait
cc @cjgillot