Skip to content

remove superseded lints #14703

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bend-n
Copy link
Contributor

@bend-n bend-n commented Apr 28, 2025

changelog: [transmute_float_to_int, transmute_int_to_char, transmute_int_to_float]: remove lints, now in rustc

these lints are now mostly in rustc, so they dont need to be in clippy anymore

rust-lang/rust#136083 (comment)

pending rust-lang/rust#140431: transmute_int_to_bool

Summary Notes

Generated by triagebot, see help for how to add more

@rustbot
Copy link
Collaborator

rustbot commented Apr 28, 2025

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 28, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 28, 2025

Failed to set assignee to lcnr: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@bend-n bend-n force-pushed the unnecessary_transmutes branch from aa7385a to 483ce48 Compare April 28, 2025 16:03
@Jarcho
Copy link
Contributor

Jarcho commented Apr 28, 2025

When uplifting lints please run cargo dev rename_lint <lint_name> <uplifted_name> --uplift. This will add all the metadata related to lint renames.

@bend-n bend-n force-pushed the unnecessary_transmutes branch from 42cf9e2 to 57d6333 Compare April 28, 2025 16:46
@rustbot

This comment has been minimized.

@bend-n bend-n force-pushed the unnecessary_transmutes branch 3 times, most recently from 330e23d to 177dc67 Compare May 1, 2025 13:42
@bend-n bend-n marked this pull request as draft May 1, 2025 13:43
@bend-n bend-n force-pushed the unnecessary_transmutes branch 3 times, most recently from 6ed70fc to 6f5bf0f Compare May 1, 2025 18:30
@samueltardieu
Copy link
Contributor

@bend-n Anything I can help with in order to merge this (I noticed it's still in draft)? Could you run cargo dev fmt and squash your commits into one?

r? @samueltardieu

@rustbot rustbot assigned samueltardieu and unassigned llogiq May 7, 2025
@bend-n
Copy link
Contributor Author

bend-n commented May 8, 2025

@samueltardieu
this PR is pending rust-lang/rust#140431 with regards to keeping transmute_int_to_bool. if that PR is closed, ill remove transmute_int_to_bool here.

@samueltardieu
Copy link
Contributor

Any reason to want to put this into the same PR, while there are two PRs on the compiler side? If your second compiler PR is not merged before tomorrow, then it won't be in Rust 1.88 while your first compiler PR would be. It means that, in Clippy, we would have to backport only the deprecation of transmute_float_to_int, transmute_int_to_char, and transmute_int_to_float, while the deprecation of transmute_int_to_bool would stay in nightly as the Rust side would land only in Rust 1.89.

This is why having two PRs, to match the compiler changes, might make things easier to keep Clippy lints deprecation in sync.

@bend-n
Copy link
Contributor Author

bend-n commented May 8, 2025

well if this gets merged today then transmute_int_to_bool will be in both clippy and rustc in 1.88, is that what we want?

@samueltardieu
Copy link
Contributor

Yes. But if it is not, please do not add the transmute_int_to_bool deprecation to this PR but open another one.

Marking this PR as blocked in the meantime and adding a note for myself when it comes to reviewing.

@rustbot note "Rust version of new lints should be checked"
@rustbot blocked

@rustbot rustbot added S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels May 8, 2025
@bend-n bend-n force-pushed the unnecessary_transmutes branch from 6f5bf0f to f51c29b Compare May 8, 2025 06:58
@bend-n bend-n marked this pull request as ready for review May 8, 2025 06:58
Copy link
Contributor

@samueltardieu samueltardieu left a comment

Choose a reason for hiding this comment

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

It looks like the unnecessary_transmutes compiler builtin lint does not lint for signed integer to float and vice-versa, while this was linted by Clippy. Removing the lints will lose functionality. Maybe the lints should be modified instead of being uplifted to handle only those cases involving signed integers.

The same thing is true for transmuting a i32 to a char: there is no compiler warning there, so the corresponding lint should probably be kept in Clippy for this case.

Also, it looks like this patch removes the code for the transmute_num_to_bytes lint, as this is also linted by the compiler, but does not remove it properly nor does it talk about it in the PR changelog. It looks like this is the only Clippy lint which is completely covered by the unnecessary_transmutes compiler lint, so probably the only one which should be uplifted.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants