Pass the whole `GenericArgs` to `Interner::for_each_relevant_impl()` by ChayimFriedman2 · Pull Request #156295 · rust-lang/rust

ChayimFriedman2 · 2026-05-07T21:55:18Z

Pass the whole GenericArgs to Interner::for_each_relevant_impl()

And not just the self type.

rustc does not make use of this, but rust-analyzer needs it to support impls in the same block as args, see https://rust-lang.zulipchat.com/#narrow/channel/144729-t-types/topic/non.20local.20impls.20for.20generic.20args/with/593629693.

I'm not entirely sure this covers all cases (e.g. an unnormalized alias), and want feedback from a types team member.

r? types

fmease · 2026-05-08T08:52:18Z

@bors try @rust-timer queue

Pass the whole `GenericArgs` to `Interner::for_each_relevant_impl()`

rust-bors · 2026-05-08T11:01:21Z

☀️ Try build successful (CI)
Build commit: f3ca1e1 (f3ca1e1620d490fd13130ed418f39f03c99ffd2d, parent: 3e353d7353b0cd6fa4eefd5c882457cc2708184c)

rust-timer · 2026-05-08T11:42:00Z

Finished benchmarking commit (f3ca1e1): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.0%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary 3.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.9%	[3.9%, 3.9%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.1%	[4.1%, 4.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.1%	[-4.1%, -4.1%]	1
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.0%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.0%	[0.0%, 0.0%]	3
Regressions ❌ (secondary)	0.1%	[0.0%, 0.1%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	3

Bootstrap: 497.449s -> 499.373s (0.39%)
Artifact size: 397.06 MiB -> 395.13 MiB (-0.49%)

oli-obk · 2026-05-11T07:51:48Z

How does this help rust-analyzer in finding impls inside fn bodies?

ChayimFriedman2 · 2026-05-11T09:42:06Z

Basically, instead of considering the block of the trait and Self only, it considers the block for any generic arg in the impl search.

fmease · 2026-05-11T12:47:09Z

At the risk of stating the obvious, in rustc, IINM for_each_relevant_impl is only used as a performance optimization to avoid running type relating / unification against all impls ever. It returns an overapproximation of relevant impls to be properly filtered later on.

I assume that you also only use for_each_relevant_impl as a jumping-off point in r-a and noticed that your type-relating is slowing you down tremendously when passing all impls, even ones nested inside bodies, to for_each_relevant_impl and so you want trim down the initial results?

ChayimFriedman2 · 2026-05-11T17:26:53Z

rust-analyzer has different treatment for impls in bodies, for both performance and incrementality reasons we can't access all impls literally. We special case const _ impls, and need for_each_relevant_impl() to determine impls from which bodies to add. This was this way even before we used the next solver, FWIW, and is why rustc started warning on non local impls.

lcnr · 2026-05-11T18:00:54Z

        trait_def_id: DefId,
-        self_ty: Ty<'tcx>,
+        args: ty::GenericArgsRef<'tcx>,


pass in the TraitRef instead

View changes since the review

The reason I didn't is that for some predicates (normalization) retrieving the trait predicate requires knowing what args belong to the parent trait which require calling the interner, which might have a performance effect.

oh, that's subtle. So the args here can also contain the GAT arguments. I think that's slightly confusing and I am fairly confident that computing the TraitRef is cheap enough to not have a perf impact, would you mind trying that. with this and green perf, r=me

lcnr

nit, with that one, lgtm

View changes since this review

lcnr · 2026-05-11T18:01:57Z

I'm not entirely sure this covers all cases (e.g. an unnormalized alias), and want feedback from a types team member.

currently switching the new solver back to eager norm, at which point all args of the trait_ref will be fully normalized

And not just the self type. rustc does not make use of this, but rust-analyzer needs it to support impls in the same block as args, see https://rust-lang.zulipchat.com/#narrow/channel/144729-t-types/topic/non.20local.20impls.20for.20generic.20args/with/593629693. I'm not entirely sure this covers all cases (e.g. an unnormalized alias), and wants feedback from a types team member.

rustbot · 2026-06-29T20:43:26Z

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.

ChayimFriedman2 · 2026-06-29T20:44:28Z

@bors try @rust-timer queue

Pass the whole `GenericArgs` to `Interner::for_each_relevant_impl()`

rust-bors · 2026-06-29T22:51:52Z

☀️ Try build successful (CI)
Build commit: 6ee476e (6ee476e2e02bacb7bbdeb184720933e1fe59c80f)
Base parent: 7dc2c16 (7dc2c162b9c197aaa76a6f9e7534569537830a01)

rust-timer · 2026-06-29T23:32:00Z

Finished benchmarking commit (6ee476e): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (secondary 6.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	6.2%	[6.2%, 6.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 511.037s -> 490.637s (-3.99%)
Artifact size: 393.19 MiB -> 393.18 MiB (-0.00%)

ChayimFriedman2 · 2026-06-29T23:37:57Z

@bors r=lcnr

rust-bors · 2026-06-29T23:38:00Z

📌 Commit abbfd2c has been approved by lcnr

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 1. This pull request will be tested once the tree is reopened.

…uwer Rollup of 11 pull requests Successful merges: - #155722 (Introduce aarch64-unknown-linux-pauthtest target) - #156230 (tests: check wasm compiler_builtins object architecture) - #156295 (Pass the whole `GenericArgs` to `Interner::for_each_relevant_impl()`) - #158375 (Support `DefKind::InlineConst` in `ConstKind::Unevaluated`) - #158556 (delegation: store child segment flag in `PathSegment`) - #158081 (trait-system: Recover deferred closure calls after errors) - #158468 (Include default-stability info in rustdoc JSON.) - #158543 (Note usage of documentation hard links in `core::io`) - #158564 (fix `-Z min-recursion-limit` unstable chapter name) - #158568 (llvm-wrapper: use accessors for private fields in LLVM 23+) - #158582 (Comment on needed RAM in huge-stacks.rs)

Rollup merge of #156295 - ChayimFriedman2:for-each-impl-args, r=lcnr Pass the whole `GenericArgs` to `Interner::for_each_relevant_impl()` Pass the whole `GenericArgs` to `Interner::for_each_relevant_impl()` And not just the self type. rustc does not make use of this, but rust-analyzer needs it to support impls in the same block as args, see https://rust-lang.zulipchat.com/#narrow/channel/144729-t-types/topic/non.20local.20impls.20for.20generic.20args/with/593629693. I'm not entirely sure this covers all cases (e.g. an unnormalized alias), and want feedback from a types team member. r? types

rustbot assigned oli-obk May 7, 2026

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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels May 7, 2026