Prepare to make `rustc_trivial_field_reads` usable in user code by tehforsch · Pull Request #164217 · rust-lang/rust

tehforsch · 2026-10-12T16:05:13Z

Zulip discussion:
#t-lang > diagnostics attribute to ignore code for unused_code lints
#t-compiler/help > #160621 (Allow ignoring fns for dead code analysis)

Open questions I have:

Some of the Debug impls are not pointed out as usage sites. For example this test:

#[derive(Debug)]
enum TupleVariant {
    Variant1(i32), //~ ERROR: variant `Variant1` is never constructed
    Variant2,
}

fn main() {
	let _ = TupleVariant::Variant2;
}

now loses its the note in the error message:

note: `TupleVariant` has a derived impl for the trait `Debug`, but this is intentionally ignored during dead code analysis

but in my mind this also makes sense - the Debug impl never constructs that Variant1 and even if rustc_trivial_field_reads is removed from the debug impl, the "variant never constructed" warning would still occur. Note that this doesn't apply to Clone impls, which do technically construct that variant although that brings me to point 2

If I am reading the code right, the whole point of the ignored_variant_stack is to make sure we don't mark a variant of an enum live from within a match arm that matches on that exact enum variant. But isn't that exactly what the Clone impl does? The inner match arm of it would look like Variant1(x) => Variant1(x). I looked into necessary_variants and found that it directly ignores the kind of variant we encounter, namely PatKind::TupleStruct(hir::QPath::TypeRelative(...)) but I don't really understand why and whether that's a bug or whether I am missing something

panstromek · 2026-10-12T16:57:28Z

@bors try @rust-timer queue

Prepare to make `rustc_trivial_field_reads` usable in user code

rust-bors · 2026-10-12T17:56:23Z

☀️ Try build successful (CI)
Build commit: e790d10 (e790d107e7868a817cc9ec9592a9b2d5fc634365)
Base parent: 0093837 (0093837afdf14ec7069c81da19cd51ed7d3476d9)

rust-timer · 2026-10-12T18:42:04Z

Finished benchmarking commit (e790d10): comparison URL.

Overall result: ❌ regressions - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=never rustc-perf
@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.3%	[0.1%, 0.7%]	47
Regressions ❌ (secondary)	0.4%	[0.2%, 0.8%]	19
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[0.1%, 0.7%]	47

Max RSS (memory usage)

Results (primary 5.1%)

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

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

Cycles

Results (primary 2.6%, secondary 2.1%)

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

	mean	range	count
Regressions ❌ (primary)	2.6%	[2.2%, 3.0%]	3
Regressions ❌ (secondary)	2.1%	[2.1%, 2.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.6%	[2.2%, 3.0%]	3

Binary size

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

Bootstrap: 488.816s -> 489.518s (0.14%)
Artifact size: 406.86 MiB -> 406.85 MiB (-0.00%)

mejrs · 2026-10-12T21:39:29Z

@bors try @rust-timer queue

Prepare to make `rustc_trivial_field_reads` usable in user code

rust-bors · 2026-10-12T22:39:24Z

☀️ Try build successful (CI)
Build commit: a64a0d1 (a64a0d13d3c0d5e6be02384b77719f7a3d8a651f)
Base parent: d64de46 (d64de46d0aab619b69efe68a99d43d11434b6924)

rust-timer · 2026-10-13T00:10:04Z

Finished benchmarking commit (a64a0d1): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=never rustc-perf
@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.3%	[0.1%, 0.6%]	48
Regressions ❌ (secondary)	0.4%	[0.2%, 0.9%]	16
Improvements ✅ (primary)	-0.4%	[-0.6%, -0.2%]	3
Improvements ✅ (secondary)	-0.5%	[-0.7%, -0.2%]	35
All ❌✅ (primary)	0.2%	[-0.6%, 0.6%]	51

Max RSS (memory usage)

Results (secondary 2.8%)

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

Cycles

Results (secondary -1.3%)

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)	-1.3%	[-1.3%, -1.3%]	1
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.1%, 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.1%	[0.0%, 0.2%]	74
Regressions ❌ (secondary)	0.1%	[0.0%, 0.2%]	53
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.2%]	74

Bootstrap: 490.869s -> 490.424s (-0.09%)
Artifact size: 406.63 MiB -> 406.91 MiB (0.07%)

mejrs · 2026-10-13T10:59:11Z

Let's run it again, i think the improvements are spurious.

@bors try @rust-timer queue

Before, when a trivial fn was used from multiple seeds, the "used here but ignored" note would appear twice.

tehforsch · 2026-11-06T15:45:19Z

I addressed some changes and added some tests and am making this ready for review. This still adds a good amount of complexity to some already complex code, so I'm still interested in the approach in #164913 .

Also we definitely have to check perf again - since the last perf checks, I added more complex diagnostics and changed the logic that decides whether items should be scanned or not, so this can definitely have an impact.

Open questions

The two questions in the PR description still apply, the first is a change of behavior in this PR that makes sense to me. The second I think is just a bug?

There is also one more decision: An item can be unused but be used from multiple trivial usage sites. In this case, I emit one note per usage site. However, diagnostics are also grouped together (for the fields in a struct, for example, if many of them are unused) and in this case I am now emitting one note per usage site and then point out all the fields that are unused from that particular site. An example diagnostic, taken from the grouped-diagnostic.rs test:

error: fields `first` and `second` are never read
  --> $DIR/grouped-diagnostic.rs:6:5
   |
LL | struct Struct {
   |        ------ fields in this struct
LL |     first: bool,
   |     ^^^^^
LL |     second: bool,
   |     ^^^^^^
   |
note: `first` is used here, but this is intentionally ignored during dead code analysis
  --> $DIR/grouped-diagnostic.rs:11:1
   |
LL | fn read_first(data: &Struct) {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: `second` is used here, but this is intentionally ignored during dead code analysis
  --> $DIR/grouped-diagnostic.rs:16:1
   |
LL | fn read_second(data: &Struct) {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: `first` and `second` are used here, but this is intentionally ignored during dead code analysis
  --> $DIR/grouped-diagnostic.rs:21:1
   |
LL | fn read_first_and_second(data: &Struct) {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I think this is quite clear, but we could also do this in other ways.

Finally, it's not entirely clear what to do about propagation of trivial usage sites from traits to their impls. I don't know whether we have to get this completely right in this PR though

rustbot · 2026-11-06T15:48:30Z

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann, @JonathanBrouwer

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

cc @jdonszelmann, @JonathanBrouwer

Changes to the code generated for builtin derived traits.

cc @nnethercote

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

rustbot · 2026-11-06T15:48:34Z

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
compiler expanded to 79 candidates
Random selection from 22 candidates

tehforsch · 2026-11-06T20:22:38Z

r? @mejrs

mejrs

Can you remove the Closes #160621 from the PR description? This is just the first step for doing that :)

I think 1) is fine.

As for 2), I'm not totally sure, I need to play with this myself. Note though, that in

 match x { Variant1(x) => Variant1(x) }

left and right two different things, the left is a pattern, the right is a function call (enum constructors are functions) call or a struct literal expression if we did this but with a struct.

It seems that ignore_variant_stack is dead? I was able to remove it with no effect on the test suite. Maybe since #148262? In any case, let's not do that in this PR.

View changes since this review

mejrs · 2026-11-07T23:31:58Z

+                let (span, usage_kind) = if usage_site.attr_span.in_derive_expansion() {
+                    (usage_site.attr_span, "in this derive macro")
+                } else if usage_site.attr_span.in_external_macro(self.tcx.sess.source_map()) {
+                    (usage_site.attr_span, "in this macro invocation")
+                } else {
+                    (self.tcx.hir_span(usage_site.hir_id), "here")
+                };


This produces notes such as

`f` is used in this derive macro

and then the note points out where the macro is used, but this approach is flawed because that is not always shown, for example people might be reading it in ide mouseover (which typically only shows the text). In that case "this" is not very helpful.

Can you name the macro in the note? You can get the macro's name from the span as well, with span.ctxt().outer_expn_data().

Typical parlance is smth like "originates in the macro foo", "used in the expansion of derive macro foo", etc.

Makes sense, I changed it in 51b6620

Now reads

error: field `0` is never read --> $DIR/from-macro.rs:11:15 | LL | struct Struct(usize); | ------ ^^^^^ | | | field in this struct | = help: consider removing this field note: `0` is used in the expansion of the derive macro `Clone`, but this is intentionally ignored during dead code analysis --> $DIR/from-macro.rs:10:10 | LL | #[derive(Clone)] | ^^^^^ note: `0` is used in the expansion of the macro `foo`, but this is intentionally ignored during dead code analysis --> $DIR/from-macro.rs:16:9 | LL | fn foo(s: &Struct) { | ^^^^^^^^^^^^^^^^^^ ... LL | foo!(); | ------ in this macro invocation note: `0` is used in the expansion of the macro `bar`, but this is intentionally ignored during dead code analysis --> $DIR/from-macro.rs:23:1 | LL | bar!(Struct); | ^^^^^^^^^^^^

tehforsch · 2026-11-08T14:15:02Z

Can you remove the Closes #160621 from the PR description? This is just the first step for doing that :)

Oh right - done

It seems that ignore_variant_stack is dead? I was able to remove it with no effect on the test suite. Maybe since #148262? In any case, let's not do that in this PR.

Ok, I can check this in more detail and maybe open an issue afterwards.

I applied your suggestions from above.

tehforsch force-pushed the fix-160621 branch from c06e80c to cf722b6 Compare October 12, 2026 16:12