Skip to content

PR for llvm/llvm-project#64515 #546

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -12694,8 +12694,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);
Expand Down
95 changes: 37 additions & 58 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
Expand All @@ -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) {
Expand Down
4 changes: 1 addition & 3 deletions clang/test/C/drs/dr4xx.c
Original file line number Diff line number Diff line change
Expand Up @@ -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}} */
}
}

Expand Down
6 changes: 0 additions & 6 deletions clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
5 changes: 0 additions & 5 deletions clang/test/Parser/cxx2a-concept-declaration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 0 additions & 7 deletions clang/test/Sema/exprs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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}} \
Expand Down
8 changes: 2 additions & 6 deletions clang/test/SemaCXX/expressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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}}
Expand Down Expand Up @@ -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}}
}
26 changes: 2 additions & 24 deletions clang/test/SemaCXX/warn-unsequenced.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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'}}
Expand Down
4 changes: 1 addition & 3 deletions clang/test/SemaTemplate/dependent-expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down