Skip to content

Oddity with lifetime elision and type aliases #140611

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

Open
traviscross opened this issue May 3, 2025 · 9 comments
Open

Oddity with lifetime elision and type aliases #140611

traviscross opened this issue May 3, 2025 · 9 comments
Labels
A-lifetimes Area: Lifetimes / regions C-bug Category: This is a bug. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. I-types-nominated Nominated for discussion during a types team meeting. needs-crater This change needs a crater run to check for possible breakage in the ecosystem. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team

Comments

@traviscross
Copy link
Contributor

traviscross commented May 3, 2025

In reviewing a test case for,

I was led to the following oddity:

pub struct W<'a>(&'a ());
pub type Alias<'a> = W<'a>;

impl<'a> Alias<'a> {
    fn f1<'x>(self: &W<'a>, x: &'x ()) -> &() { x } //~ `'_ == 'x`, what?
    fn f2<'x>(self: &Alias<'a>, x: &'x ()) -> &() { x } //~ `'_ == 'x`, what?
    fn f3<'x>(&self, _: &'x ()) -> &() { self.0 } //~ OK.
}

impl<'a> W<'a> {
    fn f4<'x>(self: &W<'a>, _: &'x ()) -> &() { self.0 } //~ OK.
    fn f5<'x>(self: &Alias<'a>, x: &'x ()) -> &() { x } //~ `'_ == 'x`, what?
    fn f6<'x>(&self, _: &'x ()) -> &() { self.0 } //~ OK.
}

Playground link

This was noticed long ago in #60944 (comment), which is presumably when the test case was added, but I can't immediately find later discussion.

The Reference doesn't document this behavior. (The FLS, as best I can tell on a skim, does not document lifetime elision at all.)

If there's a good reason for this as the correct behavior, we should probably write this down in the Reference. Otherwise, I'm going to propose we agree to try to do away with this somehow, if possible, maybe over an edition.

@rustbot labels +C-discussion +T-lang +I-lang-nominated +A-lifetimes

cc @rust-lang/lang @ehuss

@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. A-lifetimes Area: Lifetimes / regions C-discussion Category: Discussion or questions that doesn't represent real issues. I-lang-nominated Nominated for discussion during a lang team meeting. T-lang Relevant to the language team labels May 3, 2025
@traviscross traviscross removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 3, 2025
@Veykril
Copy link
Member

Veykril commented May 3, 2025

(The FLS, as best I can tell on a skim, does not document lifetime elision at all.)

It does, but it also does not capture this oddity I think: https://github.com/rust-lang/fls/blob/main/src/types-and-traits.rst#lifetime-elision

@traviscross
Copy link
Contributor Author

traviscross commented May 6, 2025

Interestingly, rust-analyzer, when configured to show inlay lifetime hints, gets f1-f3 and f5-f6 correct (i.e. matches the behavior of rustc) but gets f4 wrong.

That is, it annotates it like this:

