Implement Chris's suggestions for the precendence warnings. Reformat the code a bit. Test the fixits.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@85231 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 30 insertions, 28 deletions

diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h
index bcac18ffa2..51172045c9 100644
--- a/include/clang/AST/Expr.h
+++ b/include/clang/AST/Expr.h
@@ -1830,7 +1830,9 @@ public:
   bool isAdditiveOp() const { return Opc == Add || Opc == Sub; }
   static bool isShiftOp(Opcode Opc) { return Opc == Shl || Opc == Shr; }
   bool isShiftOp() const { return isShiftOp(Opc); }
-  bool isBitwiseOp() const { return Opc >= And && Opc <= Or; }
+
+  static bool isBitwiseOp(Opcode Opc) { return Opc >= And && Opc <= Or; }
+  bool isBitwiseOp() const { return isBitwiseOp(Opc); }
 
   static bool isRelationalOp(Opcode Opc) { return Opc >= LT && Opc <= GE; }
   bool isRelationalOp() const { return isRelationalOp(Opc); }
@@ -1838,6 +1840,9 @@ public:
   static bool isEqualityOp(Opcode Opc) { return Opc == EQ || Opc == NE; }
   bool isEqualityOp() const { return isEqualityOp(Opc); }
 
+  static bool isComparisonOp(Opcode Opc) { return Opc >= LT && Opc <= NE; }
+  bool isComparisonOp() const { return isComparisonOp(Opc); }
+
   static bool isLogicalOp(Opcode Opc) { return Opc == LAnd || Opc == LOr; }
   bool isLogicalOp() const { return isLogicalOp(Opc); }
 
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 6f7ad609a0..5c3f5e7bd1 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -5412,13 +5412,8 @@ Action::OwningExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
                                                       OpLoc));
 }
 
-static inline bool IsBitwise(int Opc) {
-  return Opc >= BinaryOperator::And && Opc <= BinaryOperator::Or;
-}
-static inline bool IsEqOrRel(int Opc) {
-  return Opc >= BinaryOperator::LT && Opc <= BinaryOperator::NE;
-}
-
+/// SuggestParentheses - Emit a diagnostic together with a fixit hint that wraps
+/// ParenRange in parentheses.
 static void SuggestParentheses(Sema &Self, SourceLocation Loc,
                                const PartialDiagnostic &PD,
                                SourceRange ParenRange)
@@ -5436,13 +5431,18 @@ static void SuggestParentheses(Sema &Self, SourceLocation Loc,
     << CodeModificationHint::CreateInsertion(EndLoc, ")");
 }
 
+/// DiagnoseBitwisePrecedence - Emit a warning when bitwise and comparison
+/// operators are mixed in a way that suggests that the programmer forgot that
+/// comparison operators have higher precedence. The most typical example of
+/// such code is "flags & 0x0020 != 0", which is equivalent to "flags & 1".
 static void DiagnoseBitwisePrecedence(Sema &Self, BinaryOperator::Opcode Opc,
                                       SourceLocation OpLoc,Expr *lhs,Expr *rhs){
-  typedef BinaryOperator::Opcode Opcode;
-  int lhsopc = -1, rhsopc = -1;
-  if (BinaryOperator *BO = dyn_cast<BinaryOperator>(lhs))
+  typedef BinaryOperator BinOp;
+  BinOp::Opcode lhsopc = static_cast<BinOp::Opcode>(-1),
+                rhsopc = static_cast<BinOp::Opcode>(-1);
+  if (BinOp *BO = dyn_cast<BinOp>(lhs))
     lhsopc = BO->getOpcode();
-  if (BinaryOperator *BO = dyn_cast<BinaryOperator>(rhs))
+  if (BinOp *BO = dyn_cast<BinOp>(rhs))
     rhsopc = BO->getOpcode();
 
   // Subs are not binary operators.
@@ -5451,26 +5451,22 @@ static void DiagnoseBitwisePrecedence(Sema &Self, BinaryOperator::Opcode Opc,
 
   // Bitwise operations are sometimes used as eager logical ops.
   // Don't diagnose this.
-  if ((IsEqOrRel(lhsopc) || IsBitwise(lhsopc)) &&
-      (IsEqOrRel(rhsopc) || IsBitwise(rhsopc)))
+  if ((BinOp::isComparisonOp(lhsopc) || BinOp::isBitwiseOp(lhsopc)) &&
+      (BinOp::isComparisonOp(rhsopc) || BinOp::isBitwiseOp(rhsopc)))
     return;
 
-  if (IsEqOrRel(lhsopc))
+  if (BinOp::isComparisonOp(lhsopc))
     SuggestParentheses(Self, OpLoc,
       PDiag(diag::warn_precedence_bitwise_rel)
-        << SourceRange(lhs->getLocStart(), OpLoc)
-        << BinaryOperator::getOpcodeStr(Opc)
-        << BinaryOperator::getOpcodeStr(static_cast<Opcode>(lhsopc)),
-      SourceRange(cast<BinaryOperator>(lhs)->getRHS()->getLocStart(),
-                  rhs->getLocEnd()));
-  else if (IsEqOrRel(rhsopc))
+          << SourceRange(lhs->getLocStart(), OpLoc)
+          << BinOp::getOpcodeStr(Opc) << BinOp::getOpcodeStr(lhsopc),
+      SourceRange(cast<BinOp>(lhs)->getRHS()->getLocStart(), rhs->getLocEnd()));
+  else if (BinOp::isComparisonOp(rhsopc))
     SuggestParentheses(Self, OpLoc,
       PDiag(diag::warn_precedence_bitwise_rel)
-        << SourceRange(OpLoc, rhs->getLocEnd())
-        << BinaryOperator::getOpcodeStr(Opc)
-        << BinaryOperator::getOpcodeStr(static_cast<Opcode>(rhsopc)),
-      SourceRange(lhs->getLocEnd(),
-                  cast<BinaryOperator>(rhs)->getLHS()->getLocStart()));
+          << SourceRange(OpLoc, rhs->getLocEnd())
+          << BinOp::getOpcodeStr(Opc) << BinOp::getOpcodeStr(rhsopc),
+      SourceRange(lhs->getLocEnd(), cast<BinOp>(rhs)->getLHS()->getLocStart()));
 }
 
 /// DiagnoseBinOpPrecedence - Emit warnings for expressions with tricky
@@ -5478,7 +5474,7 @@ static void DiagnoseBitwisePrecedence(Sema &Self, BinaryOperator::Opcode Opc,
 /// But it could also warn about arg1 && arg2 || arg3, as GCC 4.3+ does.
 static void DiagnoseBinOpPrecedence(Sema &Self, BinaryOperator::Opcode Opc,
                                     SourceLocation OpLoc, Expr *lhs, Expr *rhs){
-  if (IsBitwise(Opc))
+  if (BinaryOperator::isBitwiseOp(Opc))
     DiagnoseBitwisePrecedence(Self, Opc, OpLoc, lhs, rhs);
 }
 
diff --git a/test/Sema/parentheses.c b/test/Sema/parentheses.c
index 360aade837..a8ad260bf8 100644
--- a/test/Sema/parentheses.c
+++ b/test/Sema/parentheses.c
@@ -1,4 +1,5 @@
-// RUN: clang-cc -Wparentheses -fsyntax-only -verify %s
+// RUN: clang-cc -Wparentheses -fsyntax-only -verify %s &&
+// RUN: clang-cc -Wparentheses -fixit %s -o - | clang-cc -Wparentheses -Werror -
 
 // Test the various warnings under -Wparentheses
 void if_assign(void) {