lint against repeated repr attributes

[ettolrach]

View all comments

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

r? scottmcm

[scottmcm]

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

[traviscross]

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

[rustbot]

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

cc @jdonszelmann, @JonathanBrouwer

[jdonszelmann]

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

[jdonszelmann]

Like this it looks good!

@bors r+ rollup

[rust-bors]

📋 This PR cannot be approved because it currently has the following label: needs-fcp.

[jdonszelmann]

welp, fair enough @rust-lang/lang feel free to do an FCP on this one, r=me after that

[rustbot]

This PR was rebased onto a different main 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.

[ettolrach]

@jdonszelmann you've still got requested changes here (just in the UI, i did do the thing you wanted me to). it also doesn't look like this ping worked, so i guess i'll try it myself? though i don't think it'll go through. @rust-lang/lang

[scottmcm]

(Nominating to look at in triage tomorrow; sorry for the delay)

This adds a new repeated_reprs lint as warn-by-default that triggers on things like

#[repr(transparent)]
#[repr(transparent)]

Hopefully this is an easy accept because we'd talked about doing this in a previous meeting 🙂