Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td +++ include/clang/Basic/DiagnosticGroups.td @@ -261,6 +261,8 @@ def GlobalConstructors : DiagGroup<"global-constructors">; def BitwiseOpParentheses: DiagGroup<"bitwise-op-parentheses">; def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">; +def LogicalOpParenthesesInMacros: DiagGroup<"logical-op-parentheses-in-macros", + [LogicalOpParentheses]>; def LogicalNotParentheses: DiagGroup<"logical-not-parentheses">; def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">; def OverloadedShiftOpParentheses: DiagGroup<"overloaded-shift-op-parentheses">; Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -5483,6 +5483,10 @@ def warn_logical_and_in_logical_or : Warning< "'&&' within '||'">, InGroup; +def warn_logical_and_in_logical_or_in_macros: + Warning, + InGroup, DefaultIgnore; + def warn_overloaded_shift_in_comparison :Warning< "overloaded operator %select{>>|<<}0 has higher precedence than " "comparison operator">, Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -12172,8 +12172,16 @@ EmitDiagnosticForLogicalAndInLogicalOr(Sema &Self, SourceLocation OpLoc, BinaryOperator *Bop) { assert(Bop->getOpcode() == BO_LAnd); - Self.Diag(Bop->getOperatorLoc(), diag::warn_logical_and_in_logical_or) - << Bop->getSourceRange() << OpLoc; + Self.Diag(Bop->getOperatorLoc(), !OpLoc.isMacroID() ? + // if this warning is in a normal context + diag::warn_logical_and_in_logical_or : + // else this warning is in a macro context + // currently, this will not warn in macros by default. + // add [-Wlogical-op-parentheses-in-macros] + // to enable this warning. + diag::warn_logical_and_in_logical_or_in_macros + ) << Bop->getSourceRange() << OpLoc; + SuggestParentheses(Self, Bop->getOperatorLoc(), Self.PDiag(diag::note_precedence_silence) << Bop->getOpcodeStr(), @@ -12205,6 +12213,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 +12236,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,8 +12319,11 @@ } // 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. */) { + // 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) { DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr); DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr); } 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,28 @@ +// RUN: %clang_cc1 -fsyntax-only -verify=silence %s +// RUN: %clang_cc1 -Wparentheses -fsyntax-only -verify=silence %s +// RUN: %clang_cc1 -Wlogical-op-parentheses-in-macros -fsyntax-only \ +// RUN: -verify=logical-op-parentheses %s +// RUN: %clang_cc1 -Wlogical-op-parentheses -fsyntax-only \ +// RUN: -verify=silence %s + +#define macro_op_parentheses_check(x) ( \ + ( (void)(x) ) \ +) + +// silence-no-diagnostics +void logical_op_in_macros_test(unsigned i) { + macro_op_parentheses_check(i || i && i); // logical-op-parentheses-warning {{'&&' within '||'}} \ + // logical-op-parentheses-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-warning {{'&&' within '||'}} \ + // logical-op-parentheses-note {{place parentheses around the '&&' expression to silence this warning}} + + macro_op_parentheses_check(i || "I love LLVM" && i || i); // logical-op-parentheses-warning {{'&&' within '||'}} \ + // logical-op-parentheses-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. +}