Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -12172,7 +12172,7 @@ EmitDiagnosticForLogicalAndInLogicalOr(Sema &Self, SourceLocation OpLoc, BinaryOperator *Bop) { assert(Bop->getOpcode() == BO_LAnd); - Self.Diag(Bop->getOperatorLoc(), diag::warn_logical_and_in_logical_or) + Self.Diag(Bop->getOperatorLoc(), diag::warn_logical_and_in_logical_or) << Bop->getSourceRange() << OpLoc; SuggestParentheses(Self, Bop->getOperatorLoc(), Self.PDiag(diag::note_precedence_silence) @@ -12205,6 +12205,7 @@ if (EvaluatesAsFalse(S, RHSExpr)) return; // If it's "1 && a || b" don't warn since the precedence doesn't matter. + // And 'assert("some message" && a || b)' don't warn as well. if (!EvaluatesAsTrue(S, Bop->getLHS())) return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop); } else if (Bop->getOpcode() == BO_LOr) { @@ -12227,6 +12228,7 @@ if (EvaluatesAsFalse(S, LHSExpr)) return; // If it's "a || b && 1" don't warn since the precedence doesn't matter. + // And 'assert(a || b && "some message")' don't warn as well. if (!EvaluatesAsTrue(S, Bop->getRHS())) return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop); } @@ -12309,10 +12311,29 @@ } // Warn about arg1 || arg2 && arg3, as GCC 4.3+ does. - // We don't warn for 'assert(a || b && "bad")' since this is safe. - if (Opc == BO_LOr && !OpLoc.isMacroID()/* Don't warn in macros. */) { - DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr); - DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr); + // Here we will not skip 'logical and in logical or' checking + // in macros, since 'assert(a || b && "some message")' is equal + // to '(a || b && 1)' and 'assert("some message" && a || b)' is + // equal to '(1 && a || b)'. + if (Opc == BO_LOr){ + if (!OpLoc.isMacroID()) { + DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr); + DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr); + } else { + // This Expression is expanded from macro + SourceLocation LHSExpansionLoc, OpExpansionLoc, RHSExpansionLoc; + if (Self.getSourceManager().isMacroArgExpansion(OpLoc, + &OpExpansionLoc) && + Self.getSourceManager().isMacroArgExpansion(LHSExpr->getExprLoc(), + &LHSExpansionLoc) && + Self.getSourceManager().isMacroArgExpansion(RHSExpr->getExprLoc(), + &RHSExpansionLoc)) { + if (OpExpansionLoc == LHSExpansionLoc && OpExpansionLoc == RHSExpansionLoc) { + DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr); + DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr); + } + } + } } if ((Opc == BO_Shl && LHSExpr->getType()->isIntegralType(Self.getASTContext())) Index: test/Sema/logical-op-parentheses-in-macros.c =================================================================== --- test/Sema/logical-op-parentheses-in-macros.c +++ test/Sema/logical-op-parentheses-in-macros.c @@ -0,0 +1,51 @@ +// RUN: %clang_cc1 -Wparentheses -fsyntax-only -verify=logical_op_parentheses_check %s + +// operator of this case is expanded from macro function argument +#define macro_op_parentheses_check(x) ( \ + ( (void)(x) ) \ +) + +// operator of this case is expanded from macro function body +#define macro_op_parentheses_check_ops_args(op0, op1, x, y, z) ( \ + ( (void) (x op0 y op1 z ) ) \ +) + +// operator of this case is expanded from macro function body +#define macro_op_parentheses_check_ops_args2(op0, op1, op2, w, x, y, z) ( \ + ( (void) (w op0 x op1 y op2 z) ) \ +) + +// operator of this case is expanded from marco body +#define macro_op_parentheses_check_ops_body(x, y, z) ( \ + ( (void) (x && y || z) ) \ +) + +void logical_op_from_macro_arguments(unsigned i) { + macro_op_parentheses_check(i || i && i); // logical_op_parentheses_check-warning {{'&&' within '||'}} \ + // logical_op_parentheses_check-note {{place parentheses around the '&&' expression to silence this warning}} + macro_op_parentheses_check(i || i && "I love LLVM"); // no warning. + macro_op_parentheses_check("I love LLVM" && i || i); // no warning. + + macro_op_parentheses_check(i || i && "I love LLVM" || i); // logical_op_parentheses_check-warning {{'&&' within '||'}} \ + // logical_op_parentheses_check-note {{place parentheses around the '&&' expression to silence this warning}} + macro_op_parentheses_check(i || "I love LLVM" && i || i); // logical_op_parentheses_check-warning {{'&&' within '||'}} \ + // logical_op_parentheses_check-note {{place parentheses around the '&&' expression to silence this warning}} + macro_op_parentheses_check(i && i || 0); // no warning. + macro_op_parentheses_check(0 || i && i); // no warning. +} + +void logical_op_from_macro_arguments2(unsigned i) { + macro_op_parentheses_check_ops_args(||, &&, i, i, i); // no warning. + macro_op_parentheses_check_ops_args(||, &&, i, i, "I love LLVM"); // no warning. + macro_op_parentheses_check_ops_args(&&, ||, "I love LLVM", i, i); // no warning. + + macro_op_parentheses_check_ops_args2(||, &&, ||, i, i, "I love LLVM", i); // no warning. + macro_op_parentheses_check_ops_args2(||, &&, ||, i, "I love LLVM", i, i); // no warning. + + macro_op_parentheses_check_ops_args(&&, ||, i, i, 0); // no warning. + macro_op_parentheses_check_ops_args(||, &&, 0, i, i); // no warning. +} + +void logical_op_from_macro_body(unsigned i) { + macro_op_parentheses_check_ops_body(i, i, i); // no warning. +} \ No newline at end of file