Skip to content

Simplify the HIR ty lowering of trait object lifetime bounds#157591

Open
fmease wants to merge 2 commits into
rust-lang:mainfrom
fmease:hirtylo-obj-lt-cleanup
Open

Simplify the HIR ty lowering of trait object lifetime bounds#157591
fmease wants to merge 2 commits into
rust-lang:mainfrom
fmease:hirtylo-obj-lt-cleanup

Conversation

@fmease

@fmease fmease commented Jun 8, 2026

Copy link
Copy Markdown
Member

See individual commit messages for details.

@fmease fmease added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Jun 8, 2026
@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 Jun 8, 2026
@rustbot

rustbot commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

r? @mati865

rustbot has assigned @mati865.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler, types
  • compiler, types expanded to 73 candidates
  • Random selection from 20 candidates

@Kivooeo

Kivooeo commented Jun 8, 2026

Copy link
Copy Markdown
Member

r? me

@rustbot rustbot assigned Kivooeo and unassigned mati865 Jun 8, 2026
Letting `lower_lifetime` check `tcx.named_bound_var` & call `re_infer`
just means we now end up passing `lifetime.ident.span` to `re_infer`
instead of the `span` of the trait object type. However,

1. in the case of `LifetimeKind::ImplicitObjectLifetimeDefault`
   `lifetime.ident.span` is actually equal to said span.
2. in the case of `LifetimeKind::Infer` the span now makes more sense;
   consider `dyn Trait + '_` where the span now just contains the `'_`
   not the entire type which is just better.
@fmease fmease force-pushed the hirtylo-obj-lt-cleanup branch from c89555f to 6b234eb Compare June 8, 2026 07:53
…nfer`

This makes it really obvious that we're computing object region bounds
for `Infer`, too, not just for `ImplicitObjectLifetimeDefault` which was
really hard to see beforehand. It's unclear to me whether this was
intentional or not, needs investigation.

Introduce method `lower_trait_object_lifetime` to allow usage of early
returns to make the control flow clearer compared to the `unwrap_or_else`.
Comment on lines +456 to +457
// Curiously, we also use the *object region bound* for `Infer` (`'_`)
// while we obviously don't use the *object lifetime default* for it...

@fmease fmease Jun 8, 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.

For a tiny bit more background information, see the large paragraph in the PR description of rust-lang/reference#2281 (the one about compute_object_lifetime_bound vs. RBV's named_bound_var).

View changes since the 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.

Thanks for pointing this out, I'll take a closer look at this reference tomorrow

@Kivooeo

Kivooeo commented Jun 9, 2026

Copy link
Copy Markdown
Member

@bors r+ rollup

@rust-bors

rust-bors Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 6b234eb has been approved by Kivooeo

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-cleanup Category: PRs that clean code up or issues documenting cleanup. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants