Add a warning to catch a bug recently caught by code review, like this:

t2.c:2:12: warning: use of logical && with constant operand; switch to bitwise &
      or remove constant [-Wlogical-bitwise-confusion]
  return x && 4;
           ^  ~

wording improvement suggestions are welcome.

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

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 8fac1edecfa80dc64e6dd2aa3d7fb84568e85226..f6542afa0797ac1cb9583890c857f9294c3b9b7b 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1986,6 +1986,10 @@ def note_precedence_bitwise_first : Note<
   "place parentheses around the %0 expression to evaluate it first">;
 def note_precedence_bitwise_silence : Note<
   "place parentheses around the %0 expression to silence this warning">;
+
+def warn_logical_instead_of_bitwise : Warning<
+  "use of logical %0 with constant operand; switch to bitwise %1 or "
+  "remove constant">, InGroup<DiagGroup<"logical-bitwise-confusion">>;
   
 def err_sizeof_nonfragile_interface : Error<
   "invalid application of '%select{alignof|sizeof}1' to interface %0 in "

diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h
index c4153b6cabb12dfe6fd8c2fbff6f45db37ba4c97..d902dfea1f1287cc6f8ef8c4280a0755b86adcc5 100644 (file)
--- a/lib/Sema/Sema.h
+++ b/lib/Sema/Sema.h
@@ -4353,7 +4353,7 @@ public:
   QualType CheckBitwiseOperands( // C99 6.5.[10...12]
     Expr *&lex, Expr *&rex, SourceLocation OpLoc, bool isCompAssign = false);
   QualType CheckLogicalOperands( // C99 6.5.[13,14]
-    Expr *&lex, Expr *&rex, SourceLocation OpLoc);
+    Expr *&lex, Expr *&rex, SourceLocation OpLoc, unsigned Opc);
   // CheckAssignmentOperands is used for both simple and compound assignment.
   // For simple assignment, pass both expressions and a null converted type.
   // For compound assignment, pass both expressions and the converted type.

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 5d53ddeb98ebb4f3a59b525b02d91d00ac19e9cd..cb3518ae83c024cf26a7ed6f2f0dc4473615ef39 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -5735,7 +5735,21 @@ inline QualType Sema::CheckBitwiseOperands(
 }
 
 inline QualType Sema::CheckLogicalOperands( // C99 6.5.[13,14]
-  Expr *&lex, Expr *&rex, SourceLocation Loc) {
+  Expr *&lex, Expr *&rex, SourceLocation Loc, unsigned Opc) {
+  
+  // Diagnose cases where the user write a logical and/or but probably meant a
+  // bitwise one.  We do this when the LHS is a non-bool integer and the RHS
+  // is a constant.
+  if (lex->getType()->isIntegerType() && !lex->getType()->isBooleanType() &&
+      rex->getType()->isIntegerType() && rex->isEvaluatable(Context) &&
+      !Loc.isMacroID())
+    Diag(Loc, diag::warn_logical_instead_of_bitwise)
+     << rex->getSourceRange()
+      << (Opc == BinaryOperator::LAnd ? "&&" : "||")
+      << (Opc == BinaryOperator::LAnd ? "&" : "|");
+  
+  
+  
   if (!Context.getLangOptions().CPlusPlus) {
     UsualUnaryConversions(lex);
     UsualUnaryConversions(rex);
@@ -6363,7 +6377,7 @@ Action::OwningExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
     break;
   case BinaryOperator::LAnd:
   case BinaryOperator::LOr:
-    ResultTy = CheckLogicalOperands(lhs, rhs, OpLoc);
+    ResultTy = CheckLogicalOperands(lhs, rhs, OpLoc, Opc);
     break;
   case BinaryOperator::MulAssign:
   case BinaryOperator::DivAssign:

diff --git a/test/CodeGenCXX/static-init-2.cpp b/test/CodeGenCXX/static-init-2.cpp
index 65ab3bb1262df5fb7a3768a42fd24b4497eb2db6..7eb4a7d5aa79345f54c89d9310982512c3741c89 100644 (file)
--- a/test/CodeGenCXX/static-init-2.cpp
+++ b/test/CodeGenCXX/static-init-2.cpp
@@ -3,4 +3,4 @@
 // Make sure we don't crash generating y; its value is constant, but the
 // initializer has side effects, so EmitConstantExpr should fail.
 int x();
-int y = x() && 0;
+int y = x() && 0;   // expected-warning {{use of logical && with constant operand}}

diff --git a/test/Sema/exprs.c b/test/Sema/exprs.c
index b22b5220f2859ea7c7d6dfdc0c7d3cec17b8ef91..54e44693e085b059cded98b7c182916cf00bac3a 100644 (file)
--- a/test/Sema/exprs.c
+++ b/test/Sema/exprs.c
@@ -142,3 +142,6 @@ void test19() {
   *(volatile int*)0 = 0;  // Ok.
 }
 
+int test20(int x) {
+  return x && 4; // expected-warning {{use of logical && with constant operand; switch to bitwise & or remove constant}}
+}

diff --git a/test/Sema/i-c-e.c b/test/Sema/i-c-e.c
index c86a93fed82988300562c48813498b0ae426f350..eb77bbe3b99023b32052bd861aa454ccc9e9ffa0 100644 (file)
--- a/test/Sema/i-c-e.c
+++ b/test/Sema/i-c-e.c
@@ -51,7 +51,8 @@ char z[__builtin_constant_p(4) ? 1 : -1];
 
 // Comma tests
 int comma1[0?1,2:3];  // expected-warning {{expression result unused}}
-int comma2[1||(1,2)]; // expected-warning {{expression result unused}}
+int comma2[1||(1,2)]; // expected-warning {{expression result unused}} \
+                      // expected-warning {{use of logical || with constant operand}}
 int comma3[(1,2)]; // expected-warning {{size of static array must be an integer constant expression}} \
                                        // expected-warning {{expression result unused}}
 

diff --git a/test/Sema/switch.c b/test/Sema/switch.c
index bb4822916cc72ecc5f117ad8a50331f8a79af672..4e39e0f0c2d5d527eaca226c9da696da2745109f 100644 (file)
--- a/test/Sema/switch.c
+++ b/test/Sema/switch.c
@@ -50,12 +50,14 @@ void test4()
   }
   
   switch (cond) {
-  case g() && 0: // expected-error {{expression is not an integer constant expression}} // expected-note {{subexpression not valid in an integer constant expression}}
+  case g() && 0: // expected-error {{expression is not an integer constant expression}} // expected-note {{subexpression not valid in an integer constant expression}} \
+                    expected-warning {{use of logical && with constant operand}}
     break;
   }
   
   switch (cond) {
-  case 0 ... g() || 1: // expected-error {{expression is not an integer constant expression}} // expected-note {{subexpression not valid in an integer constant expression}}
+  case 0 ... g() || 1: // expected-error {{expression is not an integer constant expression}} // expected-note {{subexpression not valid in an integer constant expression}} \\
+                          expected-warning {{use of logical || with constant operand}}
     break;
   }
 }

diff --git a/test/SemaCXX/bool.cpp b/test/SemaCXX/bool.cpp
index 44e17ce62b79380acb7e73dbdbb22a3df41f64cb..726fa6cb60fc13cb2cb0e4a1cf4b559b112e72d9 100644 (file)
--- a/test/SemaCXX/bool.cpp
+++ b/test/SemaCXX/bool.cpp
@@ -25,6 +25,6 @@ void static_assert_arg_is_bool(T x) {
 
 void test2() {
   int n = 2;
-  static_assert_arg_is_bool(n && 4);
-  static_assert_arg_is_bool(n || 5);
+  static_assert_arg_is_bool(n && 4);  // expected-warning {{use of logical && with constant operand}}
+  static_assert_arg_is_bool(n || 5);  // expected-warning {{use of logical || with constant operand}}
 }

diff --git a/test/SemaCXX/switch.cpp b/test/SemaCXX/switch.cpp
index fc13630bbf12f457b840fbc91336a6564655d8f7..54240dcc305f619fe976e47aab44caaf95314a59 100644 (file)
--- a/test/SemaCXX/switch.cpp
+++ b/test/SemaCXX/switch.cpp
@@ -8,7 +8,8 @@ void test() {
   }
 
   int n = 3;
-  switch (n && 1) { // expected-warning {{bool}}
+  switch (n && 1) { // expected-warning {{bool}} \
+                    // expected-warning {{use of logical && with constant operand}}
     case 1:
       break;
   }