-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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 for unused type parameters #26684
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
@@ -63,6 +63,7 @@ | |||
#![feature(no_std)] | |||
#![no_std] | |||
#![allow(raw_pointer_derive)] | |||
#![allow(unused_type_parameters)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems weird that this is necessary? Where are things unused?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One example is core::intrinsics::size_of<T>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, hm. Shouldn't extern fns with type params consider them used? (But, in general, it makes sense that there may be a few things like that.)
It looks like there's a number of legitimate use cases for having "unused" parameters, so can this be an allow-by-default lint instead of warn-by-default? |
The reason this is a lint (and not a hard error, unlike |
I understand my viewpoint on whether lints should be warn by default is subject, by my believe is that any false positive on a lint means that it should not be warn-by-default. I've found that over time projects end up gaining dozens of little |
This is not the first time you argued for allow-by-default. In #10477 you wrote:
But dead_code lint was accepted warn-by-default. I am curious what changed your mind, because like @huonw on that thread, I think this should be warn-by-default. |
I agree! I have historically been very anti-lints as I feel we have far too many already and generate too many warnings for false positives.
Right, but that was almost two years ago. I would want it to be allow-by-default today. |
This is an interesting discussion. I wonder if we should adopt the convention that you can write |
☔ The latest upstream changes (presumably #27818) made this pull request unmergeable. Please resolve the merge conflicts. |
Closing due to inactivity, but feel free to resubmit with a rebase! |
Fix #25871.