Skip to content

lint against repeated repr attributes#157036

Open
ettolrach wants to merge 10 commits into
rust-lang:mainfrom
ettolrach:disallow-repeated-reprs
Open

lint against repeated repr attributes#157036
ettolrach wants to merge 10 commits into
rust-lang:mainfrom
ettolrach:disallow-repeated-reprs

Conversation

@ettolrach

@ettolrach ettolrach commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown

fixes #156029 by adding a lint against repeated repr attributes.

r? scottmcm

@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 May 27, 2026
This was referenced May 27, 2026
Closed
Open
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@traviscross traviscross added T-lang Relevant to the language team needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. labels May 27, 2026
@theemathas theemathas mentioned this pull request May 30, 2026
Merged
@scottmcm

scottmcm commented May 30, 2026

Copy link
Copy Markdown
Member

Wearing my lang hat but not speaking as team consensus (since we've not talked about it):

I think you should start with the "obviously good" version, and we can do future PRs later to expand it or to update severity. Concretely, thus, I'd say it should be a warning (because if it's inert it doesn't need to be deny) when you have a clearly-unnecessary one that wasn't emitted from a macro, or something like that.

(Our bar in the compiler for deny-by-default is pretty high. My personal heuristic is whether it's worth running your unit tests despite the warning, and an inert extra attribute doesn't need to block that.)

We can then look at impacts and ratchet it up over time or over an edition, but those decisions can be made later and don't need to block you making a useful lint in the meantime.

I'm a bad reviewer for lint stuff code-wise, though, so let's try
r? JonathanBrouwer

@rustbot rustbot assigned JonathanBrouwer and unassigned scottmcm May 30, 2026
@traviscross

traviscross commented May 30, 2026

Copy link
Copy Markdown
Contributor

+1 to what @scottmcm said on how best to approach this.

@rust-bors

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@ettolrach ettolrach marked this pull request as ready for review June 6, 2026 17:03
@rustbot

rustbot commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

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

cc @jdonszelmann, @JonathanBrouwer

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

This comment has been minimized.

Sign in to view
@rustbot rustbot added the has-merge-commits PR has merge commits, merge with caution. label Jun 6, 2026
ettolrach added 5 commits June 6, 2026 18:24
@ettolrach
lint against repeated repr attributes
d71bc52
@ettolrach
fmt
25f3f79
@ettolrach
bring back warns for repeated reprs, update tests
bcf4fc3 
had to update several tests which used repeated aligns and packeds. they
now produce both a warning and an error where appropriate (e.g. when
using conflicting packeds) which i think is correct.
@ettolrach
typo
2fe1d6d
@ettolrach
another typo
6666481
@ettolrach ettolrach force-pushed the disallow-repeated-reprs branch from ae3cdee to 314903c Compare June 6, 2026 17:28
@rustbot rustbot removed the has-merge-commits PR has merge commits, merge with caution. label Jun 6, 2026
@jdonszelmann

jdonszelmann commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

r? jdonszelmann
I don't mind giving this one a look :)

jdonszelmann
jdonszelmann reviewed Jun 11, 2026
View reviewed changes
Comment thread compiler/rustc_passes/src/check_attr.rs
for (repr, _repr_span) in reprs {
match repr {
ReprAttr::ReprRust => {
if is_explicit_rust {

@jdonszelmann jdonszelmann Jun 11, 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 very manual, and I'm not sure I like it. The logic gets quite complex. Is there any way you can figure out to do this in a more structured way that makes sure we can't make any logic errors? Something with iterators maybe?

View changes since the review

@jdonszelmann

jdonszelmann commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@rustbot author

jdonszelmann
jdonszelmann requested changes Jun 11, 2026
View reviewed changes
Comment thread compiler/rustc_passes/src/errors.rs
}

#[derive(Diagnostic)]
#[diag("representation attribute is specified more than once")]

@jdonszelmann jdonszelmann Jun 11, 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'd phrase the error as "#[repr(...)] attribute is specified more than once"

View changes since the 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 Jun 11, 2026
@rustbot

rustbot commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

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

@rustbot

rustbot commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Error: shortcut handler unexpectedly failed in this comment: failed to remove Label { name: "S-waiting-on-review" }

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdonszelmann jdonszelmann jdonszelmann requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@jdonszelmann jdonszelmann

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) I-lang-radar Items that are on lang's radar and will need eventual work or consideration. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. 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. T-lang Relevant to the language team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

#[repr()] attribute has inconsistent handling of duplicates.

7 participants

@ettolrach @rust-log-analyzer @scottmcm @traviscross @rustbot @jdonszelmann @JonathanBrouwer