Skip to content

False positive in unnecessary_cast with hex literals #5220

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
glfmn opened this issue Feb 23, 2020 · 5 comments · Fixed by #5257
Closed

False positive in unnecessary_cast with hex literals #5220

glfmn opened this issue Feb 23, 2020 · 5 comments · Fixed by #5257
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

Comments

@glfmn
Copy link

glfmn commented Feb 23, 2020

clippy 0.0.212 (69f99e7 2019-12-14)

The lint for #[warn(unnecessary_cast)] acts strangely when integers are written in hex. Example:

let r = 0x1b as f32 / 255.0;
//      ^^^^^^^^^^^ - unnecessary cast write integer literal as 29_f32

However, f32 is valid hex, so if I write 0x1b_f32 rust interprets the value as an integer. It does helpfully output the integer literal as a decimal number in the suggestion, but it isn't hex anymore which defeats the purpose.

@flip1995 flip1995 changed the title unnecessary_cast does not work with hex integers False positive in unnecessary_cast with hex literals Feb 24, 2020
@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy C-bug Category: Clippy is not doing the correct thing labels Feb 24, 2020
@mlegner
Copy link
Contributor

mlegner commented Mar 1, 2020

I would like to work on this issue and investigated a bit. Unfortunately, the AST doesn't seem to record whether the integer literal was written in hex or decimal notation. The enum LitIntType only records the suffix of the literal.

It seems to me that properly resolving these false positives would require extending the LitKind::Int node with additional information in the main rust repository. Am I missing something?

@glfmn
Copy link
Author

glfmn commented Mar 2, 2020

@mlegner that's unfortunate, I'm not really aware of those implementation details but from a glance it seems your summation of the problem is accurate.

@flip1995
Copy link
Member

flip1995 commented Mar 2, 2020

Clippy has it's own struct, that can be build from literals and from where you can detect if it is a hex, bin, or oct literal:

pub(super) struct NumericLiteral<'a> {
/// Which radix the literal was represented in.
radix: Radix,
/// The radix prefix, if present.
prefix: Option<&'a str>,
/// The integer part of the number.
integer: &'a str,
/// The fraction part of the number.
fraction: Option<&'a str>,
/// The character used as exponent seperator (b'e' or b'E') and the exponent part.
exponent: Option<(char, &'a str)>,
/// The type suffix, including preceding underscore if present.
suffix: Option<&'a str>,
}
impl<'a> NumericLiteral<'a> {
fn from_lit(src: &'a str, lit: &Lit) -> Option<NumericLiteral<'a>> {

So no need to modify the rustc AST.

@mlegner
Copy link
Contributor

mlegner commented Mar 2, 2020

Thanks a lot for the pointer, @flip1995. I was able to resolve this issue but had to modify some of the code you referenced. The changes are in the linked PR #5257.

@bors bors closed this as completed in 329923e Mar 4, 2020
@bors bors closed this as completed in #5257 Mar 4, 2020
@glfmn
Copy link
Author

glfmn commented Apr 2, 2020

Thanks for addressing this so quickly!

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants