cleanup: Use hex constants for i8/i16/i32/i64 limits. #79
Conversation
The rest of the code uses mostly hex constants, which are much less error-prone than a complicated number like 9223372036834775807 (in which I made a mistake, but it's hard to spot). I've only put `UINT64_C` around the 64 bit constant, because those macros aren't used anywhere else in the code. This avoids some warnings in overly strict compilers (cstd 6.4.4.1 allows large integer constants and puts them into whatever type can represent them, but some compilers warn about it anyway and ask you to put suffixes like `ULL`).
|
Should we use |
| return cmp_write_s16(ctx, (int16_t)d); | ||
| if (d >= (-2147483647 - 1)) |
There was a problem hiding this comment.
This was a confusing construct. Why did we have an expression like this?
There was a problem hiding this comment.
I didn't at all remember; but git blame says it once was -2147483648 which (at least some) 32-bit compilers didn't like.
Nah; these are MessagePack-specific constants, not platform constants; they shouldn't change based on what your machine thinks the biggest |
|
I tried really hard to get any compiler on Godbolt to complain about this, but nothing did so I think we're fine here. I'll merge and see if anyone experiences any issues. |
I mean INT32_MAX etc. They have exact definitions and aren't platform-specific. |
The rest of the code uses mostly hex constants, which are much less error-prone than a complicated number like 9223372036834775807 (in which I made a mistake, but it's hard to spot).
I've only put
UINT64_Caround the 64 bit constant, because those macros aren't used anywhere else in the code. This avoids some warnings in overly strict compilers (cstd 6.4.4.1 allows large integer constants and puts them into whatever type can represent them, but some compilers warn about it anyway and ask you to put suffixes likeULL).