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

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

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.

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-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-14T10:43:22Z

-/// When adding new early parsed attributes, consider whether they should be pretty printed.
+/// Synthetic attributes are inserted by the compiler and cannot be written in source code. They
+/// receive special treatment in various ways because they must not affect observable behaviour:
+/// they are invisible to proc macros, cannot be pretty-printed, and are unable to re-enter the


These attributes are not printed in AST pretty-printing, because people parse the pretty-printed expanded code in practice and we shouldn't break this scenario if we can.
They are printed in HIR pretty-printing because people mostly stopped trying to parse the pretty-printed HIR nowadays :D

"Observable" here means language behavior (including input for proc macros).
The effects from keeping the traces can still be observable in rustdoc output, lints, diagnostics, etc.

View changes since the review

petrochenkov · 2026-07-14T10:47:50Z

                        .zip(name)
                        .all(|(s, n)| s.args.is_none() && s.ident.name == *n)
            }
+            AttrKind::Synthetic(..) => false,


Suggested change

AttrKind::Synthetic(..) => false,

AttrKind::Synthetic(..) | AttrKind::DocComment(..) => false,

And a couple more similar cases below.

View changes since the review

petrochenkov · 2026-07-14T10:54:03Z

                .unwrap_or_else(|| panic!("attribute is missing tokens: {self:?}"))
                .to_attr_token_stream()
                .to_token_trees(),
+            AttrKind::Synthetic(..) => vec![],


I think this place is now load-bearing and replaces what attr_into_trace previously did (emptying the attribute's token stream), making the traces unobservable to proc macros.
It deserves a comment.

View changes since the review

petrochenkov · 2026-07-14T11:00:09Z

-    }
-
-    pub fn unwrap_normal_item(self) -> AttrItem {
+    pub fn convert_normal_to_synthetic(&mut self, synthetic_attr: SyntheticAttr) {


Suggested change

pub fn convert_normal_to_synthetic(&mut self, synthetic_attr: SyntheticAttr) {

pub fn convert_normal_to_synthetic(self, synthetic_attr: SyntheticAttr) -> Attribute {

Both of the existing uses would fit into this signature better.

View changes since the review

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 added 6 commits July 14, 2026 15:33

Remove unused unwrap_normal_item methods.

70d8bc7

Improve an attribute check

5b84683

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

04065f8

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

Remove sym::cfg_trace and sym::cfg_attr_trace

9e65a16

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.

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

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

	AttrKind::Synthetic(..) => false,
	AttrKind::Synthetic(..) \| AttrKind::DocComment(..) => false,

	pub fn convert_normal_to_synthetic(&mut self, synthetic_attr: SyntheticAttr) {
	pub fn convert_normal_to_synthetic(self, synthetic_attr: SyntheticAttr) -> Attribute {

Uh oh!

Uh oh!

Conversation

nnethercote commented Jul 14, 2026 • edited 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

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

nnethercote commented Jul 14, 2026

Uh oh!

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

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

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

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

petrochenkov commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nnethercote commented Jul 14, 2026 •

edited

Loading

petrochenkov Jul 14, 2026 •

edited by rustbot

Loading

petrochenkov Jul 14, 2026 •

edited by rustbot

Loading

petrochenkov Jul 14, 2026 •

edited by rustbot

Loading

petrochenkov Jul 14, 2026 •

edited by rustbot

Loading