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

Add a lint mode for unused unsafe blocks/functions #5797

Closed
wants to merge 6 commits into from

Conversation

alexcrichton
Copy link
Member

Closes #5487, #1913, and #4568

I tracked this by adding all used unsafe blocks/functions to a set on the tcx passed around, and then when the lint pass comes around if an unsafe block/function isn't listed in that set, it's unused.

I also removed everything from the compiler that was unused, and up to stage2 is now compiling without any known unused unsafe blocks.

I chose unused_unsafe as the name of the lint attribute, but there may be a better name...

@Kimundi
Copy link
Member

Kimundi commented Apr 9, 2013

Awesome!

@bstrie
Copy link
Contributor

bstrie commented Apr 9, 2013

I've looked over the five commits that remove the unnecessary unsafe blocks (c541b88 through 41ce4c2) and those look good. As for 74948ef , can you elaborate more on why all this purity stuff is necessary? I was under the impression that purity had been removed from the compiler, but I guess I was mistaken.

@alexcrichton
Copy link
Member Author

I'm not entirely sure why it's all still there, but I was under the impression that the purity-related naming affected more than just functions which used pure. Additionally, I think that all of the remaining "purity errors" are indeed still errors, they just need to be reworded to a more relevant error message.

I may be wrong on that, but I believe that the code should not be removed. I just continued the naming scheme of what was already there.

@alexcrichton
Copy link
Member Author

I just updated the first commit to correctly pass in the item_id when dealing with unsafe blocks. I also updated the test with a case that slipped through (#[allow(unused_unsafe)] on functions wasn't working)

@graydon
Copy link
Contributor

graydon commented Apr 13, 2013

needs a rebase

@alexcrichton
Copy link
Member Author

Just rebased, should be good to go now

bors added a commit that referenced this pull request Apr 15, 2013
Closes #5487, #1913, and #4568

I tracked this by adding all used unsafe blocks/functions to a set on the `tcx` passed around, and then when the lint pass comes around if an unsafe block/function isn't listed in that set, it's unused.

I also removed everything from the compiler that was unused, and up to stage2 is now compiling without any known unused unsafe blocks.

I chose `unused_unsafe` as the name of the lint attribute, but there may be a better name...
@bors bors closed this Apr 15, 2013
bors added a commit that referenced this pull request Apr 22, 2013
Closes #3083.

This takes a similar approach to #5797 where a set is present on the `tcx` of used mutable definitions. Everything is by default warned about, and analyses must explicitly add mutable definitions to this set so they're not warned about.

Most of this was pretty straightforward, although there was one caveat that I ran into when implementing it. Apparently when the old modes are used (or maybe `legacy_modes`, I'm not sure) some different code paths are taken to cause spurious warnings to be issued which shouldn't be issued. I'm not really sure how modes even worked, so I was having a lot of trouble tracking this down. I figured that because they're a legacy thing that I'd just de-mode the compiler so that the warnings wouldn't be a problem anymore (or at least for the compiler).

Other than that, the entire compiler compiles without warnings of unused mutable variables. To prevent bad warnings, #5965 should be landed (which in turn is waiting on #5963) before landing this. I figured I'd stick it out for review anyway though.
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.

7 participants