feat: implement lifetime elision by dfireBird · Pull Request #22927 · rust-lang/rust-analyzer

dfireBird · 2026-07-27T03:38:12Z

No description provided.

dfireBird · 2026-07-27T04:25:06Z

Not sure why this fails in CI and passes in my system?

ShoyuVanilla · 2026-07-27T05:17:49Z

Not sure why this fails in CI and passes in my system?

rust-analyzer/docs/book/src/contributing/README.md

Line 67 in 85e85df

Use `env RUN_SLOW_TESTS=1 cargo test` to run the full suite.

You need to set an extra env var to run slow tests

dfireBird · 2026-07-27T05:58:17Z

Oh I didn't know, we ran slow tests. My bad 😅

Veykril

Only a partial review so far

View changes since this review

Veykril · 2026-07-27T07:59:35Z

 fn bar(x: &u32) {}
 "#,
-        expect!["ty: &'_ &'_ u32, name: x"],
+        expect!["ty: &'_ &'<erased> u32, name: x"],


we probably need to check our rendering now, we shouldn't show erased lifetimes to the user

I could make the RegionKind::ReErased to display '_ for now, would that be fine?

Veykril · 2026-07-27T08:00:37Z

+                lc world &WorldSnapshot [type_could_unify+name+local]
+                ex world  [type_could_unify]


Interesting, so we lose a lot of type equality now which I guess makes sense. Worth to keep in mind if we want t o have something like type-equal modulo lifetimes

Veykril · 2026-07-27T08:04:29Z

Very excited for this, I've been wanting this for so long!

ChayimFriedman2

Round 1 review.

View changes since this review

ChayimFriedman2 · 2026-07-28T19:09:33Z

+                            {
+                                TypeBound::ForLifetime(binder, path)
+                            } else {
+                                TypeBound::Path(path, m)


Do we even need TypeBound::Path now? After all in rustc_type_ir every predicate has a binder. We could just always produce ForLifetime.

dfireBird · 2026-08-03T06:00:29Z

Two tests will be failing, one of which I think value in expect is correct but after infer it gets turned into a RegionKind::Error, which make the test fail (within the expect)
Other fails because of failure of const eval query, because we are passing empty generic args to query (here), not sure how to fix this or should we let the const eval fail?.

ChayimFriedman2 · 2026-08-03T06:12:59Z

Const eval doesn't care about lifetimes, as long as you only have lifetimes there you can pass erased or identity.

dfireBird · 2026-08-03T11:48:25Z

I'm not sure what you mean?

ChayimFriedman2 · 2026-08-03T13:40:56Z

If all args are lifetime, you can pass a dummy GenericArgs, with RegionKind::ReErased or GenericArgs::identity_for_item().

dfireBird · 2026-08-03T15:14:44Z

Passing the GenericArgs::identity_for_item() (let me know if I made a mistake there, once I push) make the test pass as is. Thanks for the help.

dfireBird · 2026-08-03T16:48:34Z

Found out why we get error region on other test, when we resolve the types, we fold the ReVar into a ReError here: https://github.com/rust-lang/rust-analyzer/blob/master/crates/hir-ty/src/infer/unify.rs#L705-L713, then I think this is expected behavior for this test, if so then I'll revert the expect on that test as well?

minor fixes related to review comments

rustbot · 2026-08-04T02:39:40Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Veykril · 2026-08-04T09:42:57Z

+    fn elide_return_lifetime(&mut self) {
+        let old_elision_kind =
+            mem::replace(&mut self.lifetime_elision_kind, LifetimeElisionKind::Error);
+
+        let new_elision_kind =
+            if let LifetimeElisionKind::NewLifetimeParam { return_lt, total_created, .. } =
+                old_elision_kind
+            {
+                let lifetime_param_id =
+                    return_lt.and_then(|(lifetime_param_id, elided_source)| match elided_source {
+                        ArgumentElisionContext::Self_ => Some(lifetime_param_id),
+                        ArgumentElisionContext::Param if total_created == 1 => {
+                            Some(lifetime_param_id)
+                        }
+                        ArgumentElisionContext::Param => None,
+                    });
+                LifetimeElisionKind::Lifetime(lifetime_param_id)
+            } else {
+                unreachable!("Method should not be called with other lifetime_elision_kind")
+            };
+        self.lifetime_elision_kind = new_elision_kind;
+    }


This setup doesn't quite handle these correctly i think

fn get<'a>(&'a self) -> &str fn identity<'a>(value: &'a str) -> &str type Callback = for<'a> fn(&'a str) -> &str;

These are valid signatures that participate in elision. The return types here all have 'a as their elided lifetimes.

And for this it incorrectly elides to y's lifetime.

fn bad<'a>(x: &'a str, y: &str) -> &str

That signature is an error, return type elision only happens if the args have exactly one lifetime (or if there is a self param).

Reason being that lower_lifetime_ref doesn't participate (from what I can tell in lifetime_elision_kind).

View changes since the review

I have done the return type elision very haphazardly, I should take a look at it and fix it.

I think I should introduce a new LifetimeRef for elided lifetimes or designate LifetimeRef:::Param and LifetimeRef::HrtbParam only to be used for elided (by renaming or something). Because with the fix I'm doing for this, a function like this:

fn identity<'a>(value: &'a str) -> &str // would be lowered and printed as fn identity<'a>(value: &'a str) -> &'a str

Because I use the LifetimeRef itself, since this is also a valid elision:

fn identity(value: &'static str) -> &str // it would lower and print fn identity(value: &'static str) -> &'static str

Note that the LifetimeRefId are different in argument and return but the LifetimeRef itself will be same.

The way elision works, is that first you lowered the args. Then you take all lifetimes defined from the args and for<...> in the fn, in other words all HRTB lifetimes in the last HRTB stack entry for fn ptrs or all lifetime params in a fn definition. If there is exactly one, all elided lifetimes in the return type get it. Otherwise, eliding lifetimes in the return type is an error.

Yeah I understand that now and is somewhat similar to what I have done, but as you can see from question above since I re-use the LifetimeRef of the lifetime (be it elided or named or static) that can be used for return type, it would result in lowering and analysis that comes after HIR lowering to think, the lifetime in return type was actually provided.

What is the problem that the code will think that the lifetimes were provided? What does it matter for?

Like for the two test we ignored in ide-diagnostic.

Not sure what the actual behavior is, need to check rustc.

Well I didn't check the rustc code itself, but I printed out the HIR tree for some of the functions discussed here and it seems it's all input lifetimes.

Like for the two test we ignored in ide-diagnostic.

The end-goal is to remove the elision in hir-ty.

Well I didn't check the rustc code itself, but I printed out the HIR tree for some of the functions discussed here and it seems it's all input lifetimes.

But what is an input lifetime? Any lifetime mentioned in the input?

Input lifetimes are lifetimes used in the parameters, output are the ones in the return type

https://doc.rust-lang.org/reference/lifetime-elision.html?highlight=elision#lifetime-elision-in-functions

Each elided lifetime in the parameters becomes a distinct lifetime parameter.

If there is exactly one lifetime used in the parameters (elided or not), that lifetime is assigned to all elided output lifetimes.

In method signatures there is another rule

If the receiver has type &Self or &mut Self, then the lifetime of that reference to Self is assigned to all elided output lifetime parameters.

So this is the function in rustc that retrieves the candidate for return elision: resolve_fn_params

It records distinct lifetime usages separately for each input type. I think I make some refactors based on this. But I think my current implementation (unpushed) is very similar.

Also based on this: https://github.com/rust-lang/rust/blob/main/compiler/rustc_hir/src/def.rs#L932-L964
We can consider LifetimeRef::Param to be elided always?

Veykril · 2026-08-04T10:06:33Z

    pub(in crate::expr_store) fn lower_generic_args_from_fn_path(
        &mut self,
        args: Option<ast::ParenthesizedArgList>,
        ret_type: Option<ast::RetType>,
        impl_trait_lower_fn: ImplTraitLowerFn<'_>,
    ) -> Option<GenericArgs> {
        let params = args?;
        let mut param_types = Vec::new();
        for param in params.type_args() {
            let type_ref = self.lower_type_ref_opt(param.ty(), impl_trait_lower_fn);
            param_types.push(type_ref);
        }
        let args = Box::new([GenericArg::Type(
            self.alloc_type_ref_desugared(TypeRef::Tuple(ThinVec::from_iter(param_types))),
        )]);
        let bindings = if let Some(ret_type) = ret_type {
            let type_ref = self.lower_type_ref_opt(ret_type.ty(), impl_trait_lower_fn);
            Box::new([AssociatedTypeBinding {
                name: Name::new_symbol_root(sym::Output),
                args: None,
                type_ref: Some(type_ref),
                bounds: Box::default(),
            }])
        } else {
            // -> ()
            let type_ref = self.alloc_type_ref_desugared(TypeRef::unit());
            Box::new([AssociatedTypeBinding {
                name: Name::new_symbol_root(sym::Output),
                args: None,
                type_ref: Some(type_ref),
                bounds: Box::default(),
            }])
        };
        Some(GenericArgs {
            args,
            has_self_type: false,
            bindings,
            parenthesized: GenericArgsParentheses::ParenSugar,
        })
    }


Parenthesized Fn syntax needs its own lifetime-elision state, like the FnPtrType branch.

type Callback = dyn Fn(&str) -> &str;

This should lower as dyn for<'a> Fn(&'a str) -> &'a str.

In a function parameter its a bit different as well:

fn run(callback: &dyn Fn(&str) -> &str) -> &dyn Fn(&str) -> &str;

should lower to

fn run<'p>( callback: &'p dyn for<'a> Fn(&'a str) -> &'a str, ) -> &'p dyn for<'b> Fn(&'b str) -> &'b str;

View changes since the review

Oof, I was reminding myself to do it, only to forget it :D

we can skip this in this PR fwiw and do it as a follow up as long as we track this somewhere

ChayimFriedman2

Review round 2.

I do wonder if we should resolve all lifetimes during hir lowering, instead of hir-ty, and tend towards yes. But this is not for this PR.

View changes since this review

ChayimFriedman2 · 2026-08-05T00:45:46Z

    is_lowering_coroutine: bool,

-    for_type_binder: Option<ThinVec<Name>>,
+    lifetime_arena: Option<&'a mut Arena<LifetimeParamData>>,


This should be in LifetimeElisionKind::NewLifetime. This way we avoid the Option and unwrap().

I added this as separate field so that I could do this https://github.com/rust-lang/rust-analyzer/blob/master/crates/hir-def/src/expr_store/lower.rs#L399 and not have two mutable borrows to lifetime arena.
It's now on ExprCollector: https://github.com/dfireBird/rust-analyzer/blob/lifetime_elision/crates/hir-def/src/expr_store/lower.rs#L422

This does not prevent it from being a field on LifetimeElisionKind::NewLifetime, this is the active variant when that function is called.

It's not an active variant, since we call it after the lowering the return type, LifetimeElisionKind::Lifetime for the return type lifetime and we need the named lifetime in return as well for the method to work.

ChayimFriedman2 · 2026-08-05T00:50:35Z

