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

clippy::cast_possible_truncation is inconsistent in recognizing where values will never be truncated #12721

Closed
MATSMACKE opened this issue Apr 27, 2024 · 0 comments · Fixed by #12722
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@MATSMACKE
Copy link
Contributor

Summary

In some expressions, the lint recognizes that the u16 that is being casted to u8 will always be a valid u8 but in other expressions it does not. I'm not sure whether this is just a limitation of the algorithm used to check for valid u8 internally or an actual bug, but even if it is a limitation of the algorithm it could be useful to others to expand the capabilities of the lint.

Lint Name

cast_possible_truncation

Reproducer

I tried this code:

self.emit_byte((0x00_ff & val) as u8);

I saw this happen:

warning: casting `u16` to `u8` may truncate the value
   --> codegen/src/lib.rs:277:24
    |
277 |         self.emit_byte((0x00_ff & val) as u8);
    |                        ^^^^^^^^^^^^^^^^^^^^^
    |
    = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation
    = note: `-W clippy::cast-possible-truncation` implied by `-W clippy::pedantic`
    = help: to override `-W clippy::pedantic` add `#[allow(clippy::cast_possible_truncation)]`
help: ... or use `try_from` and handle the error accordingly
    |
277 |         self.emit_byte(u8::try_from(0x00_ff & val));
    |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~

I expected to see this happen:
This clearly will never be an invalid u8, since the & operation with 0x00_ff means only the last 8 bits can be nonzero. Interestingly, a similar case where a value is bitshifted does result in the lint not triggering, so maybe it's possible to also do that for cases like this.

Where lint works correctly:

self.emit_byte(((0xff_00 & val) >> 8) as u8);

Version

rustc 1.77.0 (aedd173a2 2024-03-17)
binary: rustc
commit-hash: aedd173a2c086e558c2b66d3743b344f977621a7
commit-date: 2024-03-17
host: aarch64-apple-darwin
release: 1.77.0
LLVM version: 17.0.6

Additional Labels

No response

@MATSMACKE MATSMACKE added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Apr 27, 2024
MATSMACKE added a commit to MATSMACKE/rust-clippy that referenced this issue Apr 27, 2024
MATSMACKE added a commit to MATSMACKE/rust-clippy that referenced this issue Apr 28, 2024
MATSMACKE added a commit to MATSMACKE/rust-clippy that referenced this issue Apr 28, 2024
Fixed formatting

Added tests for issue rust-lang#12721

Checking for reduction on RHS
MATSMACKE added a commit to MATSMACKE/rust-clippy that referenced this issue Apr 28, 2024
Fixed formatting

Added tests for issue rust-lang#12721

Checking for reduction on RHS
bors added a commit that referenced this issue Apr 28, 2024
Fix false positive in `cast_possible_truncation`

Fixes [#12721](#12721)

changelog: [`cast_possible_truncation`]:  Separated checking whether integer constant has sufficient leading zeroes to be safely casted when getting remainder from bitwise and, since the latter allows a constant on either side of the operator to increase the number of leading zeroes that can be guaranteed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant