[clang-tidy] Fix bug 34845, offending standard bitmask types

Summary:
The C++ standard allows implementations to choose the underlying type for
bitmask types (e.g. std::ios_base::openmode). MSVC implemented some of them
as signed integers resulting in warnings for usual code like
`auto dd = std::ios_base::badbit | std::ios_base::failbit;`

These false positives were reported in https://bugs.llvm.org/show_bug.cgi?id=34845

The fix allows bitwise |,&,^ for known standard bitmask types under the condition
that both operands are such bitmask types.
Shifting and bitwise complement are still forbidden.

Reviewers: aaron.ballman, alexfh, hokein

Reviewed By: aaron.ballman

Subscribers: xazax.hun

Differential Revision: https://reviews.llvm.org/D39099


git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@316767 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 49 insertions, 11 deletions

diff --git a/clang-tidy/hicpp/SignedBitwiseCheck.cpp b/clang-tidy/hicpp/SignedBitwiseCheck.cpp
index 08aed657..887c0bee 100644
--- a/clang-tidy/hicpp/SignedBitwiseCheck.cpp
+++ b/clang-tidy/hicpp/SignedBitwiseCheck.cpp
@@ -20,34 +20,72 @@ namespace hicpp {
 
 void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
   const auto SignedIntegerOperand =
-      expr(ignoringImpCasts(hasType(isSignedInteger()))).bind("signed_operand");
+      expr(ignoringImpCasts(hasType(isSignedInteger()))).bind("signed-operand");
+
+  // The standard [bitmask.types] allows some integral types to be implemented
+  // as signed types. Exclude these types from diagnosing for bitwise or(|) and
+  // bitwise and(&). Shifting and complementing such values is still not
+  // allowed.
+  const auto BitmaskType = namedDecl(anyOf(
+      hasName("::std::locale::category"), hasName("::std::ctype_base::mask"),
+      hasName("::std::ios_base::fmtflags"), hasName("::std::ios_base::iostate"),
+      hasName("::std::ios_base::openmode")));
+  const auto IsStdBitmask = ignoringImpCasts(declRefExpr(hasType(BitmaskType)));
 
   // Match binary bitwise operations on signed integer arguments.
   Finder->addMatcher(
-      binaryOperator(allOf(anyOf(hasOperatorName("|"), hasOperatorName("&"),
-                                 hasOperatorName("^"), hasOperatorName("<<"),
-                                 hasOperatorName(">>")),
+      binaryOperator(
+          allOf(anyOf(hasOperatorName("^"), hasOperatorName("|"),
+                      hasOperatorName("&")),
+
+                unless(allOf(hasLHS(IsStdBitmask), hasRHS(IsStdBitmask))),
+
+                hasEitherOperand(SignedIntegerOperand),
+                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(allOf(anyOf(hasOperatorName("<<"), hasOperatorName(">>")),
                            hasEitherOperand(SignedIntegerOperand),
                            hasLHS(hasType(isInteger())),
                            hasRHS(hasType(isInteger()))))
-          .bind("binary_signed"),
+          .bind("binary-sign-interference"),
       this);
 
   // Match unary operations on signed integer types.
   Finder->addMatcher(unaryOperator(allOf(hasOperatorName("~"),
                                          hasUnaryOperand(SignedIntegerOperand)))
-                         .bind("unary_signed"),
+                         .bind("unary-signed"),
                      this);
 }
 
 void SignedBitwiseCheck::check(const MatchFinder::MatchResult &Result) {
   const ast_matchers::BoundNodes &N = Result.Nodes;
-  const auto *SignedBinary = N.getNodeAs<BinaryOperator>("binary_signed");
-  const auto *SignedUnary = N.getNodeAs<UnaryOperator>("unary_signed");
-  const auto *SignedOperand = N.getNodeAs<Expr>("signed_operand");
+  const auto *SignedOperand = N.getNodeAs<Expr>("signed-operand");
+  assert(SignedOperand &&
+         "No signed operand found in problematic bitwise operations");
+
+  bool IsUnary = false;
+  SourceLocation Location;
+
+  if (const auto *UnaryOp = N.getNodeAs<UnaryOperator>("unary-signed")) {
+    IsUnary = true;
+    Location = UnaryOp->getLocStart();
+  } else {
+    if (const auto *BinaryOp =
+            N.getNodeAs<BinaryOperator>("binary-no-sign-interference"))
+      Location = BinaryOp->getLocStart();
+    else if (const auto *BinaryOp =
+                 N.getNodeAs<BinaryOperator>("binary-sign-interference"))
+      Location = BinaryOp->getLocStart();
+    else
+      llvm_unreachable("unexpected matcher result");
+  }
 
-  const bool IsUnary = SignedUnary != nullptr;
-  diag(IsUnary ? SignedUnary->getLocStart() : SignedBinary->getLocStart(),
+  diag(Location,
        "use of a signed integer operand with a %select{binary|unary}0 bitwise "
        "operator")
       << IsUnary << SignedOperand->getSourceRange();