xgupta updated this revision to Diff 495375.
xgupta added a comment.
address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142609/new/
https://reviews.llvm.org/D142609
Files:
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
Index: clang/test/SemaTemplate/dependent-expr.cpp
===================================================================
--- clang/test/SemaTemplate/dependent-expr.cpp
+++ clang/test/SemaTemplate/dependent-expr.cpp
@@ -43,7 +43,9 @@
namespace PR7724 {
template<typename OT> 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 {
Index: clang/test/SemaCXX/warn-unsequenced.cpp
===================================================================
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ 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'}}
Index: clang/test/SemaCXX/expressions.cpp
===================================================================
--- clang/test/SemaCXX/expressions.cpp
+++ 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}}
Index: clang/test/Sema/exprs.c
===================================================================
--- clang/test/Sema/exprs.c
+++ 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}} \
Index: clang/test/Parser/cxx2a-concept-declaration.cpp
===================================================================
--- clang/test/Parser/cxx2a-concept-declaration.cpp
+++ 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<typename T> concept C17 = T{};
static_assert(!C17<bool>);
template<typename T> concept C18 = (bool&&)true;
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp
===================================================================
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp
+++ 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}}
Index: clang/test/C/drs/dr4xx.c
===================================================================
--- clang/test/C/drs/dr4xx.c
+++ 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}} */
}
}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ 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<DeclRefExpr>(HS.get())) {
- const auto *ECDHS = dyn_cast<EnumConstantDecl>(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<DeclRefExpr>(HS.get())) {
+ const auto *ECDHS = dyn_cast<EnumConstantDecl>(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
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ 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);
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
