Extend dead code lint to detect more unused enum variants #21468
Conversation
|
TL;DR: It's a good change and I'd happily r+ it but would like to take this moment to take a step back and reevaluate the story of the dead code lint. Any thoughts on the dead code lint? There is a plenty of opportunity for it to become more sophisticated but do you think that should be happening in-tree or does it qualify as the kind of tooling that would ideally live its own life outside? Hey @sanxiyn! Thank you for your PR! It seems like your solution should work. I am a little wary of it being too specific, though. Whilst you're right it is general in the sense it targets a certain subset of patterns of control flow, that subset is still rather specific. It seems like ideally it would use the control flow graph infrastructure already in place in rustc rather than the AST. That would probably buy us the greater generality for free. Even though I do not see a risk that your change would result in the dead code lint producing any false positives in safe Rust code, I am a bit torn in how much we want to grow the lint in-tree. It seems like over time it's been turning into this rather complicated tool that perhaps should live outside of rustc. At this very moment, this may not necessarily be possible due to how many the internals of rustc are exposed but maybe we should hold off with changes like this until the story is clearer. |
|
I don't think it is too specific. My understanding is that all refutable enum variant patterns are visited by visit_arm, because |
This implements a wish suggested in rust-lang#17410, detecting enum variants that are never constructed, even in the presence of `#[derive(Clone)]`. The implementation is general and not specific to `#[derive(Clone)]`. r? @jakub-
|
@Manishearth Did something go wrong with your rollup? This PR got merged even though it hadn't been r+ed. |
|
Um, looks like I accidentally included it whilst I was adding and removing PRs from an existing rollup branch. Revert at #23357. |
|
(This is what happens when I push a rollup and then go to sleep) |
|
So I reverted it in #23357. You can make this PR again (and hopefully this time it will get reviewed!) now. Note that Manishearth@825f49a is necessary to make this compile on master. Sorry for the trouble! |
This implements a wish suggested in #17410, detecting enum variants that are never constructed, even in the presence of
#[derive(Clone)]. The implementation is general and not specific to#[derive(Clone)].r? @jakub-