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
diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h
index bcac18f..5117204 100644
--- a/include/clang/AST/Expr.h
+++ b/include/clang/AST/Expr.h
@@ -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); }
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 6f7ad60..5c3f5e7 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -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);
}
diff --git a/test/Sema/parentheses.c b/test/Sema/parentheses.c
index 360aade..a8ad260 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) {