diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -12567,6 +12567,10 @@ QualType CheckBitwiseOperands( // C99 6.5.[10...12] ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, BinaryOperatorKind Opc); + void diagnoseLogicalInsteadOfBitwise(ExprResult &Op1, ExprResult &Op2, + SourceLocation Loc, + BinaryOperatorKind Opc, + bool EnumConstantInBoolContext); 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 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -13578,47 +13578,30 @@ return InvalidOperands(Loc, LHS, RHS); } -// C99 6.5.[13,14] -inline QualType Sema::CheckLogicalOperands(ExprResult &LHS, ExprResult &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(ExprResult &Op1, ExprResult &Op2, 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(HS.get())) { - const auto *ECDHS = dyn_cast(DREHS->getDecl()); - if (ECDHS && ECDHS->getInitVal() != 0 && ECDHS->getInitVal() != 1) - EnumConstantInBoolContext = true; - } - } - - if (EnumConstantInBoolContext) - Diag(Loc, diag::warn_enum_constant_in_bool_context); - - // 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() && + BinaryOperatorKind Opc, + bool EnumConstantInBoolContext) { + if (!EnumConstantInBoolContext && Op1.get()->getType()->isIntegerType() && + !Op1.get()->getType()->isBooleanType() && + Op2.get()->getType()->isIntegerType() && !Op2.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 + // If the Op2 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. + // Parens on the Op2 are ignored. Expr::EvalResult EVResult; - if (RHS.get()->EvaluateAsInt(EVResult, Context)) { + if (Op2.get()->EvaluateAsInt(EVResult, Context)) { llvm::APSInt Result = EVResult.Val.getInt(); - if ((getLangOpts().Bool && !RHS.get()->getType()->isBooleanType() && - !RHS.get()->getExprLoc().isMacroID()) || + if ((getLangOpts().Bool && !Op2.get()->getType()->isBooleanType() && + !Op2.get()->getExprLoc().isMacroID()) || (Result != 0 && Result != 1)) { Diag(Loc, diag::warn_logical_instead_of_bitwise) - << RHS.get()->getSourceRange() << (Opc == BO_LAnd ? "&&" : "||"); + << Op2.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 ? "&" : "|") @@ -13629,11 +13612,40 @@ // 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())); + SourceRange(getLocForEndOfToken(Op1.get()->getEndLoc()), + Op2.get()->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(HS.get())) { + const auto *ECDHS = dyn_cast(DREHS->getDecl()); + if (ECDHS && ECDHS->getInitVal() != 0 && ECDHS->getInitVal() != 1) + EnumConstantInBoolContext = true; + } + } + + if (EnumConstantInBoolContext) + Diag(Loc, diag::warn_enum_constant_in_bool_context); + + // Diagnose cases where the user write a logical and/or but probably meant a + // bitwise one. + diagnoseLogicalInsteadOfBitwise(LHS, RHS, Loc, Opc, + EnumConstantInBoolContext); + diagnoseLogicalInsteadOfBitwise(RHS, LHS, Loc, Opc, + EnumConstantInBoolContext); if (!Context.getLangOpts().CPlusPlus) { // OpenCL v1.1 s6.3.g: The logical operators and (&&), or (||) do diff --git a/clang/test/C/drs/dr4xx.c b/clang/test/C/drs/dr4xx.c --- a/clang/test/C/drs/dr4xx.c +++ b/clang/test/C/drs/dr4xx.c @@ -308,7 +308,9 @@ 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}} */ + 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}} */ } } 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 --- 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,9 +72,15 @@ 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 --- a/clang/test/Parser/cxx2a-concept-declaration.cpp +++ b/clang/test/Parser/cxx2a-concept-declaration.cpp @@ -79,6 +79,11 @@ // 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 concept C17 = T{}; static_assert(!C17); template concept C18 = (bool&&)true; diff --git a/clang/test/Sema/exprs.c b/clang/test/Sema/exprs.c --- a/clang/test/Sema/exprs.c +++ b/clang/test/Sema/exprs.c @@ -212,6 +212,10 @@ // 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. @@ -223,6 +227,9 @@ // 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 --- a/clang/test/SemaCXX/expressions.cpp +++ b/clang/test/SemaCXX/expressions.cpp @@ -44,6 +44,9 @@ 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; @@ -66,6 +69,8 @@ // 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}} diff --git a/clang/test/SemaCXX/warn-unsequenced.cpp b/clang/test/SemaCXX/warn-unsequenced.cpp --- a/clang/test/SemaCXX/warn-unsequenced.cpp +++ b/clang/test/SemaCXX/warn-unsequenced.cpp @@ -76,15 +76,37 @@ (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; // ok + + (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}} + (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'}} - (1 || (a = 0)) + a; // ok + // 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}} + (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 --- a/clang/test/SemaTemplate/dependent-expr.cpp +++ b/clang/test/SemaTemplate/dependent-expr.cpp @@ -43,7 +43,9 @@ namespace PR7724 { template int myMethod() - { return 2 && sizeof(OT); } + { 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}} } namespace test4 {