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

cast_possible_truncation triggers when the value is guranteed to not be truncated #7486

Open
Luro02 opened this issue Jul 24, 2021 · 4 comments
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

@Luro02
Copy link

Luro02 commented Jul 24, 2021

Lint name: cast_possible_truncation

I tried this code:

#![deny(clippy::cast_possible_truncation)]

fn modulo(number: u64, other: u32) -> u32 {
    // a u64 with guranteed value to be less than or equal to u32::max_value()
    let other_u64 = other as u64;
    // modulo gurantees that the following invariant holds:
    // result < other_u64
    // which implies that result < u32::max_value()
    let result = number % other_u64;
    result as u32
}

fn main() {
    assert_eq!(modulo(11_u64, 5_u32), 1_u32);
}

I expected to see this happen: the lint should not trigger

Instead, this happened: the lint about a possible truncation has been emitted

Meta

  • cargo clippy -V: clippy 0.1.55 (02718709 2021-07-22)
  • rustc -Vv:
rustc 1.55.0-nightly (027187094 2021-07-22)
binary: rustc
commit-hash: 027187094ee05011d6602f5742f550851ccc7fd6
commit-date: 2021-07-22
host: x86_64-pc-windows-msvc
release: 1.55.0-nightly
LLVM version: 12.0.1
@Luro02 Luro02 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 Jul 24, 2021
@dtolnay
Copy link
Member

dtolnay commented Sep 26, 2021

I've hit an even simpler form of this, where the modulus is an integer literal. I believe that Clippy should be able to avoid triggering in the literal case at least.

pub fn f(val: u64) {
    let digit = (val % 62) as u8;
    println!("{}", digit);
}
error: casting `u64` to `u8` may truncate the value
 --> src/main.rs:2:17
  |
2 |     let digit = (val % 62) as u8;
  |                 ^^^^^^^^^^^^^^^^
  |
  = note: `-D clippy::cast-possible-truncation` implied by `-D clippy::pedantic`
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation

@llogiq
Copy link
Contributor

llogiq commented Oct 16, 2021

I have a (possibly partial) solution for this in #7819.

@jyn514
Copy link
Member

jyn514 commented Dec 6, 2023

Hmm, #7819 must not have been complete because this still lints on things like

pub fn foo(i: u16) -> u8 {
    (i % 256) as u8
}

@rdrpenguin04
Copy link

rdrpenguin04 commented Dec 28, 2023

I know why that is. Lemme fork and PR quick.

There's basically a math error going on: n % m has a maximum value of m - 1, not m as the current apply_reductions function calculates.

Edit: Struggling to nail down how the function works and struggling more to make a good test.

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

No branches or pull requests

6 participants
@dtolnay @llogiq @rdrpenguin04 @jyn514 @Luro02 and others