Skip to content

cast_lossless should lint about bool -> integer casts? #7947

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

Closed
5225225 opened this issue Nov 8, 2021 · 8 comments · Fixed by #7948
Closed

cast_lossless should lint about bool -> integer casts? #7947

5225225 opened this issue Nov 8, 2021 · 8 comments · Fixed by #7948
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing good first issue These issues are a good way to get started with Clippy I-false-negative Issue: The lint should have been triggered on code, but wasn't

Comments

@5225225
Copy link
Contributor

5225225 commented Nov 8, 2021

Lint name: cast_lossless

I tried this code:

#![forbid(clippy::cast_lossless)]

fn main() {
    let _x = true as u8;
}

I expected to see this happen: Clippy to complain about the bool -> u8 cast, since it can be replaced with u8::from(true).

Instead, this happened: No lint was triggered


The lint description does say it is a cast between numeric types which can be replaced with safe conversion functions, and bool isn't strictly a numeric type, but u8::from(bool) does exist. So it's unclear to me if this warning should be on cast_lossless or a new lint, but I feel it should exist.

Meta

Rust version (rustc -Vv):

rustc 1.58.0-nightly (18bc4bee9 2021-11-02)
binary: rustc
commit-hash: 18bc4bee9710b181b440a472635150f0d6257713
commit-date: 2021-11-02
host: x86_64-unknown-linux-gnu
release: 1.58.0-nightly
LLVM version: 13.0.0

(clippy version 0.1.58 (18bc4be 2021-11-02))

@5225225 5225225 added C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't labels Nov 8, 2021
@xFrednet
Copy link
Member

xFrednet commented Nov 8, 2021

@rustbot label +good-first-issue

@rustbot rustbot added the good first issue These issues are a good way to get started with Clippy label Nov 8, 2021
@5225225
Copy link
Contributor Author

5225225 commented Nov 8, 2021

alright, i'll have a shot at this

@5225225
Copy link
Contributor Author

5225225 commented Nov 8, 2021

@rustbot claim

@xFrednet
Copy link
Member

xFrednet commented Nov 8, 2021

Looking at the issue description again, I can actually understand why this instance isn't linted, as boolean to integer casts are always lossless. Could you explain why you think this should be linted? 🤔 (Sorry, to only ask that now after I added the label 😅 )

@5225225
Copy link
Contributor Author

5225225 commented Nov 8, 2021

I thought this lint was for casts that are always lossless (as written), which is why it's being linted about.

It comes from some places in my code where I was doing some_bool as u8 which is currently lossless, but will become lossy if the type of some_bool changes to be a u16, for example. Whereas if you do u8::from(bool), that's lossless because there are no from/into conversions in the stdlib that are lossy. So if the type of some_bool changes, it's no longer lossless.

Same reason it complains about some_u8 as u16, which is currently lossless, but will become lossy if some_u8 becomes a u32.

I guess bools are a different type of value, and there are no bools for which casts to integers aren't lossless, whereas only some integers cast to other integers are lossless. So you could argue that bool as u8 shouldn't be linted about here, but I feel it matches the spirit of the lint (tell me where I can use ty::from(x) instead of x as ty).

@xFrednet
Copy link
Member

xFrednet commented Nov 8, 2021

Okay, that makes sense, thanks for clarifying! I misunderstood the lint at first.

I agree that this is a false negative like you reported. The code example in the lint documentation also shows an example where u8 as u64 is linted 🙃.

@5225225
Copy link
Contributor Author

5225225 commented Nov 8, 2021

No worries, there's a lot of lints related to as. You were probably thinking of cast_possible_truncation

The docs for as_conversions lists 11(!) casts related to it.

If you want more precise lints for as, please consider using these separate lints: unnecessary_cast, cast_lossless/possible_truncation/possible_wrap/precision_loss/sign_loss, fn_to_numeric_cast(_with_truncation), char_lit_as_u8, ref_to_mut and ptr_as_ptr.

@xFrednet
Copy link
Member

xFrednet commented Nov 8, 2021

Yes, that was the behavior I was thinking about ^^. I always find new lint's in Clippy's collection, it's actually quite cool :)

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 good first issue These issues are a good way to get started with Clippy I-false-negative Issue: The lint should have been triggered on code, but wasn't
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants