Make `AssocItem` aware of its impl kind #145186

camsteffen · 2025-08-09T21:09:19Z

The general goal is to have fewer query dependencies by making AssocItem aware of its parent impl kind (inherent vs. trait) without having to query the parent def_kind.

Introduce enum hir::ImplItemImplKind::{Inherent, Trait} at ImplItem.impl_kind which allows ImplItem to be aware of its containing impl kind, and also move impl-kind-specific fields into corresponding variants.
Split AssocItemContainer::Impl into AssocItemContainer::{InherentImpl, TraitImpl}.
Use AssocItemContainer in numerous places.

rustbot · 2025-08-09T21:09:24Z

r? @petrochenkov

rustbot has assigned @petrochenkov.
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

rustbot · 2025-08-09T21:09:27Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_sanitizers

cc @rcvalle

changes to the core type system

cc @compiler-errors, @lcnr

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

camsteffen · 2025-08-10T00:59:08Z

src/librustdoc/clean/mod.rs

@@ -1258,7 +1258,10 @@ pub(crate) fn clean_impl_item<'tcx>(
            })),
            hir::ImplItemKind::Fn(ref sig, body) => {
                let m = clean_function(cx, sig, impl_.generics, ParamsSrc::Body(body));
-                let defaultness = cx.tcx.defaultness(impl_.owner_id);
+                let defaultness = match impl_.impl_kind {
+                    hir::ImplItemImplKind::Inherent { .. } => hir::Defaultness::Final,


Maybe this should be None?

No opinion.

compiler-errors · 2025-08-10T03:06:51Z

@bors2 try @rust-timer queue

Make `AssocItem` aware of its impl kind

rust-bors · 2025-08-10T03:06:55Z

⌛ Trying commit 77a42ec with merge ecc5941…

To cancel the try build, run the command @bors try cancel.

rust-bors · 2025-08-10T05:26:01Z

☀️ Try build successful (CI)
Build commit: ecc5941 (ecc59411274ca4c68dad1b5a2b144afaf3510243, parent: 41ede7bd9bfbc9dc1e46c5c7de3b26524e5c61f0)

rust-timer · 2025-08-10T06:34:56Z

Finished benchmarking commit (ecc5941): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@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)	2.0%	[0.5%, 3.2%]	3
Regressions ❌ (secondary)	2.3%	[1.6%, 2.9%]	2
Improvements ✅ (primary)	-0.4%	[-1.7%, -0.2%]	20
Improvements ✅ (secondary)	-0.5%	[-1.1%, -0.2%]	40
All ❌✅ (primary)	-0.1%	[-1.7%, 3.2%]	23

Max RSS (memory usage)

Results (primary 1.4%, secondary -1.7%)

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

	mean	range	count
Regressions ❌ (primary)	2.2%	[0.5%, 6.3%]	21
Regressions ❌ (secondary)	3.4%	[1.2%, 6.7%]	3
Improvements ✅ (primary)	-2.3%	[-4.2%, -1.3%]	5
Improvements ✅ (secondary)	-6.9%	[-7.0%, -6.8%]	3
All ❌✅ (primary)	1.4%	[-4.2%, 6.3%]	26

Cycles

Results (primary 3.3%, secondary 2.3%)

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

	mean	range	count
Regressions ❌ (primary)	3.3%	[3.3%, 3.3%]	1
Regressions ❌ (secondary)	2.3%	[1.6%, 3.0%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.3%	[3.3%, 3.3%]	1

Binary size

Results (primary 0.3%, secondary 0.2%)

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

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.1%, 1.8%]	10
Regressions ❌ (secondary)	1.8%	[0.2%, 4.8%]	4
Improvements ✅ (primary)	-0.5%	[-1.6%, -0.1%]	4
Improvements ✅ (secondary)	-0.4%	[-0.5%, -0.1%]	12
All ❌✅ (primary)	0.3%	[-1.6%, 1.8%]	14

Bootstrap: 463.257s -> 466.12s (0.62%)
Artifact size: 377.39 MiB -> 375.29 MiB (-0.56%)

compiler/rustc_middle/src/ty/mod.rs

petrochenkov · 2025-08-11T16:47:15Z

Oh! I misunderstood that. Okay, I'm going to look back over things with that in mind. I think you're right - I'll revert this change and maybe others.

@rustbot author

rustbot · 2025-08-11T16:47:20Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

camsteffen · 2025-08-11T19:09:42Z

@rustbot ready (pending CI)

petrochenkov · 2025-08-11T19:26:42Z

@bors2 try @rust-timer queue

Make `AssocItem` aware of its impl kind

rust-bors · 2025-08-11T21:39:42Z

☀️ Try build successful (CI)
Build commit: 68df66d (68df66da903eecbd55c3d2a4c8862df210670e24, parent: fce0e74720d199eb7839fdb51af35ac5226da178)

rust-timer · 2025-08-12T03:08:25Z

Finished benchmarking commit (68df66d): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@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
Improvements ✅ (primary)	-0.2%	[-0.3%, -0.2%]	15
Improvements ✅ (secondary)	-0.4%	[-0.7%, -0.2%]	22
All ❌✅ (primary)	-0.2%	[-0.3%, -0.2%]	15

Max RSS (memory usage)

Results (primary -0.4%, secondary -6.8%)

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

	mean	range	count
Regressions ❌ (primary)	1.3%	[1.3%, 1.3%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.2%	[-1.6%, -0.8%]	2
Improvements ✅ (secondary)	-6.8%	[-6.9%, -6.8%]	3
All ❌✅ (primary)	-0.4%	[-1.6%, 1.3%]	3

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

Results (secondary -0.4%)

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)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.4%	[-0.4%, -0.4%]	6
All ❌✅ (primary)	-	-	0

Bootstrap: 464.993s -> 464.709s (-0.06%)
Artifact size: 377.36 MiB -> 377.31 MiB (-0.01%)

bors · 2025-08-12T17:23:25Z

☔ The latest upstream changes (presumably #145300) made this pull request unmergeable. Please resolve the merge conflicts.

petrochenkov · 2025-08-12T17:02:08Z

compiler/rustc_middle/src/ty/assoc.rs

@@ -25,6 +26,7 @@ pub struct AssocItem {

    /// If this is an item in an impl of a trait then this is the `DefId` of


The comment is incorrect, this field is currently set to Some for AssocItemContainer::Trait as well, but not always (?!)

It should either be consistently set for all AssocItemContainer::Traits, or not set for any of them.
In the latter case the conditions container == AssocItemContainer::Trait && trait_item_def_id.is_some() will become redundant.

In general, there's a lot of duplication in AssocItem.

trait_item_def_id is the same as def_id for trait items

container is almost a duplicate of trait_item_def_id, except for error recovery cases

The comment is incorrect, this field is currently set to Some for AssocItemContainer::Trait as well, but not always (?!)

It seems to me somebody decided to put Some there cause it's "free", but it's not the purpose of the field. I could fix this, maybe in a separate PR? Or I could fix the code comment.

container is almost a duplicate of trait_item_def_id, except for error recovery cases

I noticed this as well. This change makes the two things closer to being redundant, but they are still not totally redundant. So ultimately I think this change is good and I don't see a way to simplify it.

I'm talking about this because changing representation of AssocItem would require rewriting half of the changes in this PR, so it makes sense to not separate them.
(The HIR change can be landed separately though.)

The non-redundant representation for AssocItem would be

struct AssocItem { def_id: DefId, kind: AssocKind, other_kind: OtherKind, } enum OtherKind { Trait, InherentImpl, TraitImpl(Option<DefId>), }

Gotcha. I can give that I try.

petrochenkov · 2025-08-12T17:06:41Z

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

            ty::AssocItemContainer::Trait => impl_ty,
-            ty::AssocItemContainer::Impl => tcx.associated_item(impl_ty.trait_item_def_id.unwrap()),
+            ty::AssocItemContainer::TraitImpl => {
+                tcx.associated_item(impl_ty.trait_item_def_id.unwrap())


This is a sleeping ICE, because trait_item_def_id can indeed be None if there are compilation errors.

src/librustdoc/passes/collect_intra_doc_links.rs

rustbot assigned petrochenkov Aug 9, 2025

This comment has been minimized.

Sign in to view

camsteffen force-pushed the assoc-impl-kind branch from 0617ff2 to cfed1ee Compare August 9, 2025 22:31

This comment has been minimized.

Sign in to view

camsteffen force-pushed the assoc-impl-kind branch from cfed1ee to 77a42ec Compare August 10, 2025 00:30

camsteffen commented Aug 10, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Aug 10, 2025

Auto merge of #145186 - camsteffen:assoc-impl-kind, r=<try>

ecc5941

Make `AssocItem` aware of its impl kind

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 10, 2025

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Aug 10, 2025

This comment was marked as resolved.

Sign in to view

cjgillot reviewed Aug 11, 2025

View reviewed changes

compiler/rustc_middle/src/ty/mod.rs Outdated Show resolved Hide resolved

cjgillot reviewed Aug 11, 2025

View reviewed changes

compiler/rustc_middle/src/ty/mod.rs Outdated Show resolved Hide resolved

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 11, 2025

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 11, 2025

camsteffen added 4 commits August 11, 2025 14:00

Introduce hir::ImplItemImplKind

167df48

Split AssocItemContainer::{InherentImpl,TraitImpl}

4d28a9d

Don't store defaultness for inherent impl items

27742c8

Use AssocItemContainer more

1d8ed9f

camsteffen force-pushed the assoc-impl-kind branch from 77a42ec to 1d8ed9f Compare August 11, 2025 19:02

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 11, 2025

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Aug 11, 2025

Auto merge of #145186 - camsteffen:assoc-impl-kind, r=<try>

68df66d

Make `AssocItem` aware of its impl kind

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 11, 2025

petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 11, 2025

camsteffen mentioned this pull request Aug 11, 2025

Limit impl_trait_header query to only trait impls #144607

Open

This comment has been minimized.

Sign in to view

rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Aug 12, 2025

petrochenkov added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 12, 2025

petrochenkov reviewed Aug 12, 2025

View reviewed changes

petrochenkov 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 Aug 12, 2025

		@@ -25,6 +26,7 @@ pub struct AssocItem {

		/// If this is an item in an impl of a trait then this is the `DefId` of

Make AssocItem aware of its impl kind #145186

Are you sure you want to change the base?

Make AssocItem aware of its impl kind #145186

Uh oh!

Conversation

camsteffen commented Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Aug 9, 2025

Uh oh!

rustbot commented Aug 9, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented Aug 10, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Aug 10, 2025

Uh oh!

rust-bors bot commented Aug 10, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Aug 10, 2025

Overall result: ❌✅ regressions and improvements - please read the text below

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

petrochenkov commented Aug 11, 2025

Uh oh!

rustbot commented Aug 11, 2025

Uh oh!

camsteffen commented Aug 11, 2025

Uh oh!

petrochenkov commented Aug 11, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors bot commented Aug 11, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Aug 12, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

bors commented Aug 12, 2025

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Make `AssocItem` aware of its impl kind #145186

Make `AssocItem` aware of its impl kind #145186

camsteffen commented Aug 9, 2025 •

edited

Loading