@@ -157,6 +158,7 @@ pub enum LifetimeRef {
    Static,
    Placeholder,


We should remove this. It's only used in one place that should be replaced with Error.

ChayimFriedman2 · 2026-08-05T00:58:58Z

+                    None | Some((_, ForBinderSource::FnPtrType | ForBinderSource::ForBound)) => {
+                        self.for_type_binder.replace((ThinVec::new(), ForBinderSource::FnPtrType))
+                    }
+                    Some((_, ForBinderSource::ForType)) => None,


We should replace this with ForBinderSource::FnPtrType, otherwise nested for<...> fn(for<...> fn()) will be handled incorrectly.

Which one do you mean here?

If the current binder is ForBinderSource::ForType, we should replace it with ForBinderSource::FnPtrType.

ChayimFriedman2 · 2026-08-05T01:09:43Z

+
+#[derive(Debug)]
+pub enum LifetimeElisionKind {
+    NewLifetimeParam {


We should split this and remove ElisionBinderSource. One variant will be NewLifetimeParam { bound_type: LifetimeBoundType }, the other NewHrtbLifetime { for_type_binder: ThinVec<Name> }. parent, return_lt and total_created are not needed, because they're used to determine the elided lifetime for a return type, but it should instead be based on all lifetime generic parameters/hrtb lifetimes, because of the what @Veykril said (non-elided lifetimes also participate). ArgumentElisionContext is not needed either: when determining the elided lifetime for the return type, we can see if we have a &[mut ]self type and if yes its lifetime should always be used. We should also remove ExprCollector::for_type_binder, having it here will avoid unwraps.

We would still need for_type_binder for it's original purpose, which is to get the binder from ForType and is currently only applicable for FnPtr.

I will work on the having a for binder variant now, but can you let me know if return_lt should still be removed (the for binder variant might also have it)?

ChayimFriedman2 · 2026-08-05T01:20:39Z

+    };
+
+    if collector.argument_elision_context.is_some() && !is_trait_assoc_item {
+        let args_in_source = generic_args.last().and_then(|g| g.as_ref());


Suggested change

let args_in_source = generic_args.last().and_then(|g| g.as_ref());

let args_in_source = generic_args.pop();

Then push back. So collect_path_elided_liftetimes() can have an owned value to make use of.

As mush I like to do it, I can't get the resize correct with pop and push 😅

ChayimFriedman2 · 2026-08-05T01:23:53Z

+        let is_trait_assoc_item = matches!(def, Some(ModuleDefId::TraitId(..)))
+            && remaining_idx.is_some_and(|idx| idx > 0);


Unfortunately this is not correct; lifetimes in trait's assoc types can still be elided, but only for the trait, not for the assoc type.

Can you give an example? I have lot of unknowns here.

Veykril · 2026-08-05T06:40:42Z

I do wonder if we should resolve all lifetimes during hir lowering, instead of hir-ty, and tend towards yes. But this is not for this PR.

Yes I think we should

Veykril · 2026-08-05T06:41:36Z

Also I really appreciate you tackling this complex task up @dfireBird! (whether you knew how much work this was gonna end up being or not 😄)

dfireBird · 2026-08-05T13:41:28Z

Also I really appreciate you tackling this complex task up @dfireBird! (whether you knew how much work this was gonna end up being or not 😄)

I did not know it would be this much but this is fun compared to my day-to-day work :)

Also, sorry I've been ignoring @ChayimFriedman2 round 2 Review, will get to it, once I finish the return elision correctly.

fix: should elide lifetime in impl trait and its tests

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 27, 2026

dfireBird force-pushed the lifetime_elision branch from 06a014c to 88e02b3 Compare July 27, 2026 06:04

Veykril reviewed Jul 27, 2026

View reviewed changes

ChayimFriedman2 requested changes Jul 28, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

dfireBird added 5 commits August 4, 2026 08:05

internal: implement lifetime elision

54ecc5b

fix: tests updated for lifetime ellision

711812c

fix: hide elided lifetimes in references and generic param list

9ffa4fb

minor fixes related to review comments

refactor: change elision callbacks to internal state

6526ecd

fix: const eval failing on generic args

519237a

dfireBird force-pushed the lifetime_elision branch from e4832ca to 6526ecd Compare August 4, 2026 02:39

Veykril reviewed Aug 4, 2026

View reviewed changes

ChayimFriedman2 requested changes Aug 5, 2026

View reviewed changes

Veykril changed the title ~~internal: implement lifetime elision~~ feat: implement lifetime elision Aug 5, 2026

dfireBird force-pushed the lifetime_elision branch from fd9f45a to 4c71893 Compare August 6, 2026 17:32

dfireBird force-pushed the lifetime_elision branch from 4c71893 to 3da87fd Compare August 6, 2026 17:45

fix: lower elided lifetime in return types

9217af3

fix: should elide lifetime in impl trait and its tests

dfireBird force-pushed the lifetime_elision branch from 3da87fd to 9217af3 Compare August 6, 2026 18:03

		lc world &WorldSnapshot [type_could_unify+name+local]
		ex world [type_could_unify]

		@@ -157,6 +158,7 @@ pub enum LifetimeRef {
		Static,
		Placeholder,

	let args_in_source = generic_args.last().and_then(\|g\| g.as_ref());
	let args_in_source = generic_args.pop();

		let is_trait_assoc_item = matches!(def, Some(ModuleDefId::TraitId(..)))
		&& remaining_idx.is_some_and(\|idx\| idx > 0);

Uh oh!

Uh oh!

Conversation

dfireBird commented Jul 27, 2026

Uh oh!

dfireBird commented Jul 27, 2026

Uh oh!

ShoyuVanilla commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dfireBird commented Jul 27, 2026

Uh oh!

Veykril left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfireBird Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Veykril Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Veykril commented Jul 27, 2026

Uh oh!

ChayimFriedman2 left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfireBird commented Aug 3, 2026

Uh oh!

ChayimFriedman2 commented Aug 3, 2026

Uh oh!

dfireBird commented Aug 3, 2026

Uh oh!

ChayimFriedman2 commented Aug 3, 2026

Uh oh!

dfireBird commented Aug 3, 2026

Uh oh!

dfireBird commented Aug 3, 2026

Uh oh!

This comment has been minimized.

rustbot commented Aug 4, 2026

Uh oh!

Veykril Aug 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfireBird Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfireBird Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShoyuVanilla commented Jul 27, 2026 •

edited

Loading

Veykril left a comment •

edited by rustbot

Loading

dfireBird Jul 27, 2026 •

edited

Loading

Veykril Jul 27, 2026 •

edited

Loading

ChayimFriedman2 left a comment •

edited by rustbot

Loading

Veykril Aug 4, 2026 •

edited by rustbot

Loading

dfireBird Aug 5, 2026 •

edited

Loading

dfireBird Aug 5, 2026 •

edited

Loading

Veykril Aug 5, 2026 •

edited

Loading

dfireBird Aug 5, 2026 •

edited

Loading

Veykril Aug 4, 2026 •

edited by rustbot

Loading

dfireBird Aug 4, 2026 •

edited

Loading

ChayimFriedman2 left a comment •

edited by rustbot

Loading

dfireBird Aug 6, 2026 •

edited

Loading