[clang] a845252 - Revert "[Clang] Fix -Wconstant-logical-operand when LHS is a constant"
Shivam Gupta via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 7 19:30:44 PDT 2023
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 at -1 {{use of logical '&&' with constant operand}}
- // expected-note at -2 {{use '&' for a bitwise operation}}
- // expected-note at -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 at -1 {{use of logical '||' with constant operand}}
- // expected-note at -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 at -1{{use of logical '&&' with constant operand}}
// expected-note at -2{{use '&' for a bitwise operation}}
// expected-note at -3{{remove constant to silence this warning}}
-// expected-warning at -4{{use of logical '&&' with constant operand}}
-// expected-note at -5{{use '&' for a bitwise operation}}
-// expected-note at -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 at -1 {{unsequenced modification and access to 'a'}}
-
- (0 && (a = 0)) + a; // cxx11-warning {{use of logical '&&' with constant operand}}
- // cxx11-note at -1 {{use '&' for a bitwise operation}}
- // cxx11-note at -2 {{remove constant to silence this warning}}
- // cxx17-warning at -3 {{use of logical '&&' with constant operand}}
- // cxx17-note at -4 {{use '&' for a bitwise operation}}
- // cxx17-note at -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 at -1 {{unsequenced modification and access to 'a'}}
- // cxx11-warning at -2 {{use of logical '&&' with constant operand}}
- // cxx11-note at -3 {{use '&' for a bitwise operation}}
- // cxx11-note at -4 {{remove constant to silence this warning}}
- // cxx17-warning at -5 {{use of logical '&&' with constant operand}}
- // cxx17-note at -6 {{use '&' for a bitwise operation}}
- // cxx17-note at -7 {{remove constant to silence this warning}}
-
(xs[3] || (a = 0)) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
// cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
(0 || (a = 0)) + a; // cxx11-warning {{unsequenced modification and access to 'a'}}
// cxx17-warning at -1 {{unsequenced modification and access to 'a'}}
- // cxx11-warning at -2 {{use of logical '||' with constant operand}}
- // cxx11-note at -3 {{use '|' for a bitwise operation}}
- // cxx17-warning at -4 {{use of logical '||' with constant operand}}
- // cxx17-note at -5 {{use '|' for a bitwise operation}}
- (1 || (a = 0)) + a; // cxx11-warning {{use of logical '||' with constant operand}}
- // cxx11-note at -1 {{use '|' for a bitwise operation}}
- // cxx17-warning at -2 {{use of logical '||' with constant operand}}
- // cxx17-note at -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 at -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 {
More information about the cfe-commits
mailing list