3

What's the proper modern C++ way of converting two uint8t's into one int16_t that would satisfy clang-tidy?

a[0] << 8 | a[1]

If a[0] >= 128, I want the result to wrap around, resulting in a negative value.

Clang-Tidy message is

Use of a signed integer operand with a binary bitwise operator
L. F.
  • 19,445
  • 8
  • 48
  • 82
vasily
  • 2,850
  • 1
  • 24
  • 40
  • 4
    Dunno what will satisfy clang-tidy, but you will want to cast `a[0]` to a `int16_t` *before* shifting it left, otherwise you end up shifting a `uint8_t` left 8 bits, and losing the bits. – Jeremy Friesner Aug 02 '19 at 03:57
  • @JeremyFriesner Clang-Tidy says "Use of a signed integer operand with a binary bitwise operator" – vasily Aug 02 '19 at 04:17
  • 1
    @JeremyFriesner - I could be mistaken, but the copy of the C++ standard says, `The operands shall be of integral or unscoped enumeration type and integral promotions are performed. The type of the result is that of the promoted left operand.` That kind of implies to me that the `a[0] << 8` doesn't lose bits, but instead gets promoted to something larger. This is where I need a language lawyer to interpret, but a quick compile test shows this to be the case. But that could be undefined behavior meets conventional implementation. Somebody help me out here. – selbie Aug 02 '19 at 04:22
  • @selbie And to what would a `uint8_t` be promoted? How does this fit with the Clang-Tidy message mentioned in the comments? – JaMiT Aug 02 '19 at 05:12
  • @vasily It seems to me that the exact message is too important for explaining the question to be left in the comments... – JaMiT Aug 02 '19 at 05:13
  • *Sheepishly suggests...* [`static_cast()`](https://stackoverflow.com/questions/103512/why-use-static-castintx-instead-of-intx)? –  Aug 02 '19 at 05:31
  • See [Numeric promotions - cppreference.com](https://en.cppreference.com/w/cpp/language/implicit_conversion) (mid-page) – David C. Rankin Aug 02 '19 at 05:34
  • 2
    What is the expected result if `a[0] >= 128`? A negative number? – L. F. Aug 02 '19 at 05:38
  • `a[0] * 256 + a[1]` works just as well and generates identical executable code with modern compilers. – n. m. could be an AI Aug 02 '19 at 05:40
  • @L.F. yes, it looks that way in the datasheet. – vasily Aug 02 '19 at 05:40
  • By the way, @selbie, it seems like you are correct. ["Short types" seem to be promoted to ints](https://stackoverflow.com/a/46073296/10957435). –  Aug 02 '19 at 05:52
  • Also, [useful](https://stackoverflow.com/questions/51582231/clang-tidy-use-of-a-signed-integer-operand-with-a-binary-bitwise-operator?noredirect=1&lq=1) but I wouldn't call a duplicate. They seem to be in a slightly different situation. –  Aug 02 '19 at 05:55

1 Answers1

2

Here's one way to convert to a uint16_t (we'll talk about int16_t later)

constexpr std::uint16_t combine(std::uint8_t a, std::uint8_t b)
{
    return static_cast<unsigned>(a) << 8 | static_cast<unsigned>(b);
}

(live demo in C++11)

One way to ensure portability is to first convert the uint8_t value to unsigned int to ensure predictable integer promotion. The value is preserved regardless of the type aliased by uint8_t, because unsigned int is guaranteed to be capable of holding all nonnegative integers below 256, and the unsigned integer conversion rules guarantee that the value is preserved. Then, the operations are guaranteed to operate on unsigned int instead of a signed integer type.

Question: why convert to unsigned? You are mixing types together. Why not convert to uint16_t directly?

Note that using static_cast<uint16_t>(a) instead is not a portable solution because an uint16_t may still be promoted to int depending on the environment (for example, when uint16_t is unsigned short and int is 32 bits). Converting to unsigned gives us full control over the integer promotion rules.

Now the OP want to convert this number to int16_t, resulting in a negative number if a >= 128. You can be pretty sure you are using a two's complement system if you are actually intending to do this, so a simple static_cast may suffice. Otherwise, you can use a separate check. Again, a bit weird.

L. F.
  • 19,445
  • 8
  • 48
  • 82
  • @Chipster Because `uint16_t` may be promoted to `int`. Also, good question. – L. F. Aug 02 '19 at 05:34
  • Sorry, just realized it's actually `int16_t`, not `uint16_t`. Either way, doesn't the OP need it in an `int16_t` anyway? Why promote it to an int? I must be missing something really obvious. –  Aug 02 '19 at 05:37
  • I don't understand why an _`uint16_t` may be promoted to an int_. This type is guaranteed to be unsigned if I don't make a mistake. – Fareanor Aug 02 '19 at 06:50
  • @Fareanor Suppose that `uint16_t` is `unsigned short` and `int` is 32 bits. Then, per N3337 [\[conv.prom\]/1](https://timsong-cpp.github.io/cppwp/n3337/conv.prom#1), `unsigned short` is promoted to `int`. – L. F. Aug 02 '19 at 06:52
  • Ah ok I didn't know that. But, it is very strange anyway, to change the type of an object arbitrarily like that. Do you have an idea about the reason of wrecking the types when they are prvalues ? It is a nonsense to me... – Fareanor Aug 02 '19 at 07:04
  • @Fareanor "C was designed to implicitly and silently change the integer types of the operands used in expressions. There exist several cases where the language forces the compiler to either change the operands to a larger type, or to change their signedness. The rationale behind this is to prevent accidental overflows during arithmetic, but also to allow operands with different signedness to co-exist in the same expression ... – L. F. Aug 02 '19 at 07:09
  • ... Unfortunately, the rules for implicit type promotion cause much more harm than good, to the point where they might be one of the biggest flaws in the C language. These rules are often not even known by the average C programmer and therefore causing all manner of very subtle bugs. " (https://stackoverflow.com/a/46073296) C++ pretty much inherits these things from C. – L. F. Aug 02 '19 at 07:09
  • Ah thank you. I didn't know the existence of integral promotions. But to be sure, if you have declared a `uint16_t` and assigned to it the result of a `static_cast`, as the receiver is `uint16_t`, is it correct if I say that it is sufficient to avoid integral promotion ? If not, it is very strange that even in C or C++ we lose control of what we handle. – Fareanor Aug 02 '19 at 07:15
  • 1
    Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/197374/discussion-between-l-f-and-fareanor). – L. F. Aug 02 '19 at 07:16
  • I can't join, my network access is restricted. But thanks for your explanations :) – Fareanor Aug 02 '19 at 07:17
  • @Fareanor Oh, really. Yeah, integral promotions are only carried out when you do arithmetic. Passing the values around don't trigger integral promotion. – L. F. Aug 02 '19 at 07:18
  • Thanks, I'm reassured then :) Sorry for the extra comments. – Fareanor Aug 02 '19 at 07:19