Skip to content

More cleanup in rustc_attr_parsing#159875

Open
qaijuang wants to merge 1 commit into
rust-lang:mainfrom
qaijuang:159508-follow-up
Open

More cleanup in rustc_attr_parsing#159875
qaijuang wants to merge 1 commit into
rust-lang:mainfrom
qaijuang:159508-follow-up

Conversation

@qaijuang

@qaijuang qaijuang commented Jul 24, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Follow up to #159508.

r? @mejrs

@rustbot

rustbot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to diagnostic attributes.

cc @mejrs

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

@rustbot rustbot 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 Jul 24, 2026
mejrs
mejrs requested changes Jul 25, 2026
View reviewed changes
Comment thread compiler/rustc_attr_parsing/src/attributes/diagnostic/mod.rs Outdated
cx.emit_err(e);
}
}
(Mode::RustcOnUnimplemented, sym::on) if is_root => {

@mejrs mejrs Jul 25, 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.

Can you undo the changes in this arm? The reason it doesn't fallthrough is because as part of future work this will need to emit a different error/lint:

(Mode::RustcOnUnimplemented, sym::on)=> {
      if is root {
            ...      
      } else {
            // error about how nested filters are not supported
      }
}

and I don't feel like having to revert this when I/someone gets to it.

(and, ideally, this will be supported for more modes, so I don't want to duplicate the pattern either...)

View changes since the review

Comment thread compiler/rustc_attr_parsing/src/attributes/macro_attrs.rs Outdated
Comment on lines +152 to +158
if let Some(sym::local_inner_macros) =
l.meta_item_no_args().and_then(|i| i.path().word_sym())
{
true
} else {
cx.adcx().warn_ill_formed_attribute_input(INVALID_MACRO_EXPORT_ARGUMENTS);
return None;

@mejrs mejrs Jul 25, 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.

this can be further simplified with word_is I believe

View changes since the review

@qaijuang qaijuang Jul 25, 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.

Simplified as in?

if l.meta_item_no_args().is_some_and(|m| m.path().word_is(sym::local_inner_macros)) {
   true
} else {
   cx.adcx().warn_ill_formed_attribute_input(INVALID_MACRO_EXPORT_ARGUMENTS);
   return None;
}

Comment thread compiler/rustc_attr_parsing/src/attributes/unroll.rs
Comment on lines 30 to 35
if let Ok(val) = u32::try_from(val.get()) {
return Some(AttributeKind::Unroll(UnrollAttr::Count(val)));
} else {
cx.adcx().expected_integer_literal_in_range(l.span(), 0, u32::MAX as isize);
return None;
}
cx.adcx().expected_integer_literal_in_range(l.span(), 0, u32::MAX as isize);
return None;
}

@mejrs mejrs Jul 25, 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.

This seems a little too trivial, can you revert?

remember that there's a cost to making these changes (me reviewing, future people trying to figure out the history of this code etc), so please only do "cleanup" if it's genuinely an improvement.

View changes since the review

@qaijuang qaijuang Jul 25, 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.

Thanks!

This somehow slipped pass my diff check reluctantly, but at least i got my self familiar with rustc_attr_parsing components and things are a bit clearer to me than before(most esp! the Don'ts)

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

rustbot commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @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 25, 2026
@mejrs

mejrs commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Thanks, next time please give your commit messages a more meaningful description though (if you have multiple; I probably wouldn't in this case) 🙂

@bors squash "Various cleanups around rustc_attr_parsing"

@rust-bors

rust-bors Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Unknown argument ""Various". Did you mean to use @bors squash [msg|message="<commit-msg>"|description]? Run @bors help or go to https://bors.rust-lang.org/help to see available commands.

@mejrs

mejrs commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

@bors squash message="Various cleanups around rustc_attr_parsing"

@rust-bors

This comment has been minimized.

Sign in to view
@rust-bors

rust-bors Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

🔨 13 commits were squashed into 2360529.

@rust-bors
rust-bors Bot force-pushed the 159508-follow-up branch from b3bcb5b to 2360529 Compare July 25, 2026 21:41
mejrs
mejrs approved these changes Jul 25, 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 25, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 2360529 has been approved by mejrs

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

Reason for tree closure: spurious failures

@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 25, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 25, 2026
@JonathanBrouwer
Rollup merge of rust-lang#159875 - qaijuang:159508-follow-up, r=mejrs
5076e83 
More cleanup in `rustc_attr_parsing`

Follow up to rust-lang#159508.

r? @mejrs
@JonathanBrouwer JonathanBrouwer mentioned this pull request Jul 25, 2026
Closed
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 26, 2026
@JonathanBrouwer
Rollup merge of rust-lang#159875 - qaijuang:159508-follow-up, r=mejrs
9918b8d 
More cleanup in `rustc_attr_parsing`

Follow up to rust-lang#159508.

r? @mejrs
@JonathanBrouwer JonathanBrouwer mentioned this pull request Jul 26, 2026
Closed
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 26, 2026
@JonathanBrouwer
Rollup merge of rust-lang#159875 - qaijuang:159508-follow-up, r=mejrs
be74601 
More cleanup in `rustc_attr_parsing`

Follow up to rust-lang#159508.

r? @mejrs
@JonathanBrouwer JonathanBrouwer mentioned this pull request Jul 26, 2026
Open
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

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@qaijuang @rustbot @mejrs