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(this)) { - Result = true; - return true; - } EvalResult Scratch; return EvaluateAsRValue(Scratch, Ctx, InConstantContext) && HandleConversionToBool(Scratch.Val, Result); 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(LHSExpr)) { if (Bop->getOpcode() == BO_LAnd) { - // If it's "a && b || 0" don't warn since the precedence doesn't matter. - if (isa(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(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(Bop->getLHS()->IgnoreParenImpCasts())) + return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop); } else if (Bop->getOpcode() == BO_LOr) { if (BinaryOperator *RBop = dyn_cast(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(RBop->getRHS()->IgnoreParenImpCasts())) return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, RBop); } } @@ -15255,20 +15230,10 @@ Expr *LHSExpr, Expr *RHSExpr) { if (BinaryOperator *Bop = dyn_cast(RHSExpr)) { if (Bop->getOpcode() == BO_LAnd) { - // If it's "0 || a && b" don't warn since the precedence doesn't matter. - if (isa(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(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(Bop->getRHS()->IgnoreParenImpCasts())) + return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop); } } } 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/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); -} -