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 @@ -13593,18 +13593,21 @@ 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() && + // bitwise one. We do this when one of the operand is a non-bool integer and + // the other is a constant. + if (!EnumConstantInBoolContext && + ((RHS.get()->getType()->isIntegerType() && !RHS.get()->getType()->isBooleanType() && + LHS.get()->getType()->isIntegerType() && !LHS.get()->isValueDependent()) || + ( RHS.get()->getType()->isIntegerType() && !RHS.get()->getType()->isBooleanType() && + LHS.get()->getType()->isIntegerType() && !LHS.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 operand 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 operand are ignored. Expr::EvalResult EVResult; + if (RHS.get()->EvaluateAsInt(EVResult, Context)) { llvm::APSInt Result = EVResult.Val.getInt(); if ((getLangOpts().Bool && !RHS.get()->getType()->isBooleanType() && @@ -13626,6 +13629,28 @@ RHS.get()->getEndLoc())); } } + + if (LHS.get()->EvaluateAsInt(EVResult, Context)) { + llvm::APSInt Result = EVResult.Val.getInt(); + if ((getLangOpts().Bool && !LHS.get()->getType()->isBooleanType() && + !LHS.get()->getExprLoc().isMacroID()) || + (Result != 0 && Result != 1)) { + Diag(Loc, diag::warn_logical_instead_of_bitwise) + << LHS.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 "kNonZero && foo() " with "Foo()" + Diag(Loc, diag::note_logical_instead_of_bitwise_remove_constant) + << FixItHint::CreateRemoval( + SourceRange(getLocForEndOfToken(RHS.get()->getEndLoc()), + LHS.get()->getEndLoc())); + } + } } if (!Context.getLangOpts().CPlusPlus) { 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}}