[cfe-commits] r85231 - in /cfe/trunk: include/clang/AST/Expr.h lib/Sema/SemaExpr.cpp test/Sema/parentheses.c

[Sebastian Redl]

Author: cornedbee
Date: Tue Oct 27 07:10:02 2009
New Revision: 85231

URL: http://llvm.org/viewvc/llvm-project?rev=85231&view=rev
Log:
Implement Chris's suggestions for the precendence warnings. Reformat the code a bit. Test the fixits.

Modified:
    cfe/trunk/include/clang/AST/Expr.h
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/Sema/parentheses.c

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=85231&r1=85230&r2=85231&view=diff

==============================================================================
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Oct 27 07:10:02 2009
@@ -1830,7 +1830,9 @@
   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 @@
   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); }
 

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=85231&r1=85230&r2=85231&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Oct 27 07:10:02 2009
@@ -5412,13 +5412,8 @@
                                                       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 @@
     << 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 @@
 
   // 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 @@
 /// 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);
 }
 

Modified: cfe/trunk/test/Sema/parentheses.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/parentheses.c?rev=85231&r1=85230&r2=85231&view=diff

==============================================================================
--- cfe/trunk/test/Sema/parentheses.c (original)
+++ cfe/trunk/test/Sema/parentheses.c Tue Oct 27 07:10:02 2009
@@ -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) {