[clang-tidy] hicpp-signed-bitwise IgnorePositiveIntegerLiterals now partially recursive

[njames93]

Summary

Addresses hicpp-signed-bitwise.IgnorePositiveIntegerLiterals should be recursive.
Only recursive for |, ^ and & operations and their assignment counterparts.

[aaron.ballman]

What about bit shifting operations? e.g., ~(1 << 2) or 8 >> (1 << 2)

[JonasToth]

ping.
@njames93 are you revisiting this patch? What is the missing piece? Maybe i can help out a bit?

clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp

void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
   // bitwise and(&). Shifting and complementing such values is still not
   // allowed.
   const auto BitmaskType = namedDecl(
       hasAnyName("::std::locale::category", "::std::ctype_base::mask",
                  "::std::ios_base::fmtflags", "::std::ios_base::iostate",
                  "::std::ios_base::openmode"));
   const auto IsStdBitmask = ignoringImpCasts(declRefExpr(hasType(BitmaskType)));
 
+  const auto SignNoInterferenceOp =
+      hasAnyOperatorName("^", "^=", "|", "|=", "&", "&=");
+
+  const auto SignedIntegerOperandExtended =
+      (IgnorePositiveIntegerLiterals
+           ? expr(ignoringImpCasts(hasType(isSignedInteger())),
+                  unless(anyOf(integerLiteral(),
+                               binaryOperator(SignNoInterferenceOp))))
+           : expr(ignoringImpCasts(hasType(isSignedInteger()))))
+          .bind("signed-operand");
+
   // Match binary bitwise operations on signed integer arguments.
   Finder->addMatcher(
-      binaryOperator(anyOf(hasOperatorName("^"), hasOperatorName("|"),
-                           hasOperatorName("&"), hasOperatorName("^="),
-                           hasOperatorName("|="), hasOperatorName("&=")),
+      binaryOperator(SignNoInterferenceOp,
 
                      unless(allOf(hasLHS(IsStdBitmask), hasRHS(IsStdBitmask))),
 
-                     hasEitherOperand(SignedIntegerOperand),
+                     hasEitherOperand(SignedIntegerOperandExtended),
                      hasLHS(hasType(isInteger())), hasRHS(hasType(isInteger())))
           .bind("binary-no-sign-interference"),
       this);
 
   // Shifting and complement is not allowed for any signed integer type because
   // the sign bit may corrupt the result.
   Finder->addMatcher(
       binaryOperator(anyOf(hasOperatorName("<<"), hasOperatorName(">>"),

clang-tools-extra/test/clang-tidy/checkers/hicpp-signed-bitwise-integer-literals.cpp

void examples() {
   IResult = 10 & 2; //Ok
   IResult = 3 << -1;
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
 
   int Int = 30;
   IResult = Int << 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
   IResult = ~0; //Ok
+
+  IResult = 2 | 4 | 8;
+  IResult = 2 | 4 | 8 | 16;
+  IResult = 2 | 4 | 8 | 16 | -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
 }
 
 enum EnumConstruction {
   one = 1,
   two = 2,
   test1 = 1 << 12, //Ok
   test2 = one << two,
   // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
   test3 = 1u << 12, //Ok
 };