This might turn to be a stupid question but I really don't understand why clang-tidy is complaining here.
Consider the following config:
# .clang-tidy
---
FormatStyle: file
WarningsAsErrors: '*'
Checks: >
-*,
hicpp-signed-bitwise,
CheckOptions:
hicpp-signed-bitwise.IgnorePositiveIntegerLiterals: true
And the following minimal example:
#include "stdint.h"
#include "stdio.h"
#include "inttypes.h"
typedef uint8_t mytypeU8_t;
typedef uint32_t mytypeU32_t;
int main(void) {
mytypeU8_t mytypeU8 = 0;
mytypeU32_t mytypeU32 = 0;
unsigned char testChar = 0;
printf("%"PRIu8, testChar & 1);
printf("%"PRIu8, testChar << 2);
printf("%"PRIu8, (testChar << 2 ) | (testChar >> 2));
printf("%"PRIu8, mytypeU8 & 1);
printf("%"PRIu8, mytypeU8 << 2);
printf("%"PRIu8, (mytypeU8 << 2 ) | (mytypeU8 >> 2));
printf("%"PRIu32, mytypeU32 & 1);
printf("%"PRIu32, mytypeU32 << 2);
printf("%"PRIu32, (mytypeU32 << 2 ) | (mytypeU32 >> 2));
return 0;
}
Why does clang-tidy print these errors:
test.c:15:22: error: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise,-warnings-as-errors]
15 | printf("%"PRIu8, (testChar << 2 ) | (testChar >> 2));
| ^~~~~~~~~~~~~~~~ ~
test.c:19:22: error: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise,-warnings-as-errors]
19 | printf("%"PRIu8, (mytypeU8 << 2 ) | (mytypeU8 >> 2));
| ^~~~~~~~~~~~~~~~ ~
test.c:21:35: error: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise,-warnings-as-errors]
21 | printf("%"PRIu32, mytypeU32 & 1);
| ~ ^
EDIT:
Thinking about this more, I think the first two errors might stem from the fact that the 8-bit values get implicitly promoted to ints before being |ed, which is understandable.
But why complain about mytypeU32 & 1 and not for any of the others - mytypeU32 << 2, testChar & 1, etc?
The hicpp-signed-bitwise checker implements
Rule 5.6.1
of the High Integrity C++ Coding Standard published by Perforce:
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.
That's too vague to answer some aspects of the question, so I'll focus on what Clang implements.
With IgnorePositiveIntegerLiterals set to true (per the
.clang-tidy file in the question), the hicpp-signed-bitwise check
(doc,
source)
reports uses of bitwise operators when:
either operand matches SignedIntegerOperand, and
both operands have integral type, meaning that
Type::isIntegerType
returns true.
SignedIntegerOperand is (in this configuration) defined as the
Clang AST Matcher:
expr(ignoringImpCasts(hasType(isSignedInteger())),
unless(integerLiteral()))
This means that the operand must, after skipping implicit conversions,
have signed integral type
(Type::isSignedIntegerType is true),
but not be an integer literal. Note that the second check does
not skip implicit conversions.
(I'm ignoring some other checker details not relevant to the question.)
I do not know why the Clang implementors chose the particular interpretation of the rule that they did, but we can now proceed to use it to analyze the examples.
I'll look at each relevant expression in the test, one at a time, to see why it is or is not reported.
First, I'll note that all of the examples satisfy the second condition,
as both operands have integral type in every case. So the interesting
aspect is the SignedIntegerOperand matcher, which has to be satisfied
by at least one operand for the operator to be reported.
testChar & 1 // not reported
Here, testChar is implicitly promoted to int (C99 6.5.10 p3, 6.3.1.8
p1, 6.3.1.1 p2). But when the implicit conversion is skipped,
testChar is unsigned, so does not match. Meanwhile, 1 has no
implicit conversions, and since it is an integer literal, is also does
not match, hence there is no report.
testChar << 2 // not reported
Similarly, testChar is promoted (C99 6.5.7 p3) but is unsigned beneath
that; and 2 is not promoted (it is already an int), and is a
literal.
(testChar << 2 ) | (testChar >> 2) // reported
The shift expressions are not reported per the above logic. But for the
bitwise-OR expression, both operands have type int, and neither is a
literal, so it is reported.
mytypeU8 << 2 // not reported
Same as testChar << 2.
(mytypeU8 << 2 ) | (mytypeU8 >> 2) // reported
Same as (testChar << 2 ) | (testChar >> 2).
mytypeU32 & 1 // reported
This is the first tricky one. mytypeU32 is unsigned, so does not
match. But 1 looks like an integer literal, so the report should be
suppressed, right? But since the LHS is unsigned int, the RHS is
converted to unsigned int as well! That adds an implicit conversion,
and that conversion is not an integer literal.
One can confirm this by changing the match expression to:
expr(ignoringImpCasts(hasType(isSignedInteger())),
unless(ignoringImpCasts(integerLiteral())))
// ^^^^^^^^^^^^^^^^
// inserted
in which case it will no longer match, and hence not be reported. (I
checked this with clang-query rather than actually modifying
clang-tidy, for expedience.)
mytypeU32 << 2 // not reported
This is the second tricky one. Here, mytypeU32 is unsigned, so does
not satisfy SignedIntegerOperand. But what about 2? Doesn't the
logic used for mytypeU32 & 1 apply? No--because while & uses the
"usual arithmetic conversions" to arrive at a common type, << merely
promotes both operands independently since the types do not need to be
the same. Thus, 2 which already has type int (and thus does not
change under promotion), is not implicitly converted, and hence is
an integer literal, thus causing SignedIntegerOperand to also not
match.
(mytypeU32 << 2 ) | (mytypeU32 >> 2) // not reported
Both operands are unsigned, so SignedIntegerOperand does not match.
The fact that implicit conversions are not skipped when checking for an integer literal is certainly questionable, and the consequent report of the line:
mytypeU32 & 1 // reported
seems like pointless noise to me.
Furthermore, the documentation of IgnorePositiveIntegerLiterals states:
If this option is set to true, the check will not warn on bitwise operations with positive integer literals, [...]
which naively seems like it ought to apply to the mytypeU32 & 1 case. So I'm inclined to say yes, that report is a false positive.
However, one must acknowledge that the rationale for the original rule is a little suspect, and that rule does not have any exceptions for literals, so it's hard to make a definitive judgment.
The question "Use of a signed integer operand with a binary bitwise operator" - when using unsigned short and its answers have some interesting related discussion, including of the rationale for the rule and Clang's interpretation of it.