impl<'a> Alias<'a> {
    fn f1<'x>(self: &W<'a>, x: &'x ()) -> &'x () { x } //~ `'_ == 'x`, what?
    fn f2<'x>(self: &Alias<'a>, x: &'x ()) -> &'x () { x } //~ `'_ == 'x`, what?
    fn f3<'0, 'x>(&self, _: &'x ()) -> &'0 () { self.0 } //~ OK.
}

impl<'a> W<'a> {
    fn f4<'x>(self: &W<'a>, _: &'x ()) -> &'x () { self.0 } //~ Wrong r-a annotation.
    fn f5<'x>(self: &Alias<'a>, x: &'x ()) -> &'x () { x } //~ `'_ == 'x`, what?
    fn f6<'0, 'x>(&self, _: &'x ()) -> &'0 () { self.0 } //~ OK.
}

@Veykril
Copy link
Member

Veykril commented May 7, 2025

Those inlay hints are entirely syntactically based and known to be wrong in some scenarios (r-a has not implemented semantic lifetime elision yet)

@joshtriplett
Copy link
Member

We discussed this in today's @rust-lang/lang meeting. This seems emphatically like a bug. We should try to fix it, and do a crater run to see if fixing it breaks anything.

@traviscross traviscross added C-bug Category: This is a bug. and removed C-discussion Category: Discussion or questions that doesn't represent real issues. labels May 7, 2025
@joshtriplett joshtriplett added the I-compiler-nominated Nominated for discussion during a compiler team meeting. label May 7, 2025
@traviscross traviscross added A-lifetimes Area: Lifetimes / regions T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. I-compiler-nominated Nominated for discussion during a compiler team meeting. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. T-lang Relevant to the language team and removed A-lifetimes Area: Lifetimes / regions T-lang Relevant to the language team C-bug Category: This is a bug. I-lang-nominated Nominated for discussion during a lang team meeting. I-compiler-nominated Nominated for discussion during a compiler team meeting. labels May 7, 2025
@joshtriplett joshtriplett added the needs-crater This change needs a crater run to check for possible breakage in the ecosystem. label May 7, 2025
@apiraino
Copy link
Contributor

apiraino commented May 15, 2025

We think that T-types is more fit to discuss this (I am at RustWeek with @Urgau 🙂 ) so:

@rustbot label -I-compiler-nominated +I-types-nominated

@rustbot rustbot added I-types-nominated Nominated for discussion during a types team meeting. and removed I-compiler-nominated Nominated for discussion during a compiler team meeting. labels May 15, 2025
@compiler-errors
Copy link
Member

compiler-errors commented May 15, 2025

This is two bugs:

(1.) Lifetime elision special-casing for self: &Ty only works for structs

Res::Def(DefKind::Struct | DefKind::Union | DefKind::Enum, _,) | Res::PrimTy(_)

That is to say, we don't prefer the lifetime in self: &Alias even if we're in some impl Alias because we only prefer structs/unions/enums in this hack.

(2.) Lifetime elision special-casing does not look through type aliases

We prefer the lifetime in self: &Struct only in impl Struct, only considering syntactical equality between the impl self type and the type inside the reference.

TyKind::Path(None, _) => {
let path_res = self.r.partial_res_map[&ty.id].full_res();
if let Some(Res::SelfTyParam { .. } | Res::SelfTyAlias { .. }) = path_res {
return true;
}
self.impl_self.is_some() && path_res == self.impl_self
}


I believe (1.) could be easily tweaked to also consider aliases. That would fix:

pub struct W<'a>(&'a ());
pub type Alias<'a> = W<'a>;

impl<'a> Alias<'a> {
    fn f2<'x>(self: &Alias<'a>, x: &'x ()) -> &() { self.0 }
}

I don't believe (2.) could be fixed to look through aliases, though, since this lifetime elision happens pretty early in the compiler, and I'm not sure we have the information to look through type aliases yet.


I'm kinda unsure why we even support this partial "is self type" behavior. It's pretty inconsistent, i.e. we prefer the self lifetime if the self type matches the struct of the only shallowly i.e. W<i32> would match W<()> and prefer its lifetime in self: &W<i32>.

I think it would've only been nice to support lifetimes when users write self: &Self and not self: &Struct (when the impl self is Struct).

I didn't look for much justification in the following PRs, but maybe someone had thoughts there:

@traviscross
Copy link
Contributor Author

traviscross commented May 15, 2025

Thanks for those details and that analysis. In looking through #61207, I see there is this write-up by @nikomatsakis trying to document the then-current behavior:

https://gist.github.com/nikomatsakis/5996e4f58899ef5c6926948b6f76835e

@lcnr
Copy link
Contributor

lcnr commented May 20, 2025

I agree with errs that fixing f2 seems quite doable. I also think that resolving aliases to their underlying type is difficult at the point where we handle lifetime elision.

I believe that we can future compat lint changes to our lifetime elision rules. So if we decide to only do the self-type lifetime elision if there's an actual Self in there, then we could keep the existing behavior with a FCW. I separately think that we may want to lint on self: X where X does not contain Self. I don't know the impact of this change so we'd probably need to crater run to get an estimate first

@lcnr
Copy link
Contributor

lcnr commented May 20, 2025

Generally, I think lifetime elisions is one of the areas where there are a lot of not-nice edge-cases right now and no awesome solution is possible. I personally don't think we should spend too much of our time on it and should prioritize other things as a project. Though ofc individual contributor is more than welcome to dive into this regardless. I would still love this area to be cleaned up.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 20, 2025
use `Self` alias in self types rather than manually substituting it

Of the rougly 145 uses of `self: Ty` in the standard library, 5 of them don't use `Self` but instead choose to manually "substitute" the `impl`'s self type into the type.

This leads to weird behavior sometimes (rust-lang#140611 (comment)) -- **to be clear**, none of these usages actually trigger any bugs, but it's possible that they may break in the future (or at least lead to lints), so let's just "fix" them proactively.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue May 21, 2025
Rollup merge of rust-lang#141289 - compiler-errors:more-self, r=jhpratt

use `Self` alias in self types rather than manually substituting it

Of the rougly 145 uses of `self: Ty` in the standard library, 5 of them don't use `Self` but instead choose to manually "substitute" the `impl`'s self type into the type.

This leads to weird behavior sometimes (rust-lang#140611 (comment)) -- **to be clear**, none of these usages actually trigger any bugs, but it's possible that they may break in the future (or at least lead to lints), so let's just "fix" them proactively.
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this issue May 22, 2025
use `Self` alias in self types rather than manually substituting it

Of the rougly 145 uses of `self: Ty` in the standard library, 5 of them don't use `Self` but instead choose to manually "substitute" the `impl`'s self type into the type.

This leads to weird behavior sometimes (rust-lang/rust#140611 (comment)) -- **to be clear**, none of these usages actually trigger any bugs, but it's possible that they may break in the future (or at least lead to lints), so let's just "fix" them proactively.
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this issue May 23, 2025
use `Self` alias in self types rather than manually substituting it

Of the rougly 145 uses of `self: Ty` in the standard library, 5 of them don't use `Self` but instead choose to manually "substitute" the `impl`'s self type into the type.

This leads to weird behavior sometimes (rust-lang#140611 (comment)) -- **to be clear**, none of these usages actually trigger any bugs, but it's possible that they may break in the future (or at least lead to lints), so let's just "fix" them proactively.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: Lifetimes / regions C-bug Category: This is a bug. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. I-types-nominated Nominated for discussion during a types team meeting. needs-crater This change needs a crater run to check for possible breakage in the ecosystem. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team
Projects
None yet
Development

No branches or pull requests

7 participants