Skip to content

Visit ImplItem in dead_code lint - #59486

Merged
bors merged 3 commits into
rust-lang:masterfrom
varkor:dead-code-impl
Mar 29, 2019
Merged

Visit ImplItem in dead_code lint#59486
bors merged 3 commits into
rust-lang:masterfrom
varkor:dead-code-impl

Conversation

@varkor

@varkorvarkor commented Mar 28, 2019

Copy link
Copy Markdown
Contributor

Fixes#47131.

@rust-highfive

rust-highfive commented Mar 28, 2019

Copy link
Copy Markdown
Contributor

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 28, 2019
@Centril

Centril commented Mar 28, 2019

Copy link
Copy Markdown
Contributor

@bors rollup

Centril
Centril reviewed Mar 28, 2019
View reviewed changes
Comment threadsrc/librustc/middle/dead.rs
| hir::ItemKind::Impl(..)
| hir::ItemKind::Const(..) => {
intravisit::walk_item(self, &item);
}

@CentrilCentrilMar 28, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove _ => () and replace with "exhaustive" match to avoid future problems?

@petrochenkovpetrochenkovMar 28, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not actually sure why this is not

_ => {
intravisit::walk_item(self,&item);}

If that doesn't break anything, then walk_item can be moved out of the match.

@petrochenkovpetrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 28, 2019
@varkor

varkor commented Mar 28, 2019

Copy link
Copy Markdown
ContributorAuthor

I've made the dead_code lint walk all items by default. I had to exclude ItemKind::ForeignMod, which caused a false negative and explicitly ignore a few Defs, which were now visited, but otherwise there weren't any problems. This should lower the chance an item is accidentally missed now.

@varkorvarkor mentioned this pull request Mar 29, 2019
Closed
@sanxiyn

sanxiyn commented Mar 29, 2019

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Mar 29, 2019

Copy link
Copy Markdown
Collaborator

📌 Commit 8cdfad9 has been approved by sanxiyn

@borsbors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 29, 2019
Centril added a commit to Centril/rust that referenced this pull request Mar 29, 2019
@Centril
Rollup merge ofrust-lang#59486- varkor:dead-code-impl, r=sanxiyn
4aacc49
Visit `ImplItem` in `dead_code` lint
Fixesrust-lang#47131.
@CentrilCentril mentioned this pull request Mar 29, 2019
Merged
bors added a commit that referenced this pull request Mar 29, 2019
@bors
Auto merge of#59522- Centril:rollup, r=Centril
d920dfc
Rollup of 9 pull requests
Successful merges:
- #59366 (Update books)
- #59436 (Update jemalloc-sys to version 0.3.0)
- #59454 (Update rustfmt to 1.2.0)
- #59462 (Fix error in Rust 2018 + no_core environment)
- #59467 (Better diagnostic for binary operation on BoxedValues)
- #59473 (Do not emit incorrect borrow suggestion involving macros and fix overlapping multiline spans)
- #59480 (Update stdsimd)
- #59486 (Visit `ImplItem` in `dead_code` lint)
- #59510 (Rename `type_parameters` to `generics` and so on)
Failed merges:
- #59516 (Update cargo)
r? @ghost
bors added a commit that referenced this pull request Mar 29, 2019
@bors
Auto merge of#59522- Centril:rollup, r=Centril
e782d79
Rollup of 9 pull requests
Successful merges:
- #59366 (Update books)
- #59436 (Update jemalloc-sys to version 0.3.0)
- #59454 (Update rustfmt to 1.2.0)
- #59462 (Fix error in Rust 2018 + no_core environment)
- #59467 (Better diagnostic for binary operation on BoxedValues)
- #59473 (Do not emit incorrect borrow suggestion involving macros and fix overlapping multiline spans)
- #59480 (Update stdsimd)
- #59486 (Visit `ImplItem` in `dead_code` lint)
- #59510 (Rename `type_parameters` to `generics` and so on)
Failed merges:
- #59516 (Update cargo)
r? @ghost
@bors

bors commented Mar 29, 2019

Copy link
Copy Markdown
Collaborator

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

@borsbors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 29, 2019
@bors
bors merged commit 8cdfad9 into rust-lang:masterMar 29, 2019
@varkor
varkor deleted the dead-code-impl branch March 29, 2019 23:17
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petrochenkovpetrochenkovpetrochenkov left review comments
+1 more reviewer
@CentrilCentrilCentril left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@sanxiynsanxiyn

Labels

S-waiting-on-authorStatus: This is awaiting some action (such as code changes or more information) from the author.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@varkor@rust-highfive@Centril@sanxiyn@bors@petrochenkov