Skip to content

Make trait refs & assoc ty paths properly induce trait object lifetime defaults#129543

Open
fmease wants to merge 8 commits intorust-lang:mainfrom
fmease:obj-lt-def-gat
Open

Make trait refs & assoc ty paths properly induce trait object lifetime defaults#129543
fmease wants to merge 8 commits intorust-lang:mainfrom
fmease:obj-lt-def-gat

Conversation

@fmease
Copy link
Copy Markdown
Member

@fmease fmease commented Aug 25, 2024

View all comments

Trait Object Lifetime Defaults

Primer & Definitions

You could read this section in the Reference but it has several issues (see rust-lang/reference#1407). Here's a small explainer by me that only mentions the parts relevant to this PR:

Basically, given dyn Trait (≠ dyn Trait + '_) we want to deduce its trait object lifetime bound from context without relying on normal region inference as we might not be in a body1. The "context" means the closest – what I call — (eligible) container C<X0, …, Xn> that wraps this trait object type. A container is to be understood as a use site of a "parametrized definition" (more general than type constructors). Currently eligible are ADTs, type aliases, traits and enum variants.

So if we have C<dyn Trait> (e.g., &'r dyn Trait or Struct<'r, dyn Trait>), D<C<dyn Trait>> or C<N<dyn Trait>> (e.g., Struct<'r, (dyn Trait,)>), we use the explicit2 outlives-bounds on the corresponding type parameter of C to determine the trait object lifetime bound. Here, C & D denote (eligible) containers and N denotes a generic type that is not an eligible container. E.g., given struct Struct<'a, T: 'a + ?Sized>(…);, we elaborate Struct<'r, dyn Trait> to Struct<'r, dyn Trait + 'r>.

Finally, we call lifetime bounds used as the default for constituent trait object types of an eligible container C the trait object lifetime defaults (induced by C) which may be shadowed by inner containers.

Changes Made

These changes are theoretically breaking.

  1. Make resolved associated type paths / projections eligible containers.
    • <Y0 as TraitRef<X0, …, Xn>>::AssocTy<Y1, …, Ym> now induces trait object lifetime defaults for constituents Y0 to Ym (TraitRef is considered a separate container, see also list item (3)).
    • Notably, for the self type Y0 of (resolved) projections we now look at the bounds on the Self type param of the relevant trait (e.g., given trait Outer<'a>: 'a { type Proj; } or trait Outer<'a> where Self: 'a { type Proj; } we elaborate <dyn Inner as Outer<'r>>::Proj to <dyn Inner + 'r as Outer<'r>>::Proj).
    • Example breakages:
      trait Outer<'a> { type Ty<T: ?Sized + 'a>; }
      impl<'a> Outer<'a> for () { type Ty<T: ?Sized + 'a> = &'a T; }
      trait Inner {}
      
      fn f<'r>(x: <() as Outer<'r>>::Ty<dyn Inner>) {
      //                                ~~~~~~~~~
      //                                this branch:  dyn Inner + 'r       (due to bound `'a` on `T`)
      //                                stable/main:  dyn Inner + 'static  (due to item signature fallback)
          let _: <() as Outer<'r>>::Ty<dyn Inner + 'static> = x;
      //         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      //         this branch:  error: lifetime may not live long enough // `'r`  must outlive `'static`
      //         stable/main:  OK
      }
      trait Outer { type Ty; }
      trait Inner {}
      
      impl<'a> Outer for dyn Inner + 'a { type Ty = &'a (); }
      
      fn f<'r>(x: *mut &'r <dyn Inner as Outer>::Ty) {
      //                    ~~~~~~~~~
      //                    this branch:  dyn Inner + 'static  (due to lack of bounds on `Outer`)
      //                    stable/main:  dyn Inner + 'r       (since the assoc type path now shadows the default induced by the ref typ ctor `&`)
          let _: *mut &'r <dyn Inner + 'r as Outer>::Ty = x;
      //         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      //         this branch:  error: lifetime may not live long enough // `'r` must outlive `'static`
      //         stable/main:  OK
      }
  2. In type-relative paths Y0::Name<Y1, …, Ym> consider the trait object lifetime default indeterminate
    • Meaning if we're in an "item context" / "item signature" / "non-body" (& the principal trait isn't bounded by any outlives-bounds which would take precedence over the default) we will reject any implicit trait object lifetime bounds that would take on that default
    • Reason: Limitations of the current implementation which can't be easily overcome
      • RBV (which resolves trait object lifetime defaults by recursing into the local crate "in one sitting") would require the resolution of type-relative paths in order to look up the generics but these paths are only resolved in HIR ty lowering (that can selectively lower local items) which depends on the results of RBV (cyclic dependency!)
      • While one might be able to resolve type-relative paths in RBV in an ad-hoc fashion, it would require a lot of duplication with HIR ty lowering and its impl would be very brittle (RTN does something like that in RBV but we require a more sophisticated resolver)
      • I did attempt that but it got too gnarly and brittle and would've likely been incomplete anyway
      • See also this GH thread
      • See also #t-types/meetings > 2025-09-16 weekly @ 💬
    • This should still be maximally forward compatible and allow us to implement the desired behavior in the future.
    • Example breakage:
      trait Outer { type Ty<'a, T: 'a + ?Sized>; }
      trait Inner {}
      
      fn f<'r, T: Outer>(x: T::Ty<'r, dyn Inner>) {}
      //                              ~~~~~~~~~
      //                              this branch:  error: indeterminate  (reservation)
      //                              stable/main:  dyn Inner + 'static   (due to item signature fallback)
  3. Fixes trait object lifetime defaults inside trait refs TraitRef<X0, …, Xn> (this fell out from the previous changes). They used to be completely broken due to a nasty off-by-one error for not accounting for the implicit Self type param of traits which lead to cases like
    • Outer<'r, dyn Inner> (with trait Outer<'a, T: 'a + ?Sized> {}) getting rejected as indeterminate (it tries to access a lifetime at index 1 instead 0) (playground)
    • Outer<'r, 's, dyn Inner> (with trait Outer<'a, 'b, T: 'a + ?Sized> {}) elaborating dyn Inner to dyn Inner + 's instead of dyn Inner + 'r(!) (playground)
    • The same applies to trait alias refs (feature trait_alias)
    • Example breakage:
      trait Outer<'a, 'b, T: 'a + ?Sized> {}
      trait Inner {}
      
      struct F<'r, T>
      where
          T: Outer<'r, 'static, dyn Inner>
      //                        ~~~~~~~~~
      //                        this branch:  dyn Inner + 'r       (correctly mapping `'a` => `'r`)
      //                        stable/main:  dyn Inner + 'static  (incorrectly mapping `'a` => `'static` due to off-by-one)
      {
          g: G<'r, T>,
      //     ~~~~~~~~
      //     this branch:  error: mismatched types
      //                          expected: `Outer<'r, 'static, (dyn Inner + 'static)>`
      //                             found: `Outer<'r, 'static, (dyn Inner + 'r)>`
      //     stable/main:  OK
      }
      
      struct G<'r, T>(&'r (), T)
      where
          T: Outer<'r, 'static, dyn Inner + 'static>;
  4. In associated type binding TraitRef<AssocTy<X0, …, Xn> = Y> consider the trait object lifetime default indeterminate (in X0, …, Xn and Y) if X0, …, Xn contains any lifetime arguments.
    • Meaning if we're in an item context (& the principal trait isn't bounded) we will reject any implicit trait object lifetime bounds that would take on that default
    • This reserves us the right to (1) take into account the item bounds of AssocTy in the future when computing the default for Y (2) take into account the parameter bounds of AssocTy in the future when computing the defaults for X0, …, Xn.
    • This extends a preexisting hack that – given TraitRef<X0, …, Xn, AssocTy<Y0, …, Ym> = Z> – treats the default indeterminate in Y0, …, Ym and Z if X0, …, Xn contains any lifetime arguments.
    • Rephrased, this hack / reservation previously didn't account for GAT args, only trait ref args, which is insufficient
    • See also this GH comment of mine
    • Example breakages:
      trait Outer { type Ty<'a>: ?Sized/* + 'a*/; }
      trait Inner {}
      
      fn f<'r>(_: impl Outer<Ty<'r> = dyn Inner>) {}
      //                              ~~~~~~~~~
      //                              this branch:  error: indeterminate  (reservation)
      //                              stable/main:  dyn Inner + 'static   (forced)
      trait Outer { type Ty<'a, T: ?Sized + 'a>; }
      trait Inner {}
      
      fn f<'r>(_: impl Outer<Ty<'r, dyn Inner> = ()>) {}
      //                            ~~~~~~~~~
      //                            this branch:  error: indeterminate  (reservation)
      //                            stable/main:  dyn Inner + 'static   (forced)

Motivation

Both trait object lifetime default RFCs (599 and 1156) never explicitly specify what constitutes a — what I call — (eligible) container but it only makes sense to include anything that can be parametrized by generics and can be mentioned in places where we don't perform full region inference … like associated types. So it's only consistent to make this change.

Breakages

These changes are theoretically breaking because they can lead to different trait object lifetime bounds getting deduced compared to main which is obviously user observable. Moreover, we're now explicitly rejecting implicit trait object lifetime bounds inside type-relative paths (excl. the self type) and on the RHS of assoc type bindings if the assoc type has lifetime params.
However, the latest crater run found 0 non-spurious regressions (see here and here).


Fixes #115379.
Fixes #140710.
Fixes #141997.

Footnotes

  1. If we are in a body we do use to normal region inference as a fallback.

  2. Indeed, we don't consider implied bounds (inferred outlives-bounds).

@fmease fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. T-types Relevant to the types team, which will review and decide on the PR/issue. labels Aug 25, 2024
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 25, 2024
@fmease
Copy link
Copy Markdown
Member Author

fmease commented Aug 25, 2024

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 25, 2024
[crater] Properly deduce the object lifetime default in GAT paths

Fixes rust-lang#115379.

r? ghost
@bors
Copy link
Copy Markdown
Collaborator

bors commented Aug 25, 2024

⌛ Trying commit 8bfcd86 with merge 24cd45d...

@fmease fmease removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 25, 2024
@bors
Copy link
Copy Markdown
Collaborator

bors commented Aug 25, 2024

☀️ Try build successful - checks-actions
Build commit: 24cd45d (24cd45d7714ba43afd4b8e62fb677b069b21c4a5)

@fmease
Copy link
Copy Markdown
Member Author

fmease commented Aug 25, 2024

@craterbot check

@craterbot
Copy link
Copy Markdown
Collaborator

👌 Experiment pr-129543 created and queued.
🤖 Automatically detected try build 24cd45d
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 25, 2024
@fmease fmease changed the title [crater] Properly deduce the object lifetime default in GAT paths [crater] Properly deduce object lifetime defaults in GAT paths Aug 25, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 27, 2024
[CRATER] Crater Rollup

This is a " crater rollup" of:
* rust-lang#126452
* rust-lang#128784
* rust-lang#129392
* rust-lang#129422
* rust-lang#129543

**What is a crater rollup?** It's simply a crater job that is run on all of the containing PRs *together*, and then we can set the crates list for each of these jobs to just the failures after it's done. It should cut out on the bulk of "normal" crates that do nothing and simply just take time to build.

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 28, 2024
[CRATER] Crater Rollup

This is a " crater rollup" of:
* rust-lang#126452
* rust-lang#128784
* rust-lang#129392
* rust-lang#129422
* rust-lang#129543
* rust-lang#129604

**What is a crater rollup?** It's simply a crater job that is run on all of the containing PRs *together*, and then we can set the crates list for each of these jobs to just the failures after it's done. It should cut out on the bulk of "normal" crates that do nothing and simply just take time to build.

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 28, 2024
[CRATER] Crater Rollup

This is a " crater rollup" of:
* rust-lang#126452
* rust-lang#128784
* rust-lang#129392
* rust-lang#129422
* rust-lang#129543
* rust-lang#129604

**What is a crater rollup?** It's simply a crater job that is run on all of the containing PRs *together*, and then we can set the crates list for each of these jobs to just the failures after it's done. It should cut out on the bulk of "normal" crates that do nothing and simply just take time to build.

r? `@ghost`
@compiler-errors
Copy link
Copy Markdown
Contributor

@craterbot
Copy link
Copy Markdown
Collaborator

📝 Configuration of the pr-129543 experiment changed.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@traviscross
Copy link
Copy Markdown
Contributor

@craterbot
Copy link
Copy Markdown
Collaborator

📝 Configuration of the pr-129543 experiment changed.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Copy Markdown
Collaborator

🚧 Experiment pr-129543 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Copy Markdown
Collaborator

🎉 Experiment pr-129543 is completed!
📊 6 regressed and 0 fixed (98236 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Sep 1, 2024
@fmease fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 2, 2024
@workingjubilee workingjubilee added the C-crater Category: Issue / PR for tracking crater runs label Feb 14, 2025
@fmease fmease removed the C-crater Category: Issue / PR for tracking crater runs label Apr 17, 2025
@fmease
Copy link
Copy Markdown
Member Author

fmease commented Feb 4, 2026

Alright, this is now finally ready for a rereview wrt. both implementation and pFCP'ed semantic changes! Changes since last comment of mine:

  1. I've extended a preexisting 'hack' / reservation concerning assoc ty bindings to properly account for GATs, too. This means more theoretical breakages. Please see bullet point (4) in the PR description for technical details. Crater came back clean. The impl is found in the last commit (ae8c0a5)
  2. I've added a lot more tests and improved / fleshed out the preexisting ones
  3. I've heavily improved the source code comments I had added
  4. I've rewritten several parts of the PR description to be more informative & legible
  5. I've added 5 example breakages to the PR description (in a collapsed section)

@lcnr, @BoxyUwU, I'd be super happy if you both could take a look esp. in regards to the (latest version of the) pFCP'ed semantic changes since I'd love it if this can finally go into FCP :D Thanks a lot in advance :)

@rust-bors

This comment has been minimized.

@rustbot

This comment has been minimized.

Copy link
Copy Markdown
Member Author

@fmease fmease Feb 10, 2026

Choose a reason for hiding this comment

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

This test didn't actually test what it meant to test. I've therefore replaced it with a new one (tests/ui/object-lifetime/object-lifetime-default-inferred-args.rs). It was added in the initial generic_arg_infer PR.

It's meant to test that the presence of _ in a generic arg list doesn't throw off the trait object lifetime default calculations which is a very valuable thing to test (hence me adding a replacement). Sadly, it doesn't test that.

First of all, the only places in this file that contain implicit trait object lifetime bounds that are in the mere vicinity of an inferred arg _ are the two &dyn Tests at the bottom. However, these just get elab'ed to &'?0 (dyn Test + '?1) and &'2 (dyn Test + '?3) since the implicit inferred lifetimes passed to & aren't named, so the default is indeterminate meaning fresh region vars for both since we're in a body.

The two _ passed as part of the fn calls have no influence whatsoever on the defaults chosen for the dyn Test "in random as-casts"; that's not how trait object lifetime defaulting works.

@rustbot

This comment has been minimized.

@rust-bors

This comment has been minimized.

@rustbot

This comment has been minimized.

Copy link
Copy Markdown
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

When do we actually use trait object defaults in bodies? isn't it better to just always infer them unless we're in an ItemCtxt?

View changes since this review

//
// FIXME(mgca, #151649): Assoc (and free?) consts should also qualify.
// FIXME(return_type_notation, #151662): Assoc fns should also qualify.
let depth = path.segments.len() - index - 1;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you add a comment explaining what "depth" is, afaict what we care about for AssocTy is if we have T::Assoc, are we in T and for variant we have T::Variant we check whether we're in Variant? That feels odd to me.

I guess assoc types want the info from their trait and everything else wants it from itself, but that doesn't explain variant, with enums you can have generic args both on the enum and on the variant, i.e. both Enum::<args>::Variant and Enum::Variant::<args> are valid

Copy link
Copy Markdown
Member Author

@fmease fmease Apr 4, 2026

Choose a reason for hiding this comment

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

can you add a comment explaining what "depth" is

Will do.

I guess assoc types want the info from their trait and everything else wants it from itself

I'd like to clarify: On main, we already "accept" AssocTy but only at depth 1 (thereby setting the container to the corresp. trait). On my branch we now also "accept" AssocTy at depth 0 (thereby setting the container to itself).

AssocTy@depth=1 is necessary for resolving obj lt defs in the trait ref. <A as TraitRef<B>>::AssocTy<C> essentially gets represented as QPath(Some(`A`), [`TraitRef<B>`, `AssocTy<C>`]) in the HIR. Given such a qpath, we're reaching both AssocTy@1 and AssocTy@0 (it doesn't reach Trait@0 since the overall resolution of the path is AssocTy).

but that doesn't explain variant, with enums you can have generic args both on the enum and on the variant

That is indeed odd and should be rectified soon (not in this PR tho imo). However, IINM this issue cannot be observed at the moment thanks to a different bug (#108224). AFAICT, in this context DefKind::Variant can only be reached by "braced enum constructions" …V { … } as unit & tuple constructions will have DefKind::Ctor(Variant, Const | Fn). Now, due to aforesaid bug (concerning lifetime elision, I think) (#108224), E::<'a, …>::V { … } gets rejected (it gets expanded to E::<'a, …>::V::<'_, …> { … } which later gets rejected by HIR ty lowering for obvious reasons), so we can "assume" E::V::<'a, …> {} here (i.e., only Variant@0 bears args, never Variant@1) (until that bug is fixed).

(Note that even if we stopped computing obj lt defs in bodies (as suggested by you and with which I agree), this issue is still relevant for (m)GCA under which Variant | Ctor(Variant, Const | Fn) (ref'ing lifetime args & trait object types) can legally appear in item signatures / contexts without inference (edit: conditions apply)).

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.

Added comments describing depth reversed segment indices. Added a FIXME for the preexisting variant bug.

Edit: I've opened PR that fixes both issues: #154918. Can be merged before or after this PR, it doesn't matter.

) if depth == 0 => Some((def_id, path.segments)),
// Note: We don't need to care about definition kinds that may have generics if they
// can only ever appear in positions where we can perform type inference (i.e., bodies).
_ => None,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how annoying is it to make this exhaustive?

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.

Done. See new final commit, new function eligible_container.

generics: &ty::Generics,
tcx: TyCtxt<'_>,
def_id: DefId,
) -> (usize, usize) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

comment please 🤔 depth is "index of segment in reverse order" and "index" is "index of args of segment"?

Copy link
Copy Markdown
Member Author

@fmease fmease Apr 4, 2026

Choose a reason for hiding this comment

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

Yea I'll prolly rename it to rev_index, rev_seg_index or similar (and index to arg_index or similar). I might even newtype it cuz a very early version of this PR actually had them mixed up at some point ^^).

Edit: That's exactly what I ended up doing (see the final commit).

//@ check-pass

trait Trait<T: ?Sized> { type Assoc<'a> where T: 'a; }
impl<T: ?Sized> Trait<T> for () { type Assoc<'a> = &'a T where T: 'a; }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this impl needed?

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.

It's needed for the test case at the bottom:

// We deduce `dyn Bound + 'static` (and not `dyn Bound + 'r`).
fn f2<'r>(x: <() as Trait<dyn Bound>>::Assoc<'r>) { /*check*/ g2(x) }
fn g2<'r>(_: <() as Trait<dyn Bound + 'static>>::Assoc<'r>) {}

However, I could rewrite this test the following way if you wanted me to:

@@ -6,7 +6,6 @@
 //@ check-pass

 trait Trait<T: ?Sized> { type Assoc<'a> where T: 'a; }
-impl<T: ?Sized> Trait<T> for () { type Assoc<'a> = &'a T where T: 'a; }

 trait Bound {}

@@ -21,7 +20,9 @@
 fn g1(_: impl Trait<dyn Bound + 'static>) {}

 // We deduce `dyn Bound + 'static` (and not `dyn Bound + 'r`).
-fn f2<'r>(x: <() as Trait<dyn Bound>>::Assoc<'r>) { /*check*/ g2(x) }
-fn g2<'r>(_: <() as Trait<dyn Bound + 'static>>::Assoc<'r>) {}
+fn f2<'r, T>(_: <T as Trait<dyn Bound>>::Assoc<'r>)
+where
+    /*check*/ T: Trait<dyn Bound + 'static>
+{}

 fn main() {}

Alternatively, I could rewrite all test cases in this file using the "AbideBy pattern" (on the one hand I love it because it makes all test case super concise, on the other hand I hate it because it renders the tests even more artificial IMO (now most of my tests use it ^^'))

// We deduce `dyn Inner + 'r` from bound `'a` on ty param `T` of trait `Outer`.
fn assoc_ty_proj<'r>(_: <() as Outer<'r, dyn Inner>>::Ty) {}

impl<'a, T: 'a + AbideBy<'a> + ?Sized> Outer<'a, T> for () { type Ty = &'a T; }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

move this to trait definition?


impl<'a, T: 'a + AbideBy<'a> + ?Sized> Outer<'a, T> for () { type Ty = &'a T; }

trait Inner {}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here, move this up

self.tcx.parent(def_id),
&path.segments[..path.segments.len() - 1],
)),
_ => None,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what are the cases where this change goes from indeterminate to pass-through due to this change?

something something exhaustive match possible?

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.

what are the cases where this change goes from indeterminate to pass-through due to this change?

I'm not sure I fully understand the question, pass-through whereto? I don't think there are any such cases.

  1. If the "indeterminacy scope" is inside of the self type (pseudo: <INDET<dyn Inner> as Outer<'r>>::INELIGIBLE1), then the inner indeterminacy prevails / shadows any potential outer obj lt defs
    • meaning: indeterminate → indeterminate
  2. If the "indeterminacy scope" is outside of the resolved (ineligible) projection (pseudo: INDET<<dyn Inner as Outer<'r>>::INELIGIBLE>1) then we'll "pass through to" the "indeterminacy" … but that's already what happens on stable&main where resolved projections are always pass through wrt. object lifetime defaulting
    • meaning: indeterminate → indeterminate

Of course, "pass-through to indeterminate" is possible and so is "indeterminate to check-pass" (if the container is eligible) but that's probably not what you meant.

Footnotes

  1. INDET magically renders the object lifetime default indeterminate and INELIGIBLE is not an eligible container. 2

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.

something something exhaustive match possible?

The final commit introduces function eligible_container that's called here in visit_qpath and there in visit_path_segment_args. Said function exhaustively matches on the Def as per #129543 (comment).


fn is_static<T>(_: T) where T: 'static {}

// FIXME: Ideally, we'd elaborate `dyn Bar` to `dyn Bar + 'static` instead of rejecting it.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we? 😅 I am not even sure whether this is desirable or not.

Copy link
Copy Markdown
Member Author

@fmease fmease Apr 8, 2026

Choose a reason for hiding this comment

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

Could you clarify? Which semantics would you desire?

AFAICT, there are 5 options for how we could make associated type bindings behave: The associated type binding could...

  1. ...artificially induce "indeterminacy" unconditionally
    • i.e., reject implicit object lifetimes in item signatures
  2. ...artificially induce 'static unconditionally
  3. ...induce a lifetime default that's determined by the item bounds
    • here that would be 'static since OLD(Foo::Item) would be Empty which turns into 'static in item signatures
    • unlikely to be implemented anytime soon since RBV & HIR ty lowering would need to be consolidated or be able to interlock smh.
    • not sure if it's "desirable" but it's consistent / not surprising:
      • type arguments induce a default determined by the parameter bounds
      • assoc type bindings would induce a default determined by the item bounds
  4. ...not induce any object lifetime default
    • aka it's pass-through
    • here that would mean elab'ing dyn Bar to dyn Bar + 'a since it'd take on the default induced by the outer &
    • that'd be surprising I think à la "why are type arguments 'solid' (not pass through) but assoc type bindings are not despite being in the same <…> list?"
    • (although I have to admit that we do already have containers on stable that "look" eligible but actually aren't (apart from the things that are fixed in this PR, of course), most notably tuple type constructors which are pass-through contrary to ADTs (Solo<·> != (·,) wrt. obj lt defaulting); still, using that as a counterargument would probably be a stretch)
  5. ...artificially induce *indeterminacy" if any generic arg list contains lifetime args, otherwise artificially induce 'static
    • that's the behavior on main, stable & this branch
    • it's a HACK for reservation purposes (namely transitioning to option (3))

@lcnr
Copy link
Copy Markdown
Contributor

lcnr commented Mar 2, 2026

In the list of examples, can you match them to each of the changes from this PR. i.e have

  • Make resolved associated type paths / projections eligible containers.
// this now chooses `'a` and compiles

// this previously chose `'b` and now chooses `'c`, causing an error

...

after this I am gonna start the FCP

this is a really impressive PR :> sorry for taking so long to get to it.

@rust-bors

This comment was marked as outdated.

@QuineDot
Copy link
Copy Markdown

@lcnr

When do we actually use trait object defaults in bodies? isn't it better to just always infer them unless we're in an ItemCtxt?

If there are no lifetime bound on the trait, and if you annotate a lifetime that corresponds to the default (e.g. a reference lifetime) and completely elide the dyn lifetime (i.e. no + '_), the default lifetime takes precedence.

trait Trait {}
impl Trait for () {}

fn example<'a>(arg: &'a ()) {
    let dt: &'a dyn Trait = arg;
    // fails
    let _: &(dyn Trait + 'static) = dt;
}

But if you use &'_ dyn Trait or &dyn Trait, the dyn lifetime is inferred. (That wasn't the case before this PR.)

If there is a single lifetime bound on the trait, the default bound is used in function bodies, and even overrides + '_.

trait Single<'a>: 'a {}
impl Single<'_> for () {}

fn non_inferred_examples<'a>() {
    let bx: Box<dyn Single<'a>> = Box::new(());
    // Fails
    let _: Box<dyn Single<'a> + 'static> = bx;

    let bx: Box<dyn Single<'a> + '_> = Box::new(());
    // Fails
    let _: Box<dyn Single<'a> + 'static> = bx;
}

If there are multiple lifetime bounds on the trait, elision and '_ are considered ambiguous.

trait Double<'a, 'b>: 'a + 'b {}
impl Double<'_, '_> for () {}

fn ambiguous<'a, 'b>() {
    // Both fail
    let _: Box<dyn Double<'a, 'b>> = Box::new(());
    let _: Box<dyn Double<'a, 'b> + '_> = Box::new(());
}

Personally, I agree that just inferring would be an improvement for all these cases. See also #91302.

@lcnr
Copy link
Copy Markdown
Contributor

lcnr commented Apr 2, 2026

Oh yeah, all these examples seem very undesirable to me 🤣 unsure if that should happen in this PR

//
// FIXME(mgca, #151649): Assoc (and free?) consts should also qualify.
// FIXME(return_type_notation, #151662): Assoc fns should also qualify.
let depth = path.segments.len() - index - 1;
Copy link
Copy Markdown
Member Author

@fmease fmease Apr 4, 2026

Choose a reason for hiding this comment

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

can you add a comment explaining what "depth" is

Will do.

I guess assoc types want the info from their trait and everything else wants it from itself

I'd like to clarify: On main, we already "accept" AssocTy but only at depth 1 (thereby setting the container to the corresp. trait). On my branch we now also "accept" AssocTy at depth 0 (thereby setting the container to itself).

AssocTy@depth=1 is necessary for resolving obj lt defs in the trait ref. <A as TraitRef<B>>::AssocTy<C> essentially gets represented as QPath(Some(`A`), [`TraitRef<B>`, `AssocTy<C>`]) in the HIR. Given such a qpath, we're reaching both AssocTy@1 and AssocTy@0 (it doesn't reach Trait@0 since the overall resolution of the path is AssocTy).

but that doesn't explain variant, with enums you can have generic args both on the enum and on the variant

That is indeed odd and should be rectified soon (not in this PR tho imo). However, IINM this issue cannot be observed at the moment thanks to a different bug (#108224). AFAICT, in this context DefKind::Variant can only be reached by "braced enum constructions" …V { … } as unit & tuple constructions will have DefKind::Ctor(Variant, Const | Fn). Now, due to aforesaid bug (concerning lifetime elision, I think) (#108224), E::<'a, …>::V { … } gets rejected (it gets expanded to E::<'a, …>::V::<'_, …> { … } which later gets rejected by HIR ty lowering for obvious reasons), so we can "assume" E::V::<'a, …> {} here (i.e., only Variant@0 bears args, never Variant@1) (until that bug is fixed).

(Note that even if we stopped computing obj lt defs in bodies (as suggested by you and with which I agree), this issue is still relevant for (m)GCA under which Variant | Ctor(Variant, Const | Fn) (ref'ing lifetime args & trait object types) can legally appear in item signatures / contexts without inference (edit: conditions apply)).

generics: &ty::Generics,
tcx: TyCtxt<'_>,
def_id: DefId,
) -> (usize, usize) {
Copy link
Copy Markdown
Member Author

@fmease fmease Apr 4, 2026

Choose a reason for hiding this comment

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

Yea I'll prolly rename it to rev_index, rev_seg_index or similar (and index to arg_index or similar). I might even newtype it cuz a very early version of this PR actually had them mixed up at some point ^^).

Edit: That's exactly what I ended up doing (see the final commit).

@fmease
Copy link
Copy Markdown
Member Author

fmease commented Apr 4, 2026

When do we actually use trait object defaults in bodies? isn't it better to just always infer them unless we're in an ItemCtxt?

Oh yeah, all these examples seem very undesirable to me 🤣 unsure if that should happen in this PR

They are indeed used in bodies as well (which also surprised me quite a bit when I first noticed). As alluded to by QuineDot, changing that would make us accept more code. I agree with just performing inference in bodies.

However, I'd strongly prefer it if we didn't implement this in this PR ^^' whose scope I no longer wish to increase. I hope that's understandable.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 7, 2026

Some changes occurred in need_type_info.rs

cc @lcnr

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 7, 2026

This PR was rebased onto a different main 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.

fmease added 8 commits April 7, 2026 19:29
…ir own args (excl. self ty)

This automatically fixes trait object lifetime defaulting for trait refs
and trait alias refs, too.
These used to be broken because the index calculation for going from
middle generic args back to HIR ones didn't take into account the
implicit self ty param present on traits.

Moreover, in item signatures reject hidden trait object lifetime bounds
inside type-relative paths (excl. the self ty) on grounds of the default
being indeterminate.
* print `Empty` as it's called in code, otherwise it's unnecessarily confusing
* go through the middle::ty Generics instead of the HIR ones, so we can dump the
  default for the implicit `Self` type parameter of traits, too
…ntiated assoc ty bindings

Namely, on the RHS and in the args of the bindings.
@fmease
Copy link
Copy Markdown
Member Author

fmease commented Apr 8, 2026

In the list of examples, can you match them to each of the changes from this PR

Done.


I've addressed all of your review comments. To 3 of these comments I have replied with a counterquestion, namely: (1) #129543 (comment), (2) #129543 (comment), (3) #129543 (comment).


The final commit (Introduce fn eligible_container & rename depth to rev seg index) is new, the other commits are basically unchanged.

@rustbot review

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

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. perf-regression Performance regression. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. 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