Skip to content

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

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.

Sign up for GitHub

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

Merged
rust-bors merged 2 commits into from
May 12, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed60e2a
Don't synthesize lifetime args for variant paths if generic args were…
fmease Apr 7, 2026
d39d91f
Make enum variant paths induce trait object lifetime defaults in the …
fmease Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs
View file
Open in desktop

@fmease fmease Apr 7, 2026 •
edited
Loading

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.

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 {}

@fmease fmease Apr 7, 2026 •
edited
Loading

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.

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

@jackh726 jackh726 Apr 23, 2026

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`

@fmease fmease May 12, 2026 •
edited
Loading

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.

Both snippets get rejected with "lifetime arguments are not allowed on variant V" on nightly / main (!) due to the elision bug that's fixed in the first commit of this PR.

I'm sorry that I didn't make this clearer but you're meant to assume that the elision bug was already fixed // you're meant to feed it to rustc patched with commit (0) "Don't synthesize lifetime args for variant paths if generic args were passed to the enum itself already" but not yet patched with commit (1) "Make enum variant paths induce trait object lifetime defaults in the enum path segment".

Original file line number Diff line number Diff line change
Expand Up @@ -1691,7 +1691,9 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
// which requires object lifetime defaults.
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 => {

@fmease fmease Apr 7, 2026 •
edited
Loading

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.

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.

Some(self.tcx.parent(def_id))

@fmease fmease Apr 7, 2026

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,
},

}
Res::Def(
DefKind::Struct
| DefKind::Union
Expand Down
10 changes: 9 additions & 1 deletion compiler/rustc_resolve/src/late.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2158,7 +2158,15 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
Res::Def(DefKind::AssocTy, def_id) if i + 2 == proj_start => {
self.r.tcx.parent(def_id)
}
Res::Def(DefKind::Variant, def_id) if i + 1 == proj_start => {
Res::Def(DefKind::Variant, def_id)
if i + 2 == proj_start && segment.has_generic_args =>
{
self.r.tcx.parent(def_id)
}
Res::Def(DefKind::Variant, def_id)
if i + 1 == proj_start
&& !i.checked_sub(1).is_some_and(|i| path[i].has_generic_args) =>
{
self.r.tcx.parent(def_id)
}
Res::Def(DefKind::Struct, def_id)
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_resolve/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,7 @@ enum VisResolutionError<'a> {
struct Segment {
ident: Ident,
id: Option<NodeId>,
/// Signals whether this `PathSegment` has generic arguments. Used to avoid providing
/// nonsensical suggestions.
/// Signals whether this `PathSegment` has generic arguments.
has_generic_args: bool,
/// Signals whether this `PathSegment` has lifetime arguments.
has_lifetime_args: bool,
Expand Down
37 changes: 37 additions & 0 deletions tests/ui/lifetimes/no-lt-elision-in-variant-path-seg-as-enum-path-seg-has-gen-args.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Given an enum `Enum` with lifetime parameters, we used to lower `Enum::<…>::Variant {}` to
// `Enum::<…>::Variant::<'_, …> {}`, i.e., synthesize `'_` lifetime args for enum variants
// even if generic args were already passed to the enum itself.
// Obviously, these conflicting generic arg lists were then rejected later on (by HIR ty lowering).
//
// 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.
Comment on lines +6 to +9

@jackh726 jackh726 Apr 23, 2026

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)?

@fmease fmease May 12, 2026 •
edited
Loading

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.

Yes, that'll likely be quite the overhaul and very much out of scope I would say.

For some reason, the lowering of DefKind::Variant paths is shoehorned into lower_resolved_ty_path that's meant for HIR-ty-lowering (ASTconv'ing) paths referring to types contrary to DefKind::Ctor(Variant, Const | Fn) paths that get handled in HIR typeck in instantiate_value_path. As such, it uses lowering helpers that are meant for types like lower_path_segment1. Sure, you might be able to change the mode from "type" to "value" (I haven't tried that out & don't know the consequences) by parametrizing some of the preexisting lowering helpers but that further muddies the waters.

It's possible that the code lives in hir_ty_lowering directly instead of in HIR typeck in preparation for variant types; we already have some logic that detects whether a given associated type would conflict with an enum variant due to that.

Still, there ought to be a slightly more principled way for resolving variant paths I hope. For context, for mGCA which features type-level variant paths (const exprs), we currently mirror / duplicate a lot of code of lower_resolved_ty_path in lower_resolved_const_path, that I'm fine with since it's more principled (we do share a lot of code with the type counterpart, not to worry).

Footnotes

  1. which calls lower_generic_args_of_path_segment which calls lower_generic_args_of_path which calls check_generic_arg_count "in type mode". ↩

//
// issue: <https://github.com/rust-lang/rust/issues/108224>
//@ check-pass

fn scope<'any>() {
enum Ty0<'a> {
Unit,
Tuple,
Struct {},

Carry(&'a ()),
}

enum Ty1<'a, T: ?Sized> {
Variant,

Carry(&'a T),
}

let _ = Ty0::<'_>::Unit {};
let _ = Ty0::<'static>::Tuple {};
let _ = Ty0::<'any>::Struct {};

let _ = Ty1::</*'_, */()>::Variant {};
let _ = Ty1::<'any, ()>::Variant {};
}

fn main() {}
24 changes: 24 additions & 0 deletions tests/ui/object-lifetime/object-lifetime-default-enum-variant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//@ check-pass

enum Enum<'a, T: 'a + AbideBy<'a> + ?Sized> {
Variant,

Carry(&'a T),
}

fn scope<'any>() {
// We elaborate `dyn Trait` to `dyn Trait + 'any` due to bound `'a` on `T`.
let _ = Enum::Variant::<'any, dyn Trait> {};

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

@jackh726 jackh726 Apr 23, 2026

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.

}

trait Trait {}

// We use this to test that a given trait object lifetime bound is
// *exactly equal* to a given lifetime (not longer, not shorter).
trait AbideBy<'a> {}
impl<'a> AbideBy<'a> for dyn Trait + 'a {}

fn main() {}
Loading