Skip to content

Fix elided lifetime resolution & trait object lifetime defaulting for enum variant paths#154918

Open
fmease wants to merge 2 commits intorust-lang:mainfrom
fmease:fix-enum-var-lt
Open

Fix elided lifetime resolution & trait object lifetime defaulting for enum variant paths#154918
fmease wants to merge 2 commits intorust-lang:mainfrom
fmease:fix-enum-var-lt

Conversation

@fmease
Copy link
Copy Markdown
Member

@fmease fmease commented Apr 7, 2026

Context: #129543 (comment).
Fixes #108224.

I consider this to be a minor bug fix that doesn't demand a (T-types) FCP.

r? @lcnr or reassign

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 7, 2026
let type_def_id = match res {
Res::Def(DefKind::AssocTy, def_id) if depth == 1 => Some(self.tcx.parent(def_id)),
Res::Def(DefKind::Variant, def_id) if depth == 0 => Some(self.tcx.parent(def_id)),
Res::Def(DefKind::Variant, def_id) if depth == 0 || depth == 1 => {
Copy link
Copy Markdown
Member Author

@fmease fmease Apr 7, 2026

Choose a reason for hiding this comment

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

We are in a position to fix this without any bad repercussions (namely breaking user code) due to this bug being covered by the lifetime elision bug that's fixed in the first commit and which dates all the way back to 2016 (#108224 (comment)).

That's because the new depth == 1 case can only be reached if the enum path segment has generic arguments (otherwise this function, visit_segment_args, wouldn't've been called in the first place) but due to #108224 that always leads to an error on stable/main.

Of course, if we pivot to dropping object lifetime defaulting in bodies (#129543 (review)), this issue should go away automatically.

Copy link
Copy Markdown
Member Author

@fmease fmease Apr 7, 2026

Choose a reason for hiding this comment

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

Once or if we stop utilizing object lifetime defaulting in bodies (cc #129543 (review)), this fix will become moot.

Admittedly, in the last paragraph of #129543 (comment) I stated that this fix would still be relevant for mGCA but I'm no longer super sure:

Indeed, trait object defaulting is generally relevant to mGCA, too (see open issue #151649), and so are enum variants in (direct) const args (which aren't backed by a body in which we could perform inference). However, I was unable to come up with a snippet where this fix is observable, at least not without "cheating" since dyn Trait can't be used as the type of const args (not sure if that'll change eventually?).

This commit makes the following artificial (!) mGCA snippet compile but I don't think it "counts".

#![feature(min_generic_const_args)]
#![feature(const_param_ty_unchecked)] // (!) internal, cherry-picked from open PR #153536
#![feature(generic_const_parameter_types, generic_const_items)]
#![feature(adt_const_params, unsized_const_params)]

#![allow(dead_code, incomplete_features, internal_features)]

enum E<'a, T: ?Sized + 'a> {
    V,
    H(&'a T),
}

// Before said commit, this would've failed with a type mismatch where
//     expected: `E<'_, (dyn Trait + 'static)>`
//        found: `E<'_, (dyn Trait + 'any)>`
type const _<'any>:
    E<'any, dyn Trait + 'any> =
        E::<'any, dyn Trait>::V {};

trait Trait {}

Copy link
Copy Markdown
Member Author

@fmease fmease Apr 7, 2026

Choose a reason for hiding this comment

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

Well, I was able to replace the usage of const_param_ty_unchecked (internal) with structual_match (perma? unstable) but that's hardly better:

#![feature(min_generic_const_args)]
#![feature(generic_const_parameter_types, generic_const_items)]
#![feature(adt_const_params, unsized_const_params)]
#![feature(structural_match)] // (!)

#![allow(dead_code, incomplete_features)]

enum E<'a, T: ?Sized + 'a> {
    V,
    H(&'a Discard<T>), // H(&'a ())
}

impl<'a, T: ?Sized> PartialEq for E<'a, T> {
    fn eq(&self, other: &Self) -> bool {
        // indeed identical to native structural equality
        std::mem::discriminant(self) == std::mem::discriminant(other)
    }
}

impl<'a, T: ?Sized> Eq for E<'a, T> {}
impl<'a, T: ?Sized> std::marker::StructuralPartialEq for E<'a, T> {}
impl<'a, T: ?Sized> std::marker::ConstParamTy_ for E<'a, T> {}

type Discard<T> = <T as DiscardT>::Output;
trait DiscardT { type Output; }
impl<T: ?Sized> DiscardT for T { type Output = (); }

// Before said commit, this would've failed with a type mismatch where
//     expected: `E<'_, (dyn Trait + 'static)>`
//        found: `E<'_, (dyn Trait + 'any)>`
type const _<'any>:
    E<'any, dyn Trait + 'any> =
        E::<'any, dyn Trait>::V {};

trait Trait {}

I guess I could add that as a test? Hmmm

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not following? The bottom test only has

error[E0109]: lifetime arguments are not allowed on variant `V`

Res::Def(DefKind::AssocTy, def_id) if depth == 1 => Some(self.tcx.parent(def_id)),
Res::Def(DefKind::Variant, def_id) if depth == 0 => Some(self.tcx.parent(def_id)),
Res::Def(DefKind::Variant, def_id) if depth == 0 || depth == 1 => {
Some(self.tcx.parent(def_id))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

FYI, after #129543, this will look something like:

Res::Def(DefKind::Variant, def_id) => match rev_seg_idx {
    0 => Some((self.tcx.parent(def_id), path.segments)),
    1 => Some((self.tcx.parent(def_id), &path.segments[..=seg_idx])),
    _ => None,
},

@fmease fmease force-pushed the fix-enum-var-lt branch from cf566ea to 980fbc0 Compare April 7, 2026 03:58
@lcnr
Copy link
Copy Markdown
Contributor

lcnr commented Apr 13, 2026

r? types

@rustbot rustbot added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Apr 13, 2026
@rustbot rustbot assigned jackh726 and unassigned lcnr Apr 13, 2026
Copy link
Copy Markdown
Member

@jackh726 jackh726 left a comment

Choose a reason for hiding this comment

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

I don't really understand your github comment + test. r=me on the changes, but I'm curious - seems like there may be some nuance I'm not appreciating.

View changes since this review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not following? The bottom test only has

error[E0109]: lifetime arguments are not allowed on variant `V`

Comment on lines +6 to +9
// Now we no longer do. Why do we synthesize these lifetimes for struct variant paths in the first
// place while we don't do it for unit & tuple variants? Well, we later HIR-ty-lower the former in
// "type mode" while we lower the latter in "value mode". In "type mode", we won't infer
// elided lifetimes if the generic arg lists contains non-lifetime args which is undesirable here.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, we later HIR-ty-lower the former in "type mode" while we lower the latter in "value mode".

ahahah omg why is this so cursed

I assume changing this to be "less weird" is just out of the question (definitely for this PR, but I mean that it's likely a big change)?

Comment on lines +11 to +14
let _ = Enum::Variant::<'any, dyn Trait> {};

// Similarly here.
let _ = Enum::<'any, dyn Trait>::Variant {};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh you can just write both of these? Lol, okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Braced variant ctor gets rejected if enum has lifetime params & you pass gen args to enum explicitly

4 participants