Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -450,6 +450,8 @@ - Clang now diagnoses overflow undefined behavior in a constant expression while evaluating a compound assignment with remainder as operand. - Add ``-Wreturn-local-addr``, a GCC alias for ``-Wreturn-stack-address``. +- Clang now suppresses ``-Wlogical-op-parentheses`` on ``(x && a || b)`` and ``(a || b && x)`` + only when ``x`` is a string literal. Non-comprehensive list of changes in this release ------------------------------------------------- Index: clang/lib/Sema/SemaExpr.cpp =================================================================== --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -15434,38 +15434,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 (EvaluatesAsFalse(S, RHSExpr)) - return; - // If it's "1 && a || b" don't warn since the precedence doesn't matter. - if (!EvaluatesAsTrue(S, Bop->getLHS())) + // 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); } } @@ -15477,11 +15460,9 @@ 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 (EvaluatesAsFalse(S, LHSExpr)) - return; - // If it's "a || b && 1" don't warn since the precedence doesn't matter. - if (!EvaluatesAsTrue(S, Bop->getRHS())) + // 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 @@ -8,6 +8,7 @@ #endif void logical_op_parentheses(unsigned i) { + const unsigned t = 1; (void)(i || i && i); #ifndef SILENCE @@ -17,8 +18,55 @@ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"(" // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:20-[[@LINE-6]]:20}:")" - (void)(i || i && "w00t"); + (void)(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}:")" + + (void)(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]]:10-[[@LINE-6]]:10}:"(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:15-[[@LINE-6]]:15}:")" + + (void)(i || i && "w00t"); (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 @@ -37,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}:")" }