Impls and impl items inherit dead_code lint level of the corresponding traits and trait items by mu001999 · Pull Request #144113 · rust-lang/rust (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
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?
#[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.
Hmm, why did rustbot assign it to me again?
I'll just block this on #144863 now.
@rustbot 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
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. 😂
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
Status: Blocked on something else such as an RFC or other implementation work.
label
Nominated the issue for lang team discussion, let's see what they think before merging.
Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang
This change is insta-stable, or significant enough to need a team FCP to proceed.
labels
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
How does what's proposed in this PR compare with @nikomatsakis' model in #144060 (comment)?
For examples given by @nikomatsakis:
pub trait Foo { } impl Foo for u32 { }
struct PrivateType; impl Foo for PrivateType { } // <-- warns as dead, even though Foo is public
struct AnotherPrivateType; impl Foo for AnotherPrivateType; { } // <-- warns as dead, even though Foo is public
works like before.
#[allow(dead_code)] pub trait Foo { } impl Foo for u32 { }
struct PrivateType; impl Foo for PrivateType { } // <-- no warning, trait is "allowed"
struct AnotherPrivateType; impl Foo for AnotherPrivateType; { } // <-- no warning, trait is "allowed"
This one is related to this PR. And it will work as expected after this PR.
pub trait Foo { } impl Foo for u32 { }
struct PrivateType; #[allow(dead_code)] impl Foo for PrivateType { } // <-- no warning, impl is allowed
struct AnotherPrivateType; impl Foo for AnotherPrivateType; { } // <-- warns as dead, even though Foo is public
works like before.
🔔 This is now entering its final comment period, as per the review above. 🔔
Nominated for discussion during a lang team meeting.
Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang
labels
The final comment period, with a disposition to merge, as per the review above, is now complete.
As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.
This will be merged soon.
@wesleywiser I think the lable S-blocked can be removed now, right?
Status: Blocked on something else such as an RFC or other implementation work.
label
@wesleywiser I think the lable
S-blockedcan be removed now, right?And friendly ping @cjgillot
Fyi you can use @rustbot label -S-blocked to adjust labels yourself. @rustbot reroll allows you to ask for a new reviewer given it's been a while (but give Camille another week or two since they self-assigned and I just adjusted the labels now.)
Fyi you can use @rustbot label -S-blocked to adjust labels yourself.
thanks, glad to know that ;)
The idea sounds good to me (based on the discussions), but I know nothing of this code.
@rustbot reroll
Labels
This issue / PR is in PFCP or FCP with a disposition to merge it.
The final comment period is finished for this PR / Issue.
Items that are on lang's radar and will need eventual work or consideration.
This change is insta-stable, or significant enough to need a team FCP to proceed.
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
Relevant to the language team
Announce this issue on triage meeting