Revert "[Clang] Fix -Wconstant-logical-operand when LHS is a constant"

[xgupta]

Summary

This reverts commit dfdfd306cfaf54fbc43e2d5eb36489dac3eb9976.

An issue is reported for wrong warning, this has to be reconsidered.

clang/include/clang/Sema/Sema.h

QualType CheckShiftOperands( // C99 6.5.7
     BinaryOperatorKind Opc, bool IsCompAssign = false);
   void CheckPtrComparisonWithNullChar(ExprResult &E, ExprResult &NullE);
   QualType CheckCompareOperands( // C99 6.5.8/9
       ExprResult &LHS, ExprResult &RHS, SourceLocation Loc,
       BinaryOperatorKind Opc);
   QualType CheckBitwiseOperands( // C99 6.5.[10...12]
       ExprResult &LHS, ExprResult &RHS, SourceLocation Loc,
       BinaryOperatorKind Opc);
-  void diagnoseLogicalInsteadOfBitwise(Expr *Op1, Expr *Op2, SourceLocation Loc,
-                                       BinaryOperatorKind Opc);
   QualType CheckLogicalOperands( // C99 6.5.[13,14]
     ExprResult &LHS, ExprResult &RHS, SourceLocation Loc,
     BinaryOperatorKind 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.
   QualType CheckAssignmentOperands( // C99 6.5.16.[1,2]
       Expr *LHSExpr, ExprResult &RHS, SourceLocation Loc, QualType CompoundType,

clang/lib/Sema/SemaExpr.cpp

inline QualType Sema::CheckBitwiseOperands(ExprResult &LHS, ExprResult &RHS,
   if (Opc == BO_Xor)
     diagnoseXorMisusedAsPow(*this, LHS, RHS, Loc);
 
   if (!compType.isNull() && compType->isIntegralOrUnscopedEnumerationType())
     return compType;
   return InvalidOperands(Loc, LHS, RHS);
 }
 
-// Diagnose cases where the user write a logical and/or but probably meant a
-// bitwise one.  We do this when one of the operands is a non-bool integer and
-// the other is a constant.
-void Sema::diagnoseLogicalInsteadOfBitwise(Expr *Op1, Expr *Op2,
-                                           SourceLocation Loc,
-                                           BinaryOperatorKind Opc) {
-  if (Op1->getType()->isIntegerType() && !Op1->getType()->isBooleanType() &&
-      Op2->getType()->isIntegerType() && !Op2->isValueDependent() &&
-      // Don't warn in macros or template instantiations.
-      !Loc.isMacroID() && !inTemplateInstantiation() &&
-      !Op2->getExprLoc().isMacroID() && !Op1->getExprLoc().isMacroID()) {
-    bool IsOp1InMacro = Op1->getExprLoc().isMacroID();
-    bool IsOp2InMacro = Op2->getExprLoc().isMacroID();
-
-    // Exclude the specific expression from triggering the warning.
-    if (!(IsOp1InMacro && IsOp2InMacro &&
-          Op1->getSourceRange() == Op2->getSourceRange())) {
-      // If the RHS can be constant folded, and if it constant folds to
-      // something that isn't 0 or 1 (which indicate a potential logical
-      // operation that happened to fold to true/false) then warn. Parens on the
-      // RHS are ignored. If the RHS can be constant folded, and if it constant
-      // folds to something that isn't 0 or 1 (which indicate a potential
-      // logical operation that happened to fold to true/false) then warn.
-      // Parens on the RHS are ignored.
-      Expr::EvalResult EVResult;
-      if (Op2->EvaluateAsInt(EVResult, Context)) {
-        llvm::APSInt Result = EVResult.Val.getInt();
-        if ((getLangOpts().Bool && !Op2->getType()->isBooleanType() &&
-             !Op2->getExprLoc().isMacroID()) ||
-            (Result != 0 && Result != 1)) {
-          Diag(Loc, diag::warn_logical_instead_of_bitwise)
-              << Op2->getSourceRange() << (Opc == BO_LAnd ? "&&" : "||");
-          // Suggest replacing the logical operator with the bitwise version
-          Diag(Loc, diag::note_logical_instead_of_bitwise_change_operator)
-              << (Opc == BO_LAnd ? "&" : "|")
-              << FixItHint::CreateReplacement(
-                     SourceRange(Loc, getLocForEndOfToken(Loc)),
-                     Opc == BO_LAnd ? "&" : "|");
-          if (Opc == BO_LAnd)
-            // Suggest replacing "Foo() && kNonZero" with "Foo()"
-            Diag(Loc, diag::note_logical_instead_of_bitwise_remove_constant)
-                << FixItHint::CreateRemoval(
-                       SourceRange(getLocForEndOfToken(Op1->getEndLoc()),
-                                   Op2->getEndLoc()));
-        }
-      }
-    }
-  }
-}
-
 // C99 6.5.[13,14]
 inline QualType Sema::CheckLogicalOperands(ExprResult &LHS, ExprResult &RHS,
                                            SourceLocation Loc,
                                            BinaryOperatorKind Opc) {
   // Check vector operands differently.
   if (LHS.get()->getType()->isVectorType() ||
       RHS.get()->getType()->isVectorType())
     return CheckVectorLogicalOperands(LHS, RHS, Loc);
 
   bool EnumConstantInBoolContext = false;
   for (const ExprResult &HS : {LHS, RHS}) {
     if (const auto *DREHS = dyn_cast<DeclRefExpr>(HS.get())) {
       const auto *ECDHS = dyn_cast<EnumConstantDecl>(DREHS->getDecl());
       if (ECDHS && ECDHS->getInitVal() != 0 && ECDHS->getInitVal() != 1)
         EnumConstantInBoolContext = true;
     }
   }
 
+  if (EnumConstantInBoolContext)
+    Diag(Loc, diag::warn_enum_constant_in_bool_context);
+
   // WebAssembly tables can't be used with logical operators.
   QualType LHSTy = LHS.get()->getType();
   QualType RHSTy = RHS.get()->getType();
   const auto *LHSATy = dyn_cast<ArrayType>(LHSTy);
   const auto *RHSATy = dyn_cast<ArrayType>(RHSTy);
   if ((LHSATy && LHSATy->getElementType().isWebAssemblyReferenceType()) ||
       (RHSATy && RHSATy->getElementType().isWebAssemblyReferenceType())) {
     return InvalidOperands(Loc, LHS, RHS);
   }
 
-  if (EnumConstantInBoolContext) {
-    // Warn when converting the enum constant to a boolean
-    Diag(Loc, diag::warn_enum_constant_in_bool_context);
-  } else {
-    // Diagnose cases where the user write a logical and/or but probably meant a
-    // bitwise one.
-    diagnoseLogicalInsteadOfBitwise(LHS.get(), RHS.get(), Loc, Opc);
-    diagnoseLogicalInsteadOfBitwise(RHS.get(), LHS.get(), Loc, Opc);
+  // bitwise one.  We do this when the LHS is a non-bool integer and the RHS
+  // is a constant.
+  if (!EnumConstantInBoolContext && LHS.get()->getType()->isIntegerType() &&
+      !LHS.get()->getType()->isBooleanType() &&
+      RHS.get()->getType()->isIntegerType() && !RHS.get()->isValueDependent() &&
+      // Don't warn in macros or template instantiations.
+      !Loc.isMacroID() && !inTemplateInstantiation()) {
+    // If the RHS can be constant folded, and if it constant folds to something
+    // that isn't 0 or 1 (which indicate a potential logical operation that
+    // happened to fold to true/false) then warn.
+    // Parens on the RHS are ignored.
+    Expr::EvalResult EVResult;
+    if (RHS.get()->EvaluateAsInt(EVResult, Context)) {
+      llvm::APSInt Result = EVResult.Val.getInt();
+      if ((getLangOpts().Bool && !RHS.get()->getType()->isBooleanType() &&
+           !RHS.get()->getExprLoc().isMacroID()) ||
+          (Result != 0 && Result != 1)) {
+        Diag(Loc, diag::warn_logical_instead_of_bitwise)
+            << RHS.get()->getSourceRange() << (Opc == BO_LAnd ? "&&" : "||");
+        // Suggest replacing the logical operator with the bitwise version
+        Diag(Loc, diag::note_logical_instead_of_bitwise_change_operator)
+            << (Opc == BO_LAnd ? "&" : "|")
+            << FixItHint::CreateReplacement(
+                   SourceRange(Loc, getLocForEndOfToken(Loc)),
+                   Opc == BO_LAnd ? "&" : "|");
+        if (Opc == BO_LAnd)
+          // Suggest replacing "Foo() && kNonZero" with "Foo()"
+          Diag(Loc, diag::note_logical_instead_of_bitwise_remove_constant)
+              << FixItHint::CreateRemoval(
+                     SourceRange(getLocForEndOfToken(LHS.get()->getEndLoc()),
+                                 RHS.get()->getEndLoc()));
+      }
+    }
   }
 
   if (!Context.getLangOpts().CPlusPlus) {
     // OpenCL v1.1 s6.3.g: The logical operators and (&&), or (||) do
     // not operate on the built-in scalar and vector float types.
     if (Context.getLangOpts().OpenCL &&
         Context.getLangOpts().OpenCLVersion < 120) {
       if (LHS.get()->getType()->isFloatingType() ||

clang/test/C/drs/dr4xx.c

void dr489(void) {
 
   /* FIXME: this should be accepted in C2x and up without a diagnostic, as C23
    * added compound literals to the allowed list of things in an ICE. The
    * diagnostic is correct for C17 and earlier though.
    */
   case (int){ 2 }: break;   /* expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
                                c89only-warning {{compound literals are a C99-specific feature}}
                              */
-  case 12 || main(): break; /* expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
-                               expected-warning {{use of logical '||' with constant operand}} \
-                               expected-note {{use '|' for a bitwise operation}} */
+  case 12 || main(): break; /* expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}} */
   }
 }
 
 /* WG14 DR492: yes
  * Named Child struct-union with no Member
  */
 struct dr492_t {
   union U11 {  /* expected-warning {{declaration does not declare anything}} */

clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp

 struct S { int a; int b; int c[2]; };
 constexpr S InitList(int a) { return { a, ng }; }; // expected-error {{never produces}} expected-note {{read}}
 constexpr S InitList1a(int a) { return S{ a, ng }; }; // expected-error {{never produces}} expected-note {{read}}
 constexpr S InitList2(int a) { return { a, a, { ng } }; }; // expected-error {{never produces}} expected-note {{read}}
 constexpr S InitList3(int a) { return a ? S{ a, a } : S{ a, ng }; }; // ok
 
 constexpr int LogicalAnd1(int n) { return n && (throw, 0); } // ok
 constexpr int LogicalAnd2(int n) { return 1 && (throw, 0); } // expected-error {{never produces}} expected-note {{subexpression}}
-                                                             // expected-warning@-1 {{use of logical '&&' with constant operand}}
-                                                             // expected-note@-2 {{use '&' for a bitwise operation}}
-                                                             // expected-note@-3 {{remove constant to silence this warning}}
 
 constexpr int LogicalOr1(int n) { return n || (throw, 0); } // ok
 constexpr int LogicalOr2(int n) { return 0 || (throw, 0); } // expected-error {{never produces}} expected-note {{subexpression}}
-                                                            // expected-warning@-1 {{use of logical '||' with constant operand}}
-                                                            // expected-note@-2 {{use '|' for a bitwise operation}}
-
 
 constexpr int Conditional1(bool b, int n) { return b ? n : ng; } // ok
 constexpr int Conditional2(bool b, int n) { return b ? n * ng : n + ng; } // expected-error {{never produces}} expected-note {{both arms of conditional operator are unable to produce a constant expression}}
 
 // __builtin_constant_p ? : is magical, and is always a potential constant.
 constexpr bool BcpCall(int n) {
   return __builtin_constant_p((int*)n != &n) ? (int*)n != &n : (int*)n != &n; // expected-warning 3 {{cast to 'int *' from smaller integer type 'int'}}
 }

clang/test/Parser/cxx2a-concept-declaration.cpp

 template<bool x> concept C12 = 2 && x; // expected-error {{atomic constraint must be of type 'bool' (found 'int')}}
 template<bool x> concept C13 = x || 2 || x; // expected-error {{atomic constraint must be of type 'bool' (found 'int')}}
 template<bool x> concept C14 = 8ull && x || x; // expected-error {{atomic constraint must be of type 'bool' (found 'unsigned long long')}}
 template<typename T> concept C15 = sizeof(T); // expected-error {{atomic constraint must be of type 'bool'}}
 template<typename T> concept C16 = true && (0 && 0); // expected-error {{atomic constraint must be of type 'bool' (found 'int')}}
 // expected-warning@-1{{use of logical '&&' with constant operand}}
 // expected-note@-2{{use '&' for a bitwise operation}}
 // expected-note@-3{{remove constant to silence this warning}}
-// expected-warning@-4{{use of logical '&&' with constant operand}}
-// expected-note@-5{{use '&' for a bitwise operation}}
-// expected-note@-6{{remove constant to silence this warning}}
-
-
 template<typename T> concept C17 = T{};
 static_assert(!C17<bool>);
 template<typename T> concept C18 = (bool&&)true;
 static_assert(C18<int>);
 template<typename T> concept C19 = (const bool&)true;
 static_assert(C19<int>);
 template<typename T> concept C20 = (const bool)true;
 static_assert(C20<int>);

clang/test/Sema/exprs.c

void test19(void) {
   int __attribute__((address_space(0))) *p2 = &(*(int __attribute__((address_space(0))) *)0); // Ok.
 }
 
 int test20(int x) {
   return x && 4; // expected-warning {{use of logical '&&' with constant operand}} \
                  // expected-note {{use '&' for a bitwise operation}} \
                  // expected-note {{remove constant to silence this warning}}
 
-  return 4 && x; // expected-warning {{use of logical '&&' with constant operand}} \
-                 // expected-note {{use '&' for a bitwise operation}} \
-                 // expected-note {{remove constant to silence this warning}}
-
   return x && sizeof(int) == 4;  // no warning, RHS is logical op.
   
   // no warning, this is an idiom for "true" in old C style.
   return x && (signed char)1;
 
   return x || 0;
   return x || 1;
   return x || -1; // expected-warning {{use of logical '||' with constant operand}} \
                   // expected-note {{use '|' for a bitwise operation}}
   return x || 5; // expected-warning {{use of logical '||' with constant operand}} \
                  // expected-note {{use '|' for a bitwise operation}}
-  return 5 || x; // expected-warning {{use of logical '||' with constant operand}} \
-                 // expected-note {{use '|' for a bitwise operation}}
-
   return x && 0;
   return x && 1;
   return x && -1; // expected-warning {{use of logical '&&' with constant operand}} \
                   // expected-note {{use '&' for a bitwise operation}} \
                   // expected-note {{remove constant to silence this warning}}
   return x && 5; // expected-warning {{use of logical '&&' with constant operand}} \
                  // expected-note {{use '&' for a bitwise operation}} \
                  // expected-note {{remove constant to silence this warning}}

clang/test/SemaCXX/expressions.cpp

void test_enums() {
     bar(x += E_zero); // expected-error {{incompatible type}}
   }
 }
 
 int test2(int x) {
   return x && 4; // expected-warning {{use of logical '&&' with constant operand}} \
                    // expected-note {{use '&' for a bitwise operation}} \
                    // expected-note {{remove constant to silence this warning}}
-  return 4 && x; // expected-warning {{use of logical '&&' with constant operand}} \
-                   // expected-note {{use '&' for a bitwise operation}} \
-                   // expected-note {{remove constant to silence this warning}}
 
   return x && sizeof(int) == 4;  // no warning, RHS is logical op.
   return x && true;
   return x && false;
   return x || true;
   return x || false;
 
   return x && (unsigned)0; // expected-warning {{use of logical '&&' with constant operand}} \
                    // expected-note {{use '&' for a bitwise operation}} \
                    // expected-note {{remove constant to silence this warning}}
 
   return x || (unsigned)1; // expected-warning {{use of logical '||' with constant operand}} \
                    // expected-note {{use '|' for a bitwise operation}}
 
   return x || 0; // expected-warning {{use of logical '||' with constant operand}} \
                    // expected-note {{use '|' for a bitwise operation}}
   return x || 1; // expected-warning {{use of logical '||' with constant operand}} \
                    // expected-note {{use '|' for a bitwise operation}}
   return x || -1; // expected-warning {{use of logical '||' with constant operand}} \
                    // expected-note {{use '|' for a bitwise operation}}
   return x || 5; // expected-warning {{use of logical '||' with constant operand}} \
                    // expected-note {{use '|' for a bitwise operation}}
-  return 5 || x; // expected-warning {{use of logical '||' with constant operand}} \
-                   // expected-note {{use '|' for a bitwise operation}}
   return x && 0; // expected-warning {{use of logical '&&' with constant operand}} \
                    // expected-note {{use '&' for a bitwise operation}} \
                    // expected-note {{remove constant to silence this warning}}
   return x && 1; // expected-warning {{use of logical '&&' with constant operand}} \
                    // expected-note {{use '&' for a bitwise operation}} \
                    // expected-note {{remove constant to silence this warning}}
   return x && -1; // expected-warning {{use of logical '&&' with constant operand}} \
                    // expected-note {{use '&' for a bitwise operation}} \

 }
 
 void test4() {
   #define X 0
   #define Y 1
   bool r1 = X || Y;
 
   #define Y2 2
-  bool r2 = X || Y2;
+  bool r2 = X || Y2; // expected-warning {{use of logical '||' with constant operand}} \
+                     // expected-note {{use '|' for a bitwise operation}}
 }

clang/test/SemaCXX/warn-unsequenced.cpp

void test() {
   a = S { ++a, a++ }.n; // ok
   A { ++a, a++ }.x; // ok
   a = A { ++a, a++ }.x; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
   A { ++a, a++ }.x + A { ++a, a++ }.y; // cxx11-warning {{multiple unsequenced modifications to 'a'}}
                                        // cxx17-warning@-1 {{multiple unsequenced modifications to 'a'}}
 
   (xs[2] && (a = 0)) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
                           // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
-
-  (0 && (a = 0)) + a; // cxx11-warning {{use of logical '&&' with constant operand}}
-                      // cxx11-note@-1 {{use '&' for a bitwise operation}}
-                      // cxx11-note@-2 {{remove constant to silence this warning}}
-                      // cxx17-warning@-3 {{use of logical '&&' with constant operand}}
-                      // cxx17-note@-4 {{use '&' for a bitwise operation}}
-                      // cxx17-note@-5 {{remove constant to silence this warning}}
-
+  (0 && (a = 0)) + a; // ok
   (1 && (a = 0)) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
                       // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
-                      // cxx11-warning@-2 {{use of logical '&&' with constant operand}}
-                      // cxx11-note@-3 {{use '&' for a bitwise operation}}
-                      // cxx11-note@-4 {{remove constant to silence this warning}}
-                      // cxx17-warning@-5 {{use of logical '&&' with constant operand}}
-                      // cxx17-note@-6 {{use '&' for a bitwise operation}}
-                      // cxx17-note@-7 {{remove constant to silence this warning}}
-
 
   (xs[3] || (a = 0)) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
                           // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (0 || (a = 0)) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
                       // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
-                      // cxx11-warning@-2 {{use of logical '||' with constant operand}}
-                      // cxx11-note@-3 {{use '|' for a bitwise operation}}
-                      // cxx17-warning@-4 {{use of logical '||' with constant operand}}
-                      // cxx17-note@-5 {{use '|' for a bitwise operation}}
-  (1 || (a = 0)) + a; // cxx11-warning {{use of logical '||' with constant operand}}
-                      // cxx11-note@-1 {{use '|' for a bitwise operation}}
-                      // cxx17-warning@-2 {{use of logical '||' with constant operand}}
-                      // cxx17-note@-3 {{use '|' for a bitwise operation}}
-
+  (1 || (a = 0)) + a; // ok
 
   (xs[4] ? a : ++a) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
                          // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (0 ? a : ++a) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
                      // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
   (1 ? a : ++a) + a; // ok
   (0 ? a : a++) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
                      // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}

clang/test/SemaTemplate/dependent-expr.cpp

struct B {
     {
       C c = C();
     }
   };
 }
 
 namespace PR7724 {
   template<typename OT> int myMethod()
-  { return 2 && sizeof(OT); } // expected-warning {{use of logical '&&' with constant operand}} \
-                              // expected-note {{use '&' for a bitwise operation}} \
-                              // expected-note {{remove constant to silence this warning}}
+  { return 2 && sizeof(OT); }
 }
 
 namespace test4 {
   template <typename T> T *addressof(T &v) {
     return reinterpret_cast<T*>(
              &const_cast<char&>(reinterpret_cast<const volatile char &>(v)));
   }
 }