Skip to content

improve diagnostics for empty attributes#146653

Merged
bors merged 1 commit intorust-lang:masterfrom
jdonszelmann:empty-attr-diags
Sep 29, 2025
Merged

improve diagnostics for empty attributes#146653
bors merged 1 commit intorust-lang:masterfrom
jdonszelmann:empty-attr-diags

Conversation

@jdonszelmann
Copy link
Contributor

@jdonszelmann jdonszelmann commented Sep 16, 2025 •
edited by rustbot
Loading

Adds a note about them not having any effect. This was previously done for feature attributes but no other attributes. In converting the feature parser I removed that note. This PR adds it back in and makes it so all attributes benefit from it.

Not blocked on #146652, either can merge first

@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 Sep 16, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 16, 2025

r? @SparrowLii

rustbot has assigned @SparrowLii.
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

@jdonszelmann jdonszelmann mentioned this pull request Sep 16, 2025
Open
@jdonszelmann
Copy link
Contributor Author

jdonszelmann commented Sep 18, 2025

r? compiler

@rustbot rustbot assigned BoxyUwU and unassigned SparrowLii Sep 18, 2025
@bors
Copy link
Collaborator

bors commented Sep 22, 2025

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

@jdonszelmann
Copy link
Contributor Author

jdonszelmann commented Sep 22, 2025

r? nnethercote

@rustbot rustbot assigned nnethercote and unassigned BoxyUwU Sep 22, 2025
nnethercote
nnethercote reviewed Sep 22, 2025
View reviewed changes
Copy link
Contributor

@nnethercote nnethercote left a comment •
edited
Loading

Choose a reason for hiding this comment

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

To be honest, I don't think this is an improvement. As explained below, in every case the note feels not quite right. The motivation was to restore the old behaviour for feature but I don't think it is necessary.

View changes since this review

tests/ui/macros/macro-use-all-and-none.stderr Outdated
LL | #[macro_use()]
| ^^ help: remove this attribute
|
= note: attribute `macro_use` with an empty list has no effect
Copy link
Contributor

@nnethercote nnethercote Sep 22, 2025

Choose a reason for hiding this comment

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

I'm a bit surprised this is only a warning. But also, the note feels misleading: I assume macro_use() with an empty list does have an effect, i.e. the same effect as just macro_use.

A better improvement for this error message would be to say "remove these parentheses" instead of "remove this attribute".

Copy link
Contributor Author

@jdonszelmann jdonszelmann Sep 27, 2025

Choose a reason for hiding this comment

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

I pushed a fix that makes this error better specifically when the no-argument version is available and so valid

@rustbot
Copy link
Collaborator

rustbot commented Sep 27, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

jdonszelmann
jdonszelmann commented Sep 27, 2025
View reviewed changes
Copy link
Contributor Author

@jdonszelmann jdonszelmann left a comment •
edited
Loading

Choose a reason for hiding this comment

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

github is borken (this comment was made as a dummy to get around a UI bug)

View changes since this review

@jdonszelmann
Copy link
Contributor Author

jdonszelmann commented Sep 27, 2025

@rustbot review

@jdonszelmann
Copy link
Contributor Author

jdonszelmann commented Sep 29, 2025

(@nnethercote)

@nnethercote
Copy link
Contributor

nnethercote commented Sep 29, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 29, 2025

📌 Commit b3631e1 has been approved by nnethercote

It is now in the queue for this repository.

@bors bors 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 Sep 29, 2025
@Zalathar Zalathar mentioned this pull request Sep 29, 2025
Merged
bors added a commit that referenced this pull request Sep 29, 2025
@bors
Auto merge of #147151 - Zalathar:rollup-w81rn0j, r=Zalathar
21a13b8 
Rollup of 5 pull requests

Successful merges:

 - #146653 (improve diagnostics for empty attributes)
 - #146987 (impl Ord for params and use unstable sort)
 - #147101 (Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library )
 - #147123 (Fix removed version numbers of `doc_auto_cfg` and `doc_cfg_hide`)
 - #147149 (add joboet to library review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cf07cce into rust-lang:master Sep 29, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 29, 2025
rust-timer added a commit that referenced this pull request Sep 29, 2025
@rust-timer
Unrolled build for #146653
1d14c61 
Rollup merge of #146653 - jdonszelmann:empty-attr-diags, r=nnethercote

improve diagnostics for empty attributes

Adds a note about them not having any effect. This was previously done for `feature` attributes but no other attributes. In [converting the `feature` parser](#146652) I removed that note. This PR adds it back in and makes it so all attributes benefit from it.

Not blocked on #146652, either can merge first
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Sep 30, 2025
@bors
Auto merge of #147151 - Zalathar:rollup-w81rn0j, r=Zalathar
8422b2c 
Rollup of 5 pull requests

Successful merges:

 - rust-lang/rust#146653 (improve diagnostics for empty attributes)
 - rust-lang/rust#146987 (impl Ord for params and use unstable sort)
 - rust-lang/rust#147101 (Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library )
 - rust-lang/rust#147123 (Fix removed version numbers of `doc_auto_cfg` and `doc_cfg_hide`)
 - rust-lang/rust#147149 (add joboet to library review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
github-actions bot pushed a commit to rust-lang/rust-analyzer that referenced this pull request Oct 2, 2025
@bors
Auto merge of #147151 - Zalathar:rollup-w81rn0j, r=Zalathar
84e54dd 
Rollup of 5 pull requests

Successful merges:

 - rust-lang/rust#146653 (improve diagnostics for empty attributes)
 - rust-lang/rust#146987 (impl Ord for params and use unstable sort)
 - rust-lang/rust#147101 (Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library )
 - rust-lang/rust#147123 (Fix removed version numbers of `doc_auto_cfg` and `doc_cfg_hide`)
 - rust-lang/rust#147149 (add joboet to library review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Oct 18, 2025
@bors
Auto merge of #147151 - Zalathar:rollup-w81rn0j, r=Zalathar
62601a8 
Rollup of 5 pull requests

Successful merges:

 - rust-lang/rust#146653 (improve diagnostics for empty attributes)
 - rust-lang/rust#146987 (impl Ord for params and use unstable sort)
 - rust-lang/rust#147101 (Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library )
 - rust-lang/rust#147123 (Fix removed version numbers of `doc_auto_cfg` and `doc_cfg_hide`)
 - rust-lang/rust#147149 (add joboet to library review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
Kobzol pushed a commit to Kobzol/rustc_codegen_gcc that referenced this pull request Dec 21, 2025
@bors
Auto merge of #147151 - Zalathar:rollup-w81rn0j, r=Zalathar
41f3a8e 
Rollup of 5 pull requests

Successful merges:

 - rust-lang/rust#146653 (improve diagnostics for empty attributes)
 - rust-lang/rust#146987 (impl Ord for params and use unstable sort)
 - rust-lang/rust#147101 (Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library )
 - rust-lang/rust#147123 (Fix removed version numbers of `doc_auto_cfg` and `doc_cfg_hide`)
 - rust-lang/rust#147149 (add joboet to library review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
Kobzol pushed a commit to Kobzol/rustc_codegen_cranelift that referenced this pull request Dec 29, 2025
@bors
Auto merge of #147151 - Zalathar:rollup-w81rn0j, r=Zalathar
a063d95 
Rollup of 5 pull requests

Successful merges:

 - rust-lang/rust#146653 (improve diagnostics for empty attributes)
 - rust-lang/rust#146987 (impl Ord for params and use unstable sort)
 - rust-lang/rust#147101 (Use `Iterator::eq` and (dogfood) `eq_by` in compiler and library )
 - rust-lang/rust#147123 (Fix removed version numbers of `doc_auto_cfg` and `doc_cfg_hide`)
 - rust-lang/rust#147149 (add joboet to library review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nnethercote nnethercote nnethercote approved these changes

Assignees

@nnethercote nnethercote

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.92.0

Development

Successfully merging this pull request may close these issues.

6 participants

@jdonszelmann @rustbot @bors @nnethercote @BoxyUwU @SparrowLii