Skip to content

unnecessary_cast on as u64 for a c_ulong #10555

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
joshtriplett opened this issue Mar 27, 2023 · 5 comments · Fixed by #10927
Open

unnecessary_cast on as u64 for a c_ulong #10555

joshtriplett opened this issue Mar 27, 2023 · 5 comments · Fixed by #10927
Labels
C-bug Category: Clippy is not doing the correct thing E-hard Call for participation: This a hard problem and requires more experience or effort to work on I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@joshtriplett
Copy link
Member

Summary

unnecessary_cast triggers on casts from C FFI type aliases, even though the type aliases may not always match the type being cast to.

Lint Name

unnecessary_cast

Reproducer

I tried this code:

use core::ffi::c_ulong;

fn main() {
    let x: c_ulong = 0;
    let y = x as u64;
    println!("Hello, {y}!");
}

I saw this happen:

warning: casting to the same type is unnecessary (`u64` -> `u64`)
 --> src/main.rs:5:13
  |
5 |     let y = x as u64;
  |             ^^^^^^^^ help: try: `x`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
  = note: `#[warn(clippy::unnecessary_cast)]` on by default

I expected to not get an error, because c_ulong is a type alias that isn't always u64.

I thought #8596 was supposed to fix this.

Version

rustc 1.68.1 (8460ca823 2023-03-20)
binary: rustc
commit-hash: 8460ca823e8367a30dda430efda790588b8c84d3
commit-date: 2023-03-20
host: x86_64-unknown-linux-gnu
release: 1.68.1
LLVM version: 15.0.6

Additional Labels

No response

@joshtriplett joshtriplett 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 Mar 27, 2023
@joshtriplett
Copy link
Member Author

Note that .into() has a similar false positive:

use core::ffi::c_ulong;

fn main() {
    let x: c_ulong = 0;
    let y: u64 = x.into();
    println!("Hello, {y}!");
}

produces:

warning: useless conversion to the same type: `u64`
 --> src/main.rs:5:18
  |
5 |     let y: u64 = x.into();
  |                  ^^^^^^^^ help: consider removing `.into()`: `x`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
  = note: `#[warn(clippy::useless_conversion)]` on by default

@Alexendoo
Copy link
Member

This one is tricky, #8596 would cover the case of some_u64 as c_ulong where we can check the rustc_hir::Ty of c_ulong to see it's an alias, but for the x in x as u64 we only have the rustc_middle::ty::Ty which doesn't distinguish u64 from c_ulong

@joshtriplett
Copy link
Member Author

joshtriplett commented Mar 30, 2023 via email

@Alexendoo
Copy link
Member

I saw https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/type.20aliases.20and.20names.20in.20errors today, may be a good way to resolve it if it does end up happening

Yeah I'm not saying it's unimportant, just a note for contributors looking through issues that this one wouldn't be easy

@Alexendoo Alexendoo added the E-hard Call for participation: This a hard problem and requires more experience or effort to work on label Jun 10, 2023
@bors bors closed this as completed in edaf740 Jun 12, 2023
@Alexendoo
Copy link
Member

The demonstrative example has been handled but the general issue remains, e.g.

fn main() {
    let pid = unsafe { libc::getpid() };
    pid as i32;
}

@Alexendoo Alexendoo reopened this Jun 12, 2023
bors added a commit that referenced this issue Jun 16, 2023
Ignore more type aliases in `unnecessary_cast`

This is potentially the worst code I've ever written, and even if not, it's very close to being on par with starb. This will ignore `call() as i32` and `local_obtained_from_call as i32` now.

This should fix every reasonable way to reproduce #10555, but likely not entirely.

changelog: Ignore more type aliases in `unnecessary_cast`
brgl pushed a commit to brgl/libgpiod-private that referenced this issue Jun 30, 2023
clippy falsely complains about these lines. The problem is known, but
unfixed [1]. So lets silence the warning until a fix is widely available.

clippy version: clippy 0.1.70 (90c5418 2023-05-31).

[1] rust-lang/rust-clippy#10555

Reported-by: Kent Gibson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Erik Schilling <[email protected]>
Acked-by: Viresh Kumar <[email protected]>
Reviewed-by: Kent Gibson <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
Enet4 added a commit to Enet4/dos-like-rs that referenced this issue Oct 18, 2023
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 E-hard Call for participation: This a hard problem and requires more experience or effort to work on 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.

2 participants