Skip to content

Some cleanup in rustc_attr_parsing#159508

Merged
rust-bors[bot] merged 12 commits into
rust-lang:mainfrom
qaijuang:rustc-attr-parsing-cleanup
Jul 19, 2026
Merged

Some cleanup in rustc_attr_parsing#159508
rust-bors[bot] merged 12 commits into
rust-lang:mainfrom
qaijuang:rustc-attr-parsing-cleanup

Conversation

@qaijuang

@qaijuang qaijuang commented Jul 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

My hands got itchy while digging through a few modules.

...and maybe run perf? IDK

r? compiler

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 18, 2026
@qaijuang
qaijuang marked this pull request as ready for review July 18, 2026 12:10
@qaijuang

qaijuang commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot 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 18, 2026
mejrs
mejrs requested changes Jul 18, 2026
View reviewed changes

@mejrs mejrs left a comment •
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.

Unfortunately having comments in the middle of a pattern breaks rustfmt:

fn foo() {
    match a {
        a |
        // comment
               all_the_way_over_here
    }
}

will not be formatted. Can you amend this pr to not put comments in patterns?

View changes since this review

@rustbot rustbot 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 18, 2026
@rustbot

rustbot commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@qaijuang

qaijuang commented Jul 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Unfortunately having comments in the middle of a pattern breaks rustfmt:

fn foo() {
    match a {
        a |
        // comment
               all_the_way_over_here
    }
}

will not be formatted. Can you amend this pr to not put comments in patterns?

Addressed!

@rustbot ready

@rustbot

rustbot commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in check-cfg diagnostics

cc @Urgau

@rustbot rustbot 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 18, 2026
mejrs
mejrs requested changes Jul 18, 2026
View reviewed changes

@mejrs mejrs left a comment •
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.

Thanks, I've had the time to take a more detailed look now.

Can you undo the match -> if changes in cfg_select.rs? Also those deleted comments really should be FIXME's instead, not be deleted. Turning these into FIXME's would be the proper cleanup. #149960 (comment) would be useful to link to from that FIXME as well.

View changes since this review

Comment thread compiler/rustc_attr_parsing/src/session_diagnostics.rs Outdated
diag.span_label(
self.span,
format!("expected this to be of the form `... = \"...\"`"),
"expected this to be of the form `... = \"...\"`".to_string(),

@mejrs mejrs Jul 18, 2026

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.

Suggested change
"expected this to be of the form `... = \"...\"`".to_string(),
"expected this to be of the form `... = \"...\"`",

It accepts &'static str as well as String.

@qaijuang qaijuang Jul 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done! good catch

@rustbot ready

@rustbot rustbot 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 18, 2026
@jieyouxu jieyouxu assigned mejrs and unassigned jieyouxu Jul 19, 2026
@rustbot rustbot 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 19, 2026
mejrs
mejrs approved these changes Jul 19, 2026
View reviewed changes

@mejrs mejrs left a comment •
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.

@bors r+ rollup

View changes since this review

@rust-bors

rust-bors Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 6109397 has been approved by mejrs

It is now in the queue for this repository.

@rust-bors 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 19, 2026
@jhpratt jhpratt mentioned this pull request Jul 19, 2026
Merged
rust-bors Bot pushed a commit that referenced this pull request Jul 19, 2026
@bors
Auto merge of #159556 - jhpratt:rollup-h68KPYG, r=jhpratt
234c31c 
Rollup of 5 pull requests

Successful merges:

 - #158546 (Move `std::io::BufRead` to `alloc::io`)
 - #159372 (Remove SIZE generic param from DisplayBuffer in core::net)
 - #159548 (miri subtree update)
 - #159508 (Some cleanup in `rustc_attr_parsing`)
 - #159539 ([rustc_abi] Customize `Debug` for `Primitive` and `Scalar`)
@rust-bors
rust-bors Bot merged commit 052331f into rust-lang:main Jul 19, 2026
13 checks passed
@rustbot rustbot added this to the 1.99.0 milestone Jul 19, 2026
rust-timer added a commit that referenced this pull request Jul 19, 2026
@rust-timer
Unrolled build for #159508
fd3d254 
Rollup merge of #159508 - qaijuang:rustc-attr-parsing-cleanup, r=mejrs

Some cleanup in `rustc_attr_parsing`

My hands got itchy while digging through a few modules.

...and maybe run perf? IDK

r? compiler
@qaijuang
qaijuang deleted the rustc-attr-parsing-cleanup branch July 19, 2026 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mejrs mejrs mejrs approved these changes

Assignees

@mejrs mejrs

Labels

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

Projects

None yet

Milestone

1.99.0

Development

Successfully merging this pull request may close these issues.

4 participants

@qaijuang @rustbot @mejrs @jieyouxu