[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

3 files changed, 318 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();
diff --git a/test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp b/test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp
new file mode 100644
index 00000000..bfb00bfd
--- /dev/null
+++ b/test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp
@@ -0,0 +1,188 @@
+// RUN: clang-tidy %s -checks='-*,hicpp-signed-bitwise' -- -std=c++11
+
+#include "hicpp-signed-bitwise-standard-types.h"
+
+void pure_bitmask_types() {
+  // std::locale::category
+  int SResult = 0;
+  std::locale::category C = std::locale::category::ctype;
+
+  SResult = std::locale::category::none | std::locale::category::collate;
+  SResult = std::locale::category::ctype & std::locale::category::monetary;
+  SResult = std::locale::category::numeric ^ std::locale::category::time;
+  SResult = std::locale::category::messages | std::locale::category::all;
+
+  SResult = std::locale::category::all & C;
+  SResult = std::locale::category::all | C;
+  SResult = std::locale::category::all ^ C;
+
+  // std::ctype_base::mask
+  std::ctype_base::mask M = std::ctype_base::mask::punct;
+
+  SResult = std::ctype_base::mask::space | std::ctype_base::mask::print;
+  SResult = std::ctype_base::mask::cntrl & std::ctype_base::mask::upper;
+  SResult = std::ctype_base::mask::lower ^ std::ctype_base::mask::alpha;
+  SResult = std::ctype_base::mask::digit | std::ctype_base::mask::punct;
+  SResult = std::ctype_base::mask::xdigit & std::ctype_base::mask::alnum;
+  SResult = std::ctype_base::mask::alnum ^ std::ctype_base::mask::graph;
+
+  SResult = std::ctype_base::mask::space & M;
+  SResult = std::ctype_base::mask::space | M;
+  SResult = std::ctype_base::mask::space ^ M;
+
+  // std::ios_base::fmtflags
+  std::ios_base::fmtflags F = std::ios_base::fmtflags::floatfield;
+
+  SResult = std::ios_base::fmtflags::dec | std::ios_base::fmtflags::oct;
+  SResult = std::ios_base::fmtflags::hex & std::ios_base::fmtflags::basefield;
+  SResult = std::ios_base::fmtflags::left ^ std::ios_base::fmtflags::right;
+  SResult = std::ios_base::fmtflags::internal | std::ios_base::fmtflags::adjustfield;
+  SResult = std::ios_base::fmtflags::scientific & std::ios_base::fmtflags::fixed;
+  SResult = std::ios_base::fmtflags::floatfield ^ std::ios_base::fmtflags::boolalpha;
+  SResult = std::ios_base::fmtflags::showbase | std::ios_base::fmtflags::showpoint;
+  SResult = std::ios_base::fmtflags::showpos & std::ios_base::fmtflags::skipws;
+  SResult = std::ios_base::fmtflags::unitbuf ^ std::ios_base::fmtflags::uppercase;
+
+  SResult = std::ios_base::fmtflags::unitbuf | F;
+  SResult = std::ios_base::fmtflags::unitbuf & F;
+  SResult = std::ios_base::fmtflags::unitbuf ^ F;
+
+  // std::ios_base::iostate
+  std::ios_base::iostate S = std::ios_base::iostate::goodbit;
+
+  SResult = std::ios_base::iostate::goodbit | std::ios_base::iostate::badbit;
+  SResult = std::ios_base::iostate::failbit & std::ios_base::iostate::eofbit;
+  SResult = std::ios_base::iostate::failbit ^ std::ios_base::iostate::eofbit;
+
+  SResult = std::ios_base::iostate::goodbit | S;
+  SResult = std::ios_base::iostate::goodbit & S;
+  SResult = std::ios_base::iostate::goodbit ^ S;
+
+  // std::ios_base::openmode
+  std::ios_base::openmode B = std::ios_base::openmode::binary;
+
+  SResult = std::ios_base::openmode::app | std::ios_base::openmode::binary;
+  SResult = std::ios_base::openmode::in & std::ios_base::openmode::out;
+  SResult = std::ios_base::openmode::trunc ^ std::ios_base::openmode::ate;
+
+  SResult = std::ios_base::openmode::trunc | B;
+  SResult = std::ios_base::openmode::trunc & B;
+  SResult = std::ios_base::openmode::trunc ^ B;
+}
+
+void still_forbidden() {
+  // std::locale::category
+  unsigned int UResult = 0u;
+  int SResult = 0;
+
+  SResult = std::ctype_base::mask::print ^ 8u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = std::ctype_base::mask::cntrl | 8;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = std::ctype_base::mask::upper & 8;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = std::ctype_base::mask::lower ^ -8;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = std::locale::category::collate << 1u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::locale::category::ctype << 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::locale::category::monetary >> 1u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::locale::category::numeric >> 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = ~std::locale::category::messages;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator
+  SResult = ~std::locale::category::all;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator
+
+  // std::ctype_base::mask
+  UResult = std::ctype_base::mask::space | 8;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ctype_base::mask::print & 8u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ctype_base::mask::cntrl ^ -8;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = std::ctype_base::mask::upper << 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ctype_base::mask::lower << 1u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ctype_base::mask::alpha >> 1u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ctype_base::mask::digit >> 1u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = ~std::ctype_base::mask::punct;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator
+  SResult = ~std::ctype_base::mask::xdigit;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator
+
+  // std::ios_base::fmtflags
+  UResult = std::ios_base::fmtflags::dec | 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::fmtflags::oct & 1u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::fmtflags::hex ^ -1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = std::ios_base::fmtflags::basefield >> 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::fmtflags::left >> 1u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::fmtflags::right << 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::fmtflags::internal << 1u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = ~std::ios_base::fmtflags::adjustfield;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator
+  SResult = ~std::ios_base::fmtflags::scientific;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator
+
+  // std::ios_base::iostate
+  UResult = std::ios_base::iostate::goodbit | 8;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::iostate::badbit & 8u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::iostate::failbit ^ -8;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = std::ios_base::iostate::eofbit << 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::iostate::goodbit << 1u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::iostate::badbit >> 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::iostate::failbit >> 1u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = ~std::ios_base::iostate::eofbit;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator
+  SResult = ~std::ios_base::iostate::goodbit;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator
+
+  // std::ios_base::openmode
+  UResult = std::ios_base::app | 8;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::binary & 8u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::in ^ -8;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = std::ios_base::out >> 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::trunc >> 1u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::ate << 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = std::ios_base::ate << 1u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = ~std::ios_base::openmode::app;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator
+  SResult = ~std::ios_base::openmode::binary;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator
+}
diff --git a/test/clang-tidy/hicpp-signed-bitwise-standard-types.h b/test/clang-tidy/hicpp-signed-bitwise-standard-types.h
new file mode 100644
index 00000000..e66fd090
--- /dev/null
+++ b/test/clang-tidy/hicpp-signed-bitwise-standard-types.h
@@ -0,0 +1,81 @@
+#pragma clang system_header
+
+// Implement standard types that are known to be defined as unsigned in some
+// implementations like MSVC.
+namespace std {
+namespace locale {
+enum category : int {
+  none = 0u,
+  collate = 1u << 1u,
+  ctype = 1u << 2u,
+  monetary = 1u << 3u,
+  numeric = 1u << 4u,
+  time = 1u << 5u,
+  messages = 1u << 6u,
+  all = none | collate | ctype | monetary | numeric | time | messages
+  // CHECK MESSAGES: [[@LINE-1]]:9: warning: use of a signed integer operand with a binary bitwise operator
+};
+} // namespace locale
+
+namespace ctype_base {
+enum mask : int {
+  space,
+  print,
+  cntrl,
+  upper,
+  lower,
+  alpha,
+  digit,
+  punct,
+  xdigit,
+  /* blank, // C++11 */
+  alnum = alpha | digit,
+  // CHECK MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
+  graph = alnum | punct
+  // CHECK MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
+};
+} // namespace ctype_base
+
+namespace ios_base {
+enum fmtflags : int {
+  dec = 0u,
+  oct = 1u << 2u,
+  hex = 1u << 3u,
+  basefield = dec | oct | hex | 0u,
+  // CHECK MESSAGES: [[@LINE-1]]:15: warning: use of a signed integer operand with a binary bitwise operator
+  left = 1u << 4u,
+  right = 1u << 5u,
+  internal = 1u << 6u,
+  adjustfield = left | right | internal,
+  // CHECK MESSAGES: [[@LINE-1]]:17: warning: use of a signed integer operand with a binary bitwise operator
+  scientific = 1u << 7u,
+  fixed = 1u << 8u,
+  floatfield = scientific | fixed | (scientific | fixed) | 0u,
+  // CHECK MESSAGES: [[@LINE-1]]:16: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK MESSAGES: [[@LINE-2]]:38: warning: use of a signed integer operand with a binary bitwise operator
+  boolalpha = 1u << 9u,
+  showbase = 1u << 10u,
+  showpoint = 1u << 11u,
+  showpos = 1u << 12u,
+  skipws = 1u << 13u,
+  unitbuf = 1u << 14u,
+  uppercase = 1u << 15u
+};
+
+enum iostate : int {
+  goodbit = 0u,
+  badbit = 1u << 1u,
+  failbit = 1u << 2u,
+  eofbit = 1u << 3u
+};
+
+enum openmode : int {
+  app = 0u,
+  binary = 0u << 1u,
+  in = 0u << 2u,
+  out = 0u << 3u,
+  trunc = 0u << 4u,
+  ate = 0u << 5u
+};
+} // namespace ios_base
+} // namespace std