Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotate dead code lint with notes about ignored derived impls #92783

Merged
merged 2 commits into from
Jan 19, 2022

Conversation

FabianWolff
Copy link
Contributor

Fixes #92726. CC @pmetzger, is this what you had in mind?

r? @nikomatsakis

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 11, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 11, 2022
@camelid camelid added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Jan 11, 2022
@pmetzger
Copy link

Yes, this is more or less what I had in mind! The wording might be tuned very slightly. Right now it seems to say things like

Foo has a derived impl for the trait 'Debug', but this is ignored during dead code analysis

It might be slightly more informative to say something to the effect of:

Foo does have a derived impl for the trait 'Debug', but this is intentionally ignored for purposes of dead code analysis

My suggestion feels like it might be too verbose, but my thought is to make it clear "yes, we know you have a derived impl of Bar, but we have good reasons not to count that when we're figuring out what's dead." Some wordsmithing help would be good here. I bet some small change will make it much more obvious to the reader.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Looks nice! I'm wondering if we can improve the output by citing the debug impl itself, perhaps with a message like

= note: this Debug impl exists, but Debug impls are intentionally ignored by the dead code analysis

compiler/rustc_passes/src/dead.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

this looks nice :)

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 18, 2022

📌 Commit 8b459dd has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 18, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 19, 2022
…askrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#88642 (Formally implement let chains)
 - rust-lang#89621 (doc: guarantee call order for sort_by_cached_key)
 - rust-lang#91278 (Use iterator instead of recursion in `codegen_place`)
 - rust-lang#92124 (Little improves in CString `new` when creating from slice)
 - rust-lang#92783 (Annotate dead code lint with notes about ignored derived impls)
 - rust-lang#92797 (Remove horizontal lines at top of page)
 - rust-lang#92920 (Move expr- and item-related pretty printing functions to modules)
 - rust-lang#93041 (Remove some unused ordering derivations based on `DefId`)
 - rust-lang#93051 (Add Option::is_some_with and Result::is_{ok,err}_with)
 - rust-lang#93062 (Update books)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9a82f74 into rust-lang:master Jan 19, 2022
@rustbot rustbot added this to the 1.60.0 milestone Jan 19, 2022
@pmetzger
Copy link

Hooray!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Annotate dead-code lint with a note that Debug impls are ignored
7 participants