Skip to content

Conversation

mu001999
Copy link
Contributor

@mu001999 mu001999 commented Jul 18, 2025

https://github.com/rust-lang/rust/blob/master/compiler/rustc_passes/src/dead.rs#L360-L361 won't insert assoc items into the live set, so that impl items cannot be marked live.

This PR lets impls and impl items can inherit lint levels of the corresponding traits and trait items.

Fixes #144060

r? @petrochenkov

@rustbot rustbot added 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 18, 2025
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

This behavior seems really non-local and pretty broad. #[allow(dead_code)] on a trait definition suppresses all dead code warnings in all impls with this change?

@mu001999 mu001999 force-pushed the dead-code/allow-trait branch from 7eba1f1 to 7a9e2e2 Compare July 19, 2025 01:34
@mu001999
Copy link
Contributor Author

#[allow(dead_code)] on a trait definition suppresses all dead code warnings in all impls with this change?

@compiler-errors yes, and I think this makes sense. We would have only one trait definition but many impls, letting #[allow(dead_code)] on traits propagate to impls is convenient. Otherwise we need to add #[allow(dead_code)] also on each impls which use unused items.

@petrochenkov
Copy link
Contributor

r? @compiler-errors

@petrochenkov
Copy link
Contributor

Hmm, why did rustbot assign it to me again?
I'll just block this on #144863 now.
@rustbot blocked

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 4, 2025
@mu001999
Copy link
Contributor Author

mu001999 commented Aug 4, 2025

Hmm, why did rustbot assign it to me again?

I tried reassigning, but rustbot didn’t respond after waiting a while, so I deleted the comment. 😂

@cjgillot cjgillot self-assigned this Aug 4, 2025
@petrochenkov petrochenkov removed their assignment Aug 5, 2025
@bors
Copy link
Collaborator

bors commented Aug 5, 2025

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

@mu001999 mu001999 force-pushed the dead-code/allow-trait branch from 7a9e2e2 to c1be949 Compare August 7, 2025 16:34
@mu001999
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 10, 2025
@tgross35
Copy link
Contributor

@cjgillot gentle ping for a review here

@wesleywiser wesleywiser added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Sep 25, 2025
@wesleywiser
Copy link
Member

Nominated the issue for lang team discussion, let's see what they think before merging.

@mu001999
Copy link
Contributor Author

mu001999 commented Sep 25, 2025

oh, I missed the non-local in the note from @compiler-errors

and if the trait is non-local, the dead-code lint level won't spread to the local impls. cc @wesleywiser

we only care about local traits, see https://github.com/rust-lang/rust/pull/144113/files#diff-719b4f36bbbf3b3bf48683429408f5d27e5633f70c1144a93554bd536d5bf655R786

@traviscross traviscross added T-lang Relevant to the language team I-lang-nominated Nominated for discussion during a lang team meeting. labels Oct 8, 2025
@traviscross traviscross added the I-lang-radar Items that are on lang's radar and will need eventual work or consideration. label Oct 8, 2025
@joshtriplett
Copy link
Member

Temporarily removing T-compiler so we can do a lang FCP on this. (Note that this is fixing something perceived as a stable-to-stable regression.)

@joshtriplett joshtriplett removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 8, 2025
@joshtriplett
Copy link
Member

@rfcbot merge

@rust-rfcbot
Copy link
Collaborator

rust-rfcbot commented Oct 8, 2025

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 8, 2025
@joshtriplett joshtriplett added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 8, 2025
@traviscross traviscross added P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. labels Oct 8, 2025
@Nadrieril Nadrieril changed the title Impls and impl items inherit lint levels of the corresponding traits and trait items Impls and impl items inherit dead_code lint level of the corresponding traits and trait items Oct 8, 2025
@tmandry
Copy link
Member

tmandry commented Oct 8, 2025

@rfcbot reviewed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. I-lang-nominated Nominated for discussion during a lang team meeting. 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. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-blocked Status: Blocked on something else such as an RFC or other implementation work. 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. T-lang Relevant to the language team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lint regression: dead_code ignores #[allow(dead_code)] on traits