Skip to content

Refactor CheckAttrVisitor so rustfmt can format it. - #156466

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
kpreid:checkattr
May 12, 2026
Merged

Refactor CheckAttrVisitor so rustfmt can format it.#156466
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
kpreid:checkattr

Conversation

@kpreid

Copy link
Copy Markdown
Contributor

I don’t have any need for this myself, but I heard it was annoying people and I like fixing formatting and structure problems.

The key change is that the do-nothing cases are now individual match arms instead of an enormous or-pattern. In order to achieve this cleanly, I moved the code handling Attribute::Parsed into a separate function matching AttributeKinds rather than Attributes.

This also fixes RustcAllowIncoherentImpl being non-alphabetical because it was spelled without a leading “|” (though that by itself could be achieved by adding the optional leading “|”).

The key change is that the do-nothing cases are now individual
match arms instead of an enormous or-pattern. In order to achieve this
cleanly, I moved the code handling `Attribute::Parsed` into a separate
function matching `AttributeKind`s rather than `Attribute`s.
This also fixes `RustcAllowIncoherentImpl` being non-alphabetical
because it was spelled without a leading “|”.
@rustbot

Copy link
Copy Markdown
Collaborator

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

cc @jdonszelmann, @JonathanBrouwer

@rustbotrustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 11, 2026
@rustbot

Copy link
Copy Markdown
Collaborator

r? @nnethercote

rustbot has assigned @nnethercote.
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 73 candidates
  • Random selection from 16 candidates

@JonathanBrouwerJonathanBrouwer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@bors r+ rollup
Thanks, this is prettier regardless of rustfmt issues

View changes since this review

@rust-bors

rust-borsBot commented May 11, 2026

Copy link
Copy Markdown
Contributor

📌 Commit a194d65 has been approved by JonathanBrouwer

It is now in the queue for this repository.

@rust-borsrust-borsBot 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 May 11, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request May 11, 2026
Refactor `CheckAttrVisitor` so rustfmt can format it.
I don’t have any need for this myself, but [I heard it was annoying people](https://rust-lang.zulipchat.com/#narrow/channel/147480-t-compiler.2Fdiagnostics/topic/.60.23.5Bdiagnostic.3A.3Aon_unimplemented.5D.60.20on.20trait.20methods.3F/near/594113991) and I like fixing formatting and structure problems.
The key change is that the do-nothing cases are now individual match arms instead of an enormous or-pattern. In order to achieve this cleanly, I moved the code handling `Attribute::Parsed` into a separate function matching `AttributeKind`s rather than `Attribute`s.
This also fixes `RustcAllowIncoherentImpl` being non-alphabetical because it was spelled without a leading “|” (though that by itself could be achieved by adding the optional leading “|”).
This was referenced May 11, 2026
rust-borsBot pushed a commit that referenced this pull request May 11, 2026
…uwer
Rollup of 11 pull requests
Successful merges:
- #156448 (miri subtree update)
- #155023 (Introduce move expressions (`move($expr)`) )
- #156429 (Simplify `intrinsic::raw_eq` in MIR when possible)
- #147672 (LLBC-linker: Do not strip debug symbols for the nvptx target anymore)
- #155169 (jsondoclint: simplify code using idiomatic Rust)
- #155184 ( Have arrays' `drop_glue` just unsize and call the slice version)
- #156022 (rustdoc: Fix cosmetic issues when reporting unresolved paths in `broken_intra_doc_links`)
- #156442 (Show intrinsics::gpu in docs)
- #156461 (LLVM 23: Specify `returnaddress` intrinsic return type)
- #156462 (LLVM 23: Accept float (instead of hex) literals in codegen tests)
- #156466 (Refactor `CheckAttrVisitor` so rustfmt can format it.)

@nnethercotennethercote left a comment

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.

Was this a case where rustfmt silently failed because function was too big? Those are always annoying.

View changes since this review

@kpreid

Copy link
Copy Markdown
ContributorAuthor

@nnethercote On further investigation, the problem (or at least, a problem) is not the length of the or-pattern, but having a comment in the middle of an or-pattern. If I delete just the comments from the original check_attr.rs code, then it formats. Minimized example that fails to format:

use std::cmp::Ordering;pubfntest_case(attr:Ordering){match attr {Ordering::Less// blah blah blah
| Ordering::Equal | Ordering::Greater
=> {}}}

Similar rustfmt issues: rust-lang/rustfmt#6491, rust-lang/rustfmt#4119

rust-timer added a commit that referenced this pull request May 12, 2026
Rollup merge of #156466 - kpreid:checkattr, r=JonathanBrouwer
Refactor `CheckAttrVisitor` so rustfmt can format it.
I don’t have any need for this myself, but [I heard it was annoying people](https://rust-lang.zulipchat.com/#narrow/channel/147480-t-compiler.2Fdiagnostics/topic/.60.23.5Bdiagnostic.3A.3Aon_unimplemented.5D.60.20on.20trait.20methods.3F/near/594113991) and I like fixing formatting and structure problems.
The key change is that the do-nothing cases are now individual match arms instead of an enormous or-pattern. In order to achieve this cleanly, I moved the code handling `Attribute::Parsed` into a separate function matching `AttributeKind`s rather than `Attribute`s.
This also fixes `RustcAllowIncoherentImpl` being non-alphabetical because it was spelled without a leading “|” (though that by itself could be achieved by adding the optional leading “|”).
@rust-bors
rust-borsBot merged commit 65d0e52 into rust-lang:mainMay 12, 2026
11 checks passed
@rustbotrustbot added this to the 1.97.0 milestone May 12, 2026
@kpreid
kpreid deleted the checkattr branch May 12, 2026 04:03
WhySoBad pushed a commit to WhySoBad/miri that referenced this pull request May 13, 2026
…uwer
Rollup of 11 pull requests
Successful merges:
- rust-lang/rust#156448 (miri subtree update)
- rust-lang/rust#155023 (Introduce move expressions (`move($expr)`) )
- rust-lang/rust#156429 (Simplify `intrinsic::raw_eq` in MIR when possible)
- rust-lang/rust#147672 (LLBC-linker: Do not strip debug symbols for the nvptx target anymore)
- rust-lang/rust#155169 (jsondoclint: simplify code using idiomatic Rust)
- rust-lang/rust#155184 ( Have arrays' `drop_glue` just unsize and call the slice version)
- rust-lang/rust#156022 (rustdoc: Fix cosmetic issues when reporting unresolved paths in `broken_intra_doc_links`)
- rust-lang/rust#156442 (Show intrinsics::gpu in docs)
- rust-lang/rust#156461 (LLVM 23: Specify `returnaddress` intrinsic return type)
- rust-lang/rust#156462 (LLVM 23: Accept float (instead of hex) literals in codegen tests)
- rust-lang/rust#156466 (Refactor `CheckAttrVisitor` so rustfmt can format it.)
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Aug 17, 2026
…uwer
Rollup of 11 pull requests
Successful merges:
- rust-lang/rust#156448 (miri subtree update)
- rust-lang/rust#155023 (Introduce move expressions (`move($expr)`) )
- rust-lang/rust#156429 (Simplify `intrinsic::raw_eq` in MIR when possible)
- rust-lang/rust#147672 (LLBC-linker: Do not strip debug symbols for the nvptx target anymore)
- rust-lang/rust#155169 (jsondoclint: simplify code using idiomatic Rust)
- rust-lang/rust#155184 ( Have arrays' `drop_glue` just unsize and call the slice version)
- rust-lang/rust#156022 (rustdoc: Fix cosmetic issues when reporting unresolved paths in `broken_intra_doc_links`)
- rust-lang/rust#156442 (Show intrinsics::gpu in docs)
- rust-lang/rust#156461 (LLVM 23: Specify `returnaddress` intrinsic return type)
- rust-lang/rust#156462 (LLVM 23: Accept float (instead of hex) literals in codegen tests)
- rust-lang/rust#156466 (Refactor `CheckAttrVisitor` so rustfmt can format it.)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributesArea: Attributes (`#[…]`, `#![…]`)S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@kpreid@rustbot@nnethercote@JonathanBrouwer