"Use of a signed integer operand with a binary bitwise operator" - when using unsigned short

Question

In the following C snippet that checks if the first two bits of a 16-bit sequence are set:

bool is_pointer(unsigned short int sequence) {   return (sequence >> 14) == 3; } 

CLion's Clang-Tidy is giving me a "Use of a signed integer operand with a binary bitwise operator" warning, and I can't understand why. Is unsigned short not unsigned enough?

Answer 1

The code for this warning checks if either operand to the bitwise operator is signed. It is not sequence causing the warning, but 14, and you can alleviate the problem by making 14 unsigned by appending a u to the end.

(sequence >> 14u) 

This warning is bad. As Roland's answer describes, CLion is fixing this.

Answer 2

There is a check in clang-tidy that is called hicpp-signed-bitwise. This check follows the wording of the HIC++ standard. That standard is freely available and says:

5.6.1. Do not use bitwise operators with signed operands

Use of signed operands with bitwise operators is in some cases subject to undefined or implementation defined behavior. Therefore, bitwise operators should only be used with operands of unsigned integral types.

The authors of the HIC++ coding standard misinterpreted the intention of the C and C++ standards and either accidentally or intentionally focused on the type of the operands instead of the value of the operands.

The check in clang-tidy implements exactly this wording, in order to conform to that standard. That check is not intended to be generally useful, its only purpose is to help the poor souls whose programs have to conform to that one stupid rule from the HIC++ standard.

The crucial point is that by definition integer literals without any suffix are of type int, and that type is defined as being a signed type. HIC++ now wrongly concludes that positive integer literals might be negative and thus could invoke undefined behavior.

For comparison, the C11 standard says:

6.5.7 Bitwise shift operators

If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined.

This wording is carefully chosen and emphasises that the value of the right operand is important, not its type. It also covers the case of a too large value, while the HIC++ standard simply forgot that case. Therefore, saying 1u << 1000u is ok in HIC++, while 1 << 3 isn't.

The best strategy is to explicitly disable this single check. There are several bug reports for CLion mentioning this, and it is getting fixed there.

---

Update 2019-12-16: I asked Perforce what the motivation behind this exact wording was and whether the wording was intentional. Here is their response:

Our C++ team who were involved in creating the HIC++ standard have taken a look at the Stack Overflow question you mentioned.

In short, referring to the object type in the HIC++ rule instead of the value is an intentional choice to allow easier automated checking of the code. The type of an object is always known, while the value is not.

• HIC++ rules in general aim to be "decidable". Enforcing against the type ensures that a decidable check is always possible, ie. directly where the operator is used or where a signed type is converted to unsigned.
• The rationale explicitly refers to "possible" undefined behavior, therefore a sensible implementation can exclude:
  • constants unless there is definitely an issue and,
  • unsigned types that are promoted to signed types.
• The best operation is therefore for CLion to limit the checking to non-constant types before promotion.