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

decimal_literal_representation is subjective #2421

Closed
vorner opened this issue Feb 1, 2018 · 3 comments
Closed

decimal_literal_representation is subjective #2421

vorner opened this issue Feb 1, 2018 · 3 comments
Labels
S-needs-discussion Status: Needs further discussion before merging or work can be started

Comments

@vorner
Copy link
Contributor

vorner commented Feb 1, 2018

Hello

The lint about decimal literal being better written as hexadecimal feels very subjective. For me, if I see 4096, I know at once it's 4 kilobytes. If I see 0x1000, I have to really think how approximately large the number is. Sure, the second conveys the fact it is power of two more strongly, but the actual size of the number is better expressed in decimal, at least to me ‒ so I think it very much depends on the context, what is more important.

I find most of clippie's lints reasonable and based on some sane argument ‒ eg. performance, better suited method, etc. This one kind of seems to be out of line with that.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 1, 2018

@vorner it's configurable (e.g. literal_representation_threshold = 10000 in clippy.toml), so if you feel the 4096 lower limit is too low, you can always move it up. Or do you think we should set the default to something higher to begin with?

I agree that 4096 is something I'll trivially recognize. 8192, too, but from there on I don't recognize them anymore.

@oli-obk oli-obk added the S-needs-discussion Status: Needs further discussion before merging or work can be started label Feb 1, 2018
@vorner
Copy link
Contributor Author

vorner commented Feb 1, 2018

My point isn't about the limit, but about the lint itself. Whether I write a number in hexa or decimal is based on context, not on the value of the number.

If I care about the bit/byte patterns, I use hexa, because it preserves positions ‒ for example, when writing an RGB color, typing 0x0000FF is more readable blue, opposed to 255. But that's a rather small number.

On the other side, when talking about sizes, 1_073_741_824 is much more readable gigabyte for me (well, I'm not sure it's exactly one gigabyte ‒ but counting the triplets of digits gives a lot of insight) than 0x40_000_000, since in the later case I know it is binary-round number, but I don't have a clue about how large it actually is in some human-accessible units until I take a calculator and let it translate it for me.

@flip1995
Copy link
Member

flip1995 commented Feb 6, 2018

I wrote this lint and also thought about the context problem. Therefore the limit exists.

For big numbers like 1_073_741_824 vs. 0x4000_0000 it's personal preference IMO. If I would see the former number in some code, I wouldn't think of a gigabyte or associate it with a power of two. I would rather wonder where this magic number is coming from. With the hex-representation I instantly know that it is a power of two, where I can look up the decimal representation if I need to know this.

If you have a constant like const GIGABYTE: u32 = 0x4000_0000; I would also prefer the hex-representation, because the const name already gives away the meaning of that number, while it is less error-prone (can't really make a typo in this hex-representation).

So what should we do:

  • Raise the limit to 16384 = 0x4000: IMO for big powers of two, hex-representation is almost always better
  • Context sensitive: I don't really know if there is a good way to do this. For bit-operations you should probably almost always use a hex- or binary-representation, while for constants, variables and literals you can't really tell what the programmers intention is (plus personal preference).
  • Allow by default

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-needs-discussion Status: Needs further discussion before merging or work can be started
Projects
None yet
Development

No branches or pull requests

3 participants