Author: Shivam Gupta Date: 2023-08-08T08:00:02+05:30 New Revision: a84525233776a716e2c6291993f0b33fd1c76f7c
URL: https://github.com/llvm/llvm-project/commit/a84525233776a716e2c6291993f0b33fd1c76f7c DIFF: https://github.com/llvm/llvm-project/commit/a84525233776a716e2c6291993f0b33fd1c76f7c.diff LOG: Revert "[Clang] Fix -Wconstant-logical-operand when LHS is a constant" This reverts commit dfdfd306cfaf54fbc43e2d5eb36489dac3eb9976. An issue is reported for wrong warning, this has to be reconsidered. Differential Revision: https://reviews.llvm.org/D157352 Added: Modified: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaExpr.cpp clang/test/C/drs/dr4xx.c clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp clang/test/Parser/cxx2a-concept-declaration.cpp clang/test/Sema/exprs.c clang/test/SemaCXX/expressions.cpp clang/test/SemaCXX/warn-unsequenced.cpp clang/test/SemaTemplate/dependent-expr.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 44bd3c4cf3a665..4cd58ba071283a 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -12728,8 +12728,6 @@ class Sema final { 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); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index b37e8598249944..c1c6a34e8bc84f 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -13880,56 +13880,6 @@ inline QualType Sema::CheckBitwiseOperands(ExprResult &LHS, ExprResult &RHS, 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, @@ -13948,6 +13898,9 @@ inline QualType Sema::CheckLogicalOperands(ExprResult &LHS, ExprResult &RHS, } } + 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(); @@ -13958,14 +13911,40 @@ inline QualType Sema::CheckLogicalOperands(ExprResult &LHS, ExprResult &RHS, 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); + // 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 (!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) { diff --git a/clang/test/C/drs/dr4xx.c b/clang/test/C/drs/dr4xx.c index 24fa2670955478..5c5349aa7c1409 100644 --- a/clang/test/C/drs/dr4xx.c +++ b/clang/test/C/drs/dr4xx.c @@ -308,9 +308,7 @@ void dr489(void) { 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}} */ } } diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp index a5b57b76625df2..18b2c6b1d6d15b 100644 --- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp +++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp @@ -72,15 +72,9 @@ 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}} diff --git a/clang/test/Parser/cxx2a-concept-declaration.cpp b/clang/test/Parser/cxx2a-concept-declaration.cpp index fa30703bf8df76..0a7af847112de1 100644 --- a/clang/test/Parser/cxx2a-concept-declaration.cpp +++ b/clang/test/Parser/cxx2a-concept-declaration.cpp @@ -79,11 +79,6 @@ template<typename T> concept C16 = true && (0 && 0); // expected-error {{atomic // 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; diff --git a/clang/test/Sema/exprs.c b/clang/test/Sema/exprs.c index 20a4c8fcc8b982..31c6d1e01491a7 100644 --- a/clang/test/Sema/exprs.c +++ b/clang/test/Sema/exprs.c @@ -212,10 +212,6 @@ int test20(int x) { // 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. @@ -227,9 +223,6 @@ int test20(int x) { // 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}} \ diff --git a/clang/test/SemaCXX/expressions.cpp b/clang/test/SemaCXX/expressions.cpp index bde7adb756974b..641cfc8af7ce99 100644 --- a/clang/test/SemaCXX/expressions.cpp +++ b/clang/test/SemaCXX/expressions.cpp @@ -44,9 +44,6 @@ 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; @@ -69,8 +66,6 @@ int test2(int x) { // 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}} @@ -144,5 +139,6 @@ void test4() { 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}} } diff --git a/clang/test/SemaCXX/warn-unsequenced.cpp b/clang/test/SemaCXX/warn-unsequenced.cpp index 5076a2dfcedd7a..50dde8f3a57891 100644 --- a/clang/test/SemaCXX/warn-unsequenced.cpp +++ b/clang/test/SemaCXX/warn-unsequenced.cpp @@ -76,37 +76,15 @@ void test() { (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'}} diff --git a/clang/test/SemaTemplate/dependent-expr.cpp b/clang/test/SemaTemplate/dependent-expr.cpp index f2d74ffbcfdd52..51bd375d7920ee 100644 --- a/clang/test/SemaTemplate/dependent-expr.cpp +++ b/clang/test/SemaTemplate/dependent-expr.cpp @@ -43,9 +43,7 @@ namespace PR7198 { 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 { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
