-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Uplifts and extends clippy::needless-maybe-sized
into rustc
#145924
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
base: master
Are you sure you want to change the base?
Uplifts and extends clippy::needless-maybe-sized
into rustc
#145924
Conversation
This PR modifies Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
clippy::needless-maybe-sized
into rustc
This comment has been minimized.
This comment has been minimized.
6f856bf
to
fe3f26f
Compare
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.
Reviewed only the Clippy part.
fe3f26f
to
7306887
Compare
Thank you @samueltardieu for your feedback. Regarding the small changes to unrelated files, I believe that was my auto-formatter doing me a disservice, and I believe I have removed said changes from the commits. |
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.
I also believe I have addressed the other points you made, but do let me know if anything is amiss.
I think you still have to remove src/tools/clippy/tests/ui/needless_maybe_sized.rs
and src/tools/clippy/tests/ui/needless_maybe_sized.stderr
.
As far as I can tell those files have already been removed, let me know if I'm missing anything. Thank you |
These have been removed as far as I can tell, they're just showing as moves in Git as similar tests have been added to the rustc ui test suite w/ some changes. |
Oh yes, the subpar GitHub diff UI shows this only at the point of destination, not at the point of origin. |
7306887
to
8446c5a
Compare
I have added a .fixed file to the tests |
@rfcbot fcp merge |
@traviscross proposal cancelled. |
@rfcbot fcp merge This adds the lint fn g<T: Clone + Sized>() {} |
Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
a02ff43
to
00b61bb
Compare
@rfcbot reviewed new name lgtm |
@rfcbot reviewed |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
00b61bb
to
302136b
Compare
This comment was marked as resolved.
This comment was marked as resolved.
- Copies the lint and test (+ stderr) from clippy into rustc_lint/src and tests/ui/lint respectively. - Makes necessary changes to uplifted lint to work correctly in rustc. - Removes needless `?Sized` bounds from 3 files as detected and suggested by the lint.
- Changes name from needless-maybe-sized to redundant-sizedness-bound to reflect extended functionality.
- Extends the test to include tests covering the lint's soon to be extended functionality. - Adds `//@ run-rustfix` to test and creates corresponding .fixed file.
- Extends lint to check for redundant sizedness bounds. - Applies the extended lint's suggestions to `clone.rs` as flagged by the lint.
- Uses clippy dev tool to rename and deprecate needless-maybe-sized in clippy. - Removes needless-maybe-sized from late pass lint register.
302136b
to
66f91da
Compare
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Fixes #140962
Uplift the
clippy::needless_maybe_sized
lint into rustc asredundant_sizedness_bounds
. This is useful for #144404 as it deals with redundant bound that would need to be addressed during migration.redundant_sizedness_bounds
(warn-by-default)
The
needless_maybe_sized
lint checks that?Sized
bounds aren't redundant. This lint is extended to do the equivalent checks for the sizedness traits introduced by #144404 and thus renamed fromneedless_maybe_sized
.Example
Explanation
Relaxing a default
Sized
bound with?Sized
does nothing when there's another bound with aSized
supertrait (Clone
in the example above)@rustbot label: +I-lang-nominated
r? @lcnr
cc @flip1995
For Clippy:
changelog: Moves: Uplifted
clippy::needless_maybe_sized
into rustc asredundant_sizedness_bounds