rust-clippy
rust-clippy copied to clipboard
Resolve duplicate diag message for: tests/ui/renamed_builtin_attr.rs
This PR is to resolve duplicate diag messages for the renamed_builtin_attr UI test which is related to: #12379
This was caused by multiple functions calling the get_attr function resulting in the deprecated attribute to be reevaluated.
I added a state to the attr.rs file to track if the warning has already been emitted and skip the evaluation if the attribute warning has already been emitted.
changelog: [renamed_builtin_attr]: Fix duplicate diagnostics
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) some time within the next two weeks.
Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:
@rustbot author: the review is finished, PR author should check the comments and take action accordingly@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue
This one is a bit of an edge case compared to the others since it's not being caused by a single lint. Since the deduplication here is similar to what the diagnostic infrastructure is doing internally it might be worth keeping this one as a duplicate emitter
r? @Alexendoo
This one is a bit of an edge case compared to the others since it's not being caused by a single lint. Since the deduplication here is similar to what the diagnostic infrastructure is doing internally it might be worth keeping this one as a duplicate emitter
Hi @Alexendoo. The only thing worth mentioning is that currently, without this fix, each deprecated attribute will be evaluated 4x which means 4 messages per attribute will be emitted. With this fix it will only be emitted once. More of a cosmetic thing but wanted to mention just in case.
:umbrella: The latest upstream changes (presumably #12999) made this pull request unmergeable. Please resolve the merge conflicts.
UI wise duplicates are hidden by default, we set -Zdeduplicate-diagnostics=no to show duplicates as it may indicate an issue. You're right though that it's still being evaluated repeatedly which is wasteful, we could move the check into e.g. https://github.com/rust-lang/rust-clippy/blob/68a799aea9b65e2444fbecfe32217ce7d5a3604f/clippy_lints/src/attrs/mod.rs#L532
so it's only happening once per attribute
@rustbot author
:umbrella: The latest upstream changes (possibly d28d2344d000aa96bef729cf408731f952f71fb0) made this pull request unmergeable. Please resolve the merge conflicts.
@grtn316 Hi, this is a ping from triage, would you mind resolving the merge conflicts and take look of the last comment by 'Alexendoo'?
Closing this since there's been no response. @grtn316 if you wish to come back to this feel free to reopen this or create a new PR.