CppCoreGuidelines
CppCoreGuidelines copied to clipboard
C++ integer promotion defeats "ES.101: Use unsigned types for bit manipulation" on small integer types
I just fell on "Clang-Tidy: Use of a signed integer operand with a binary bitwise operator" warning.
Point is, I was writing 8bit unsigned integer expressions from my point of view, but expression below can involve signed arithmetic:
uint8_t{} | uint8_t{} | uint8_t{}
Which is not obvious.
Because of [over.built] and [conv.prom] sections, below assert is not guaranted to hold:
static_assert(std::is_same_v<decltype(std::declval<T>() | std::declval<T>()), T>)
And more importantly, this one does not hold either:
static_assert(std::is_signed_v< decltype(std::declval<T>() | std::declval<T>())> == std::is_signed_v<T>)
It currently fails on major implementations for uint8_t, uint_fast8_t, etc. My limited understanding of the standard is that it could fail for any integer type smaller than int.
Technically the rule is correct, and I am not saying it should be changed to forbid bitwise ops on small ints. But I think it reveals a quite uncomfortable situation that deserves a note.
Similar issue: #1120, integer promotion is kind of a problem :see_no_evil:
Arguably clang-tidy (or the guideline) should be a bit smarter. Although the operands will get promoted to signed int
the values will still be in the range 0 to 255 and so the sign bit won't be set in any of the operands or in the result of the |
operator.
@jwakely
Is there any reason to expose int operator|(uint8_t, uint8_t)
instead of unsigned int operator|(uint8_t, uint8_t)
?
[conv.prom] says
can be converted
so for me a choice is possible. All compilers seems to have picked the least intuitive type to me, which makes me curious.
Note 1
I think it is even allowed to expose uint8_t operator|(uint8_t, uint8_t)
but that might be an arguable choice for operator+ and I guess you are supposed to always pick the same common type.
Note 2 From my limited testing with uint8_t, with gcc/clang/msvc/icc
- casting to uint8_t at each stage produce the best assembly overall
-
- only really changes for msvc and icc
-
- marginally changes for gcc
-
- does not change at all for clang
- unsurprisingly, casting to unsigned int changes nothing in assembly, in both optimized builds and default ones
See https://godbolt.org/z/jarff5
No choice is possible, promotion to int
is required.