Skip to content

Remove NormalAttr#159221

Closed
nnethercote wants to merge 2 commits into
rust-lang:mainfrom
nnethercote:rm-NormalAttr
Closed

Remove NormalAttr#159221
nnethercote wants to merge 2 commits into
rust-lang:mainfrom
nnethercote:rm-NormalAttr

Conversation

@nnethercote

@nnethercote nnethercote commented Jul 13, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

After #158942 it's no longer necessary. Details in individual commits.

r? @petrochenkov

@nnethercote
Remove unused unwrap_normal_item methods.
5d0ace2
@nnethercote
Remove NormalAttr
8c97bb0 
By inlining `AttrItem` into `NormalAttr` and then renaming the result
`AttrItem` (because `AttrItem` is a better and more widely-used name
than `NormalAttr`).

This is a very mechanical change, except for the following.
- In `expand_cfg_attr_item`, where the commit adds some comments
  explaining the token usage and adds an assertion, in order to clarify
  existing behaviour.
- An expanded comment on `AttrItem`.
- An expanded comment on `ParseNtResult`.
@rustbot

rustbot commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

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

cc @rust-lang/rustfmt

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

cc @fmease

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

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

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

cc @ZuseZ4

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustfmt Relevant to the rustfmt team, which will review and decide on the PR/issue. labels Jul 13, 2026
petrochenkov
petrochenkov reviewed Jul 13, 2026
View reviewed changes
Comment thread compiler/rustc_ast/src/ast.rs
pub args: AttrItemKind,
// Tokens for the full attribute, e.g. `#[foo]`, `#![bar]`. (Compare this with
// `ParseNtResult::Meta`; `expand_cfg_attr_item` is where the two cases interact.)
pub tokens: Option<LazyAttrTokenStream>,

@petrochenkov petrochenkov Jul 13, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think much changed here since #158942, these tokens still do not belong to the AttrItem.
Putting them here removes one type, but instead introduces confusion, IMO.

Semantically the tokens belong to the Attribute level, they were moved to the AttrKind::Normal level only as an optimization for doc comments. Moving them into AttrItem goes even further from the right mental model.

  • Normal(Box<AttrItem>, Option<LazyAttrTokenStream>) would be fine semantically, but too large (?).
  • Normal(Box<(AttrItem, Option<LazyAttrTokenStream>)>) would be fine semantically too, and avoids the size issue, but a NormalAttr struct with named fields is probably still better than a tuple.

View changes since the review

@petrochenkov 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 13, 2026
@rust-bors

rust-bors Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

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

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

Assignees

@petrochenkov petrochenkov

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustfmt Relevant to the rustfmt team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nnethercote @rustbot @petrochenkov