hazohelet updated this revision to Diff 486776.
hazohelet added a comment.
This update limits the warning suppression case to string literals only, and
delete no longer necessary functions.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140860/new/
https://reviews.llvm.org/D140860
Files:
clang/lib/AST/ExprConstant.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/logical-op-parentheses.c
clang/test/Sema/logical-op-parentheses.cpp
Index: clang/test/Sema/logical-op-parentheses.cpp
===================================================================
--- clang/test/Sema/logical-op-parentheses.cpp
+++ /dev/null
@@ -1,66 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -DSILENCE
-// RUN: %clang_cc1 -fsyntax-only -verify %s -Wlogical-op-parentheses
-// RUN: %clang_cc1 -fsyntax-only -verify %s -Wparentheses
-// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s -Wlogical-op-parentheses 2>&1 | FileCheck %s
-
-#ifdef SILENCE
-// expected-no-diagnostics
-#endif
-
-void logical_op_parentheses(unsigned i) {
- constexpr bool t = 1;
- (void)(i ||
- i && i);
-#ifndef SILENCE
- // expected-warning@-2 {{'&&' within '||'}}
- // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
-#endif
- // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
- // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:20-[[@LINE-6]]:20}:")"
-
- static_assert(t ||
- t && t);
-#ifndef SILENCE
- // expected-warning@-2 {{'&&' within '||'}}
- // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
-#endif
- // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
- // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:20-[[@LINE-6]]:20}:")"
-
- static_assert(t &&
- t || t);
-#ifndef SILENCE
- // expected-warning@-3 {{'&&' within '||'}}
- // expected-note@-4 {{place parentheses around the '&&' expression to silence this warning}}
-#endif
- // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:17-[[@LINE-6]]:17}:"("
- // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:15-[[@LINE-6]]:15}:")"
-
- (void)(i || i && "w00t");
- (void)("w00t" && i || i);
- (void)("w00t" && t || t);
- static_assert(t && t || 0);
- static_assert(1 && t || t);
- static_assert(0 || t && t);
- static_assert(t || t && 1);
-
- (void)(i || i && "w00t" || i);
-#ifndef SILENCE
- // expected-warning@-2 {{'&&' within '||'}}
- // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
-#endif
- // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
- // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:26-[[@LINE-6]]:26}:")"
-
- (void)(i || "w00t" && i || i);
-#ifndef SILENCE
- // expected-warning@-2 {{'&&' within '||'}}
- // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
-#endif
- // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
- // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:26-[[@LINE-6]]:26}:")"
-
- (void)(i && i || 0);
- (void)(0 || i && i);
-}
-
Index: clang/test/Sema/logical-op-parentheses.c
===================================================================
--- clang/test/Sema/logical-op-parentheses.c
+++ clang/test/Sema/logical-op-parentheses.c
@@ -40,9 +40,33 @@
(void)("w00t" && i || i);
(void)("w00t" && t || t);
(void)(t && t || 0);
+#ifndef SILENCE
+ // expected-warning@-2 {{'&&' within '||'}}
+ // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:")"
(void)(1 && t || t);
+#ifndef SILENCE
+ // expected-warning@-2 {{'&&' within '||'}}
+ // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:")"
(void)(0 || t && t);
+#ifndef SILENCE
+ // expected-warning@-2 {{'&&' within '||'}}
+ // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:21-[[@LINE-6]]:21}:")"
(void)(t || t && 1);
+#ifndef SILENCE
+ // expected-warning@-2 {{'&&' within '||'}}
+ // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:21-[[@LINE-6]]:21}:")"
(void)(i || i && "w00t" || i);
#ifndef SILENCE
@@ -61,5 +85,17 @@
// CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:26-[[@LINE-6]]:26}:")"
(void)(i && i || 0);
+#ifndef SILENCE
+ // expected-warning@-2 {{'&&' within '||'}}
+ // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:")"
(void)(0 || i && i);
+#ifndef SILENCE
+ // expected-warning@-2 {{'&&' within '||'}}
+ // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:21-[[@LINE-6]]:21}:")"
}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15204,46 +15204,21 @@
Bop->getSourceRange());
}
-/// Returns true if the given expression can be evaluated as a constant
-/// 'true'.
-static bool EvaluatesAsTrue(Sema &S, Expr *E) {
- bool Res;
- return !E->isValueDependent() &&
- E->EvaluateAsBooleanCondition(Res, S.getASTContext()) && Res;
-}
-
-/// Returns true if the given expression can be evaluated as a constant
-/// 'false'.
-static bool EvaluatesAsFalse(Sema &S, Expr *E) {
- bool Res;
- return !E->isValueDependent() &&
- E->EvaluateAsBooleanCondition(Res, S.getASTContext()) && !Res;
-}
-
/// Look for '&&' in the left hand of a '||' expr.
static void DiagnoseLogicalAndInLogicalOrLHS(Sema &S, SourceLocation OpLoc,
Expr *LHSExpr, Expr *RHSExpr) {
if (BinaryOperator *Bop = dyn_cast<BinaryOperator>(LHSExpr)) {
if (Bop->getOpcode() == BO_LAnd) {
- // If it's "a && b || 0" don't warn since the precedence doesn't matter.
- if (isa<IntegerLiteral, FloatingLiteral, CharacterLiteral,
- CXXBoolLiteralExpr, CXXNullPtrLiteralExpr, FixedPointLiteral,
- ImaginaryLiteral>(RHSExpr) &&
- EvaluatesAsFalse(S, RHSExpr))
- return;
- Expr *LLHS = Bop->getLHS()->IgnoreParenImpCasts();
- // If it's "1 && a || b" don't warn since the precedence doesn't matter.
- if (isa<IntegerLiteral, FloatingLiteral, CharacterLiteral,
- CXXBoolLiteralExpr, CXXNullPtrLiteralExpr, FixedPointLiteral,
- ImaginaryLiteral, StringLiteral, CompoundLiteralExpr>(LLHS) &&
- EvaluatesAsTrue(S, LLHS))
- return;
- return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop);
+ // If it's "string_literal && a || b" don't warn since the precedence
+ // doesn't matter.
+ if (!isa<StringLiteral>(Bop->getLHS()->IgnoreParenImpCasts()))
+ return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop);
} else if (Bop->getOpcode() == BO_LOr) {
if (BinaryOperator *RBop = dyn_cast<BinaryOperator>(Bop->getRHS())) {
- // If it's "a || b && 1 || c" we didn't warn earlier for
- // "a || b && 1", but warn now.
- if (RBop->getOpcode() == BO_LAnd && EvaluatesAsTrue(S, RBop->getRHS()))
+ // If it's "a || b && string_literal || c" we didn't warn earlier for
+ // "a || b && string_literal", but warn now.
+ if (RBop->getOpcode() == BO_LAnd &&
+ isa<StringLiteral>(RBop->getRHS()->IgnoreParenImpCasts()))
return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, RBop);
}
}
@@ -15255,20 +15230,10 @@
Expr *LHSExpr, Expr *RHSExpr) {
if (BinaryOperator *Bop = dyn_cast<BinaryOperator>(RHSExpr)) {
if (Bop->getOpcode() == BO_LAnd) {
- // If it's "0 || a && b" don't warn since the precedence doesn't matter.
- if (isa<IntegerLiteral, FloatingLiteral, CharacterLiteral,
- CXXBoolLiteralExpr, CXXNullPtrLiteralExpr, FixedPointLiteral,
- ImaginaryLiteral>(LHSExpr) &&
- EvaluatesAsFalse(S, LHSExpr))
- return;
- // If it's "a || b && 1" don't warn since the precedence doesn't matter.
- Expr *RRHS = Bop->getRHS()->IgnoreParenImpCasts();
- if (isa<IntegerLiteral, FloatingLiteral, CharacterLiteral,
- CXXBoolLiteralExpr, CXXNullPtrLiteralExpr, FixedPointLiteral,
- ImaginaryLiteral, StringLiteral, CompoundLiteralExpr>(RRHS) &&
- EvaluatesAsTrue(S, RRHS))
- return;
- return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop);
+ // If it's "a || b && string_literal" don't warn since the precedence
+ // doesn't matter.
+ if (!isa<StringLiteral>(Bop->getRHS()->IgnoreParenImpCasts()))
+ return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop);
}
}
}
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -15234,10 +15234,6 @@
assert(!isValueDependent() &&
"Expression evaluator can't be called on a dependent expression.");
ExprTimeTraceScope TimeScope(this, Ctx, "EvaluateAsBooleanCondition");
- if (isa<StringLiteral>(this)) {
- Result = true;
- return true;
- }
EvalResult Scratch;
return EvaluateAsRValue(Scratch, Ctx, InConstantContext) &&
HandleConversionToBool(Scratch.Val, Result);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
