Skip to content

Improvements to the dead code lint #17410

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

Merged
merged 4 commits into from Sep 24, 2014
Merged

Improvements to the dead code lint #17410

merged 4 commits into from Sep 24, 2014

Conversation

ghost
Copy link

@ghost ghost commented Sep 20, 2014

No description provided.

@ghost
Copy link
Author

ghost commented Sep 20, 2014

The main change here is that rustc will now warn about variants that are never constructed. I concluded this would be a worthwhile lint after finding that

cat_discr(cmt, ast::NodeId), // match discriminant (see preserve())
is never constructed, yet there is plenty of code throughout rustc that deals with it. Which could be a bug but I haven't investigated yet.

Funnily enough, with this change rustc still doesn't warn about that particular case due to the Clone trait being derived on the categorization enum. I think it's worth investigating potential ways of allowing the dead lint to be useful even in the presence of traits like Clone being derived but it's not included in this PR.

In addition, I changed the messages to be more descriptive with regards to the particular nodes that are proven unused.

Last, but not least, I also investigated how much dead code there is in rustc after disabling re-exports. Turns out, quite a lot. I carefully removed what I thought was truly dead code but I still question if it's something we should do.

ExponentFormat, ExpNone, ExpDec, ExpBin,
SignFormat, SignNone, SignNeg, SignAll,
SignificantDigits, DigAll, DigMax, DigExact
};
Copy link
Member

Choose a reason for hiding this comment

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

This module is still currently one I'd like to leave as a private implementation detail as part of libcore, was there a reason for exporting these variants?

I do feel bad for duplicating the parsing between std/core!

Copy link
Author

Choose a reason for hiding this comment

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

Ah, I just noticed the duplication between libcore and libstd.

That is fair enough. Not all of the variants in libcore are actually being used so I can remove the unused ones or annotate them with #[allow(dead_code)].

Copy link
Author

Choose a reason for hiding this comment

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

@alexcrichton I brought back the libcore versions of the types without the unused variants, if that's OK.

Copy link
Member

Choose a reason for hiding this comment

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

Awesome, thanks!

@ghost
Copy link
Author

ghost commented Sep 23, 2014

@alexcrichton Fixed the Windows issue. r?

@bors bors closed this Sep 24, 2014
@bors bors merged commit fd52224 into rust-lang:master Sep 24, 2014
@ghost ghost deleted the dead-code branch October 2, 2014 20:24
Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 13, 2015
 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-
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants