Implement a warning for mixing bitwise logical with comparison ops. Fixes PR5297.

llvm-svn: 85117

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -1385,6 +1385,10 @@ def warn_shift_negative : Warning<
 def warn_shift_gt_typewidth : Warning<
   "shift count >= width of type">;
 
+def warn_precedence_bitwise_rel : Warning<
+  "%0 has lower precedence than %1; %1 will be evaluated first">,
+  InGroup<Parentheses>;
+
 def err_sizeof_nonfragile_interface : Error<
   "invalid application of '%select{alignof|sizeof}1' to interface %0 in "
   "non-fragile ABI">;

clang/lib/Sema/SemaExpr.cpp

@@ -5412,6 +5412,53 @@ 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;
+}
+
+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))
+    lhsopc = BO->getOpcode();
+  if (BinaryOperator *BO = dyn_cast<BinaryOperator>(rhs))
+    rhsopc = BO->getOpcode();
+
+  // Subs are not binary operators.
+  if (lhsopc == -1 && rhsopc == -1)
+    return;
+
+  // Bitwise operations are sometimes used as eager logical ops.
+  // Don't diagnose this.
+  if ((IsEqOrRel(lhsopc) || IsBitwise(lhsopc)) &&
+      (IsEqOrRel(rhsopc) || IsBitwise(rhsopc)))
+    return;
+
+  if (IsEqOrRel(lhsopc))
+    Self.Diag(OpLoc, diag::warn_precedence_bitwise_rel)
+      << SourceRange(lhs->getLocStart(), OpLoc)
+      << BinaryOperator::getOpcodeStr(Opc)
+      << BinaryOperator::getOpcodeStr(static_cast<Opcode>(lhsopc));
+  else if (IsEqOrRel(rhsopc))
+    Self.Diag(OpLoc, diag::warn_precedence_bitwise_rel)
+      << SourceRange(OpLoc, rhs->getLocEnd())
+      << BinaryOperator::getOpcodeStr(Opc)
+      << BinaryOperator::getOpcodeStr(static_cast<Opcode>(rhsopc));
+}
+
+/// DiagnoseBinOpPrecedence - Emit warnings for expressions with tricky
+/// precedence. This currently diagnoses only "arg1 'bitwise' arg2 'eq' arg3".
+/// 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))
+    DiagnoseBitwisePrecedence(Self, Opc, OpLoc, lhs, rhs);
+}
+
 // Binary Operators.  'Tok' is the token for the operator.
 Action::OwningExprResult Sema::ActOnBinOp(Scope *S, SourceLocation TokLoc,
                                           tok::TokenKind Kind,
@@ -5422,6 +5469,9 @@ Action::OwningExprResult Sema::ActOnBinOp(Scope *S, SourceLocation TokLoc,
   assert((lhs != 0) && "ActOnBinOp(): missing left expression");
   assert((rhs != 0) && "ActOnBinOp(): missing right expression");
 
+  // Emit warnings for tricky precedence issues, e.g. "bitfield & 0x4 == 0"
+  DiagnoseBinOpPrecedence(*this, Opc, TokLoc, lhs, rhs);
+
   if (getLangOptions().CPlusPlus &&
       (lhs->getType()->isOverloadableType() ||
        rhs->getType()->isOverloadableType())) {

clang/test/Sema/parentheses.c

@@ -0,0 +1,19 @@
+// RUN: clang-cc -Wparentheses -fsyntax-only -verify %s
+
+// Test the various warnings under -Wparentheses
+void if_assign(void) {
+  int i;
+  if (i = 4) {} // expected-warning {{assignment as a condition}}
+  if ((i = 4)) {}
+}
+
+void bitwise_rel(unsigned i) {
+  (void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}}
+  (void)(0 == i & 0x2); // expected-warning {{& has lower precedence than ==}}
+  (void)(i & 0xff < 30); // expected-warning {{& has lower precedence than <}}
+  (void)((i & 0x2) == 0);
+  (void)(i & (0x2 == 0));
+  // Eager logical op
+  (void)(i == 1 | i == 2 | i == 3);
+  (void)(i != 1 & i != 2 & i != 3);
+}