Overhaul `CfgTrace`/`CfgAttrTrace` handling by nnethercote · Pull Request #159266 · rust-lang/rust

nnethercote · 2026-07-14T05:38:13Z

View all comments

I noticed that AttrItemKind was a bit clunky and by the time I was finished I had a PR that:

removes AttrItemKind;
removes sym::cfg_trace and sym::cfg_attr_trace;
overhauls how these attributes are handled;
changes the terminology used for these attributes;
fixes some erroneous comments;
and a few other small improvements.

It's a sizeable change but I think it makes things a lot clearer. Details in individual commits.

r? @petrochenkov

rustbot · 2026-07-14T05:38:16Z

Some changes occurred in compiler/rustc_builtin_macros/src/autodiff.rs

cc @ZuseZ4

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

clippy is developed in its own repository. If possible, consider making this change to rust-lang/rust-clippy instead.

cc @rust-lang/clippy

The parser was modified, potentially altering the grammar of (stable) Rust
which would be a breaking change.

cc @fmease

nnethercote · 2026-07-14T05:40:05Z

cc @JonathanBrouwer

The final two commits are opinionated, especially the final one. I do think the new terminology is much better, as explained in the commit logs, but I would appreciate careful checking to make sure I haven't overlooked or misinterpreted anything. In particular, if there are any future plans for handling AST attributes that might be relevant.

nnethercote · 2026-07-14T05:40:27Z

@bors try @rust-timer queue

Overhaul `CfgTrace`/`CfgAttrTrace` handling

JonathanBrouwer · 2026-07-14T06:15:41Z

(Would like to take a look at this as well, might take me a few days to get to it)

petrochenkov · 2026-07-14T06:38:41Z

There's one more PR in flight changing the data in cfg_attr traces - #158980, this PR may depend on it (depending on what it does).

nnethercote · 2026-07-14T07:35:37Z

this PR may depend on it

There will be conflicts but I think they will be superficial. That PR is about adding an argument to CfgAttrTrace. This PR moves CfgAttrTrace around but doesn't change what it's doing.

rust-bors · 2026-07-14T07:52:25Z

☀️ Try build successful (CI)
Build commit: 61408d1 (61408d1532b7aa37a3d4bf619a1e7d38a884bb06)
Base parent: 55b6bd8 (55b6bd8ecbed23599e966a04508a8a13cbfe90b3)

rust-timer · 2026-07-14T08:32:33Z

Finished benchmarking commit (61408d1): comparison URL.

Overall result: ✅ improvements - no action needed

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.

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

Max RSS (memory usage)

Results (primary 4.4%, 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)	4.4%	[4.4%, 4.4%]	1
Regressions ❌ (secondary)	1.7%	[1.0%, 3.5%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.4%	[-4.7%, -4.1%]	2
All ❌✅ (primary)	4.4%	[4.4%, 4.4%]	1

Cycles

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

Binary size

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

Bootstrap: 489.277s -> 489.827s (0.11%)
Artifact size: 389.95 MiB -> 389.32 MiB (-0.16%)

nnethercote · 2026-07-14T09:18:25Z

Some tiny perf wins? Wasn't the goal here, but I'll take it :)

petrochenkov · 2026-07-14T11:12:13Z

I like all the changes here, including the last commits.

(Most of the code refactored here was added during the rustc_attr_parsing migration, previously the cfg trace attributes were just regular normal attributes with a weird non-identifier name and emptied tokens).

nnethercote · 2026-07-14T23:43:32Z

I added a commit that addresses @petrochenkov's comments. Now waiting for @JonathanBrouwer's feedback, which might take a few days.

nnethercote · 2026-07-15T02:27:34Z

The job pr-check-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Caused by a network failure:

fatal: unable to access 'https://github.com/rust-lang-nursery/rust-toolstate.git/': Failed to connect to github.com port 443 after 133489 ms

petrochenkov · 2026-07-15T10:25:20Z

r=me
r? @JonathanBrouwer

Once `sym::ignore` has been matched, the attribute can't be a `DocComment` and trying to match one is pointless.

To explain some things that weren't obvious to me.

`AttrItem` currently contains an `AttrItemKind` which is either `Parsed` or `Unparsed`. In the `Parsed` case the surrounding `AttrItem` is basically fake, with a meaningless `unsafety` field, a synthetic (non-ident) symbol, and an empty `tokens`. This commit moves `Parsed` up one level to become a new variant of `AttrKind`. It no longer needs to pretend to be an `AttrKind::Normal`, and makes a number of things simpler: - No `unparsed_ref` calls needed. - Replaces the `attr_into_trace` and `Attribute::replace_args` mess with the much simpler `convert_normal_to_parsed`. The commit converts a numberof non-exhaustive `AttrKind` matches (those that don't involve matching a single attribute) to exhaustive, for future-proofing. It uses `unreachable!` on all the `AttrKind::Parsed` cases that aren't hit by the full test suite.

This commit pushes further than the previous one, relying more heavily on looking directly at `AttrKind::Parsed` to handle various cases. Note that the removed clippy code was actually dead.

AST attributes use "early parsed"/"parsed" terminology to refer to the `CfgTrace` and `CfgAttrTrace` attributes. I think this terminology is meant to echo the terminology used for HIR attributes, i.e. `hir::Attribute::{Parsed,Unparsed}`, probably because `hir::Attribute::Parsed` is used for attributes that aren't stored in a token-based form. But this naming is misleading. "Early parsed" attributes aren't parsed at all because they are inserted by the compiler and cannot be written in source code. There are also two comments that claim that these attributes are kept in parsed form "so they don't have to be reparsed every time they're used, for performance", which is simply incorrect. This commit renames these as "synthetic" attributes, which better reflects their nature. The commit also fixes the incorrect comments. Note that `is_parsed_attribute` is unchanged, because it refers to the HIR attribute meaning. (And the removal of the synthetic attributes from it in the previous commit is now more obviously correct.)

rustbot · 2026-07-16T23:43:52Z

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.

JonathanBrouwer

This makes the code significantly nicer, all commits are also welcome by me, thanks a lot for changing it :)

@bors r=petrochenkov,JonathanBrouwer rollup=never note=perf

View changes since this review

rust-bors · 2026-07-17T12:05:19Z

📌 Commit fa56a01 has been approved by petrochenkov,JonathanBrouwer

It is now in the queue for this repository.

rust-bors · 2026-07-17T16:37:07Z

☀️ Test successful - CI
Approved by: petrochenkov,JonathanBrouwer
Duration: 3h 18m 29s
Pushing 656ccbe to main...

github-actions · 2026-07-17T16:40:19Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 4a9d536 (parent) -> 656ccbe (this PR)

Test differences

Show 130 test diffs

130 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 656ccbe796ff98def9b555c118e1620c5389e3b2 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

x86_64-msvc-2: 1h 42m -> 2h 20m (+36.9%)
i686-gnu-nopt-1: 2h 20m -> 1h 29m (-36.1%)
dist-apple-various: 1h 36m -> 2h 11m (+36.1%)
x86_64-msvc-ext3: 1h 23m -> 1h 52m (+34.2%)
dist-powerpc64le-linux-musl: 1h 41m -> 1h 10m (-30.8%)
dist-i686-mingw: 2h 48m -> 2h 3m (-26.6%)
dist-ohos-armv7: 58m 27s -> 1h 13m (+26.4%)
i686-msvc-2: 1h 59m -> 1h 28m (-26.2%)
dist-arm-linux-musl: 1h 16m -> 1h 36m (+25.3%)
dist-x86_64-linux-alt: 1h 53m -> 2h 21m (+25.1%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2026-07-17T18:36:17Z

Finished benchmarking commit (656ccbe): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -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.4%	[-0.6%, -0.2%]	11
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.2%]	1
All ❌✅ (primary)	-0.4%	[-0.6%, -0.2%]	11

Max RSS (memory usage)

Results (primary 4.5%, secondary -3.4%)

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

	mean	range	count
Regressions ❌ (primary)	4.5%	[2.8%, 6.7%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.4%	[-5.6%, -1.7%]	3
All ❌✅ (primary)	4.5%	[2.8%, 6.7%]	3

Cycles

Results (secondary 2.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)	8.0%	[8.0%, 8.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.0%	[-4.0%, -4.0%]	1
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 491.028s -> 490.561s (-0.10%)
Artifact size: 389.42 MiB -> 390.05 MiB (0.16%)

rustbot assigned petrochenkov Jul 14, 2026

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 14, 2026

rust-bors Bot pushed a commit that referenced this pull request Jul 14, 2026

Auto merge of #159266 - nnethercote:rm-AttrItemKind, r=<try>

61408d1

Overhaul `CfgTrace`/`CfgAttrTrace` handling

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 14, 2026

petrochenkov reviewed Jul 14, 2026

View reviewed changes

Comment thread compiler/rustc_ast/src/ast.rs Outdated

Comment thread compiler/rustc_ast/src/attr/mod.rs Outdated

Comment thread compiler/rustc_ast/src/attr/mod.rs

Comment thread compiler/rustc_ast/src/attr/mod.rs Outdated

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 Jul 14, 2026

This comment has been minimized.

Sign in to view

petrochenkov 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 Jul 15, 2026

petrochenkov approved these changes Jul 15, 2026

View reviewed changes

rustbot assigned JonathanBrouwer and unassigned petrochenkov Jul 15, 2026

nnethercote added 7 commits July 17, 2026 08:55

Remove unused unwrap_normal_item methods.

e8c660b

Improve an attribute check

0e29789

Once `sym::ignore` has been matched, the attribute can't be a `DocComment` and trying to match one is pointless.

Expand some comments about cfg_trace/cfg_attr_trace

d0042c3

To explain some things that weren't obvious to me.

Remove sym::cfg_trace and sym::cfg_attr_trace

1a8ecb0

This commit pushes further than the previous one, relying more heavily on looking directly at `AttrKind::Parsed` to handle various cases. Note that the removed clippy code was actually dead.

Address review comments.

fa56a01

nnethercote force-pushed the rm-AttrItemKind branch from dc82ea1 to fa56a01 Compare July 16, 2026 23:43

JonathanBrouwer approved these changes Jul 17, 2026

View reviewed changes

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 Jul 17, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 17, 2026

rust-bors Bot merged commit 656ccbe into rust-lang:main Jul 17, 2026
14 checks passed

rustbot added this to the 1.99.0 milestone Jul 17, 2026

Uh oh!

Uh oh!

Conversation

nnethercote commented Jul 14, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 14, 2026

Uh oh!

nnethercote commented Jul 14, 2026

Uh oh!

nnethercote commented Jul 14, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

JonathanBrouwer commented Jul 14, 2026

Uh oh!

petrochenkov commented Jul 14, 2026

Uh oh!

nnethercote commented Jul 14, 2026

Uh oh!

rust-bors Bot commented Jul 14, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Jul 14, 2026

Overall result: ✅ improvements - no action needed

Uh oh!

nnethercote commented Jul 14, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

petrochenkov commented Jul 14, 2026

Uh oh!

nnethercote commented Jul 14, 2026

Uh oh!

This comment has been minimized.

nnethercote commented Jul 15, 2026

Uh oh!

This comment has been minimized.

petrochenkov commented Jul 15, 2026

Uh oh!

rustbot commented Jul 16, 2026

Uh oh!

JonathanBrouwer left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-bors Bot commented Jul 17, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Jul 17, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jul 17, 2026

Test differences

Job duration changes

Uh oh!

rust-timer commented Jul 17, 2026

Overall result: ✅ improvements - no action needed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nnethercote commented Jul 14, 2026 •

edited by rustbot

Loading

JonathanBrouwer left a comment •

edited by rustbot

Loading