Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -12310,9 +12310,30 @@ // 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); + if (Opc == BO_LOr) { + if (!OpLoc.isMacroID()) { + // Operator is not in macros + 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(OpLoc, + &OpExpansionLoc) && + Self.getSourceManager().isMacroArgExpansion(RHSExpr->getExprLoc(), + &LHSExpansionLoc))) { + if (OpExpansionLoc == LHSExpansionLoc) { + DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr); + } + if (OpExpansionLoc == RHSExpansionLoc) { + DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr); + } + } + } } if ((Opc == BO_Shl && LHSExpr->getType()->isIntegralType(Self.getASTContext())) Index: test/Sema/parentheses.c =================================================================== --- test/Sema/parentheses.c +++ test/Sema/parentheses.c @@ -14,6 +14,15 @@ if ((i = 4)) {} } +// testing macros +// nesting macros testing +#define NESTING_VOID(cond) ( (void)(cond) ) +#define NESTING_VOID_WRAPPER(op0, op1, x, y, z) ( (void)(x op0 y op1 z) ) + +// non-nesting macros +#define NON_NESTING_VOID_0(cond) ( (void)(cond) ) +#define NON_NESTING_VOID_1(op0, op1, x, y, z) ( (void)(x op0 y op1 z) ) + void bitwise_rel(unsigned i) { (void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} \ // expected-note{{place parentheses around the '==' expression to silence this warning}} \ @@ -96,6 +105,74 @@ (void)(i && i || 0); // no warning. (void)(0 || i && i); // no warning. + + //===---------------------------------------------------------------------- + // Logical operator in macros + //===---------------------------------------------------------------------- + + // actionable expression tests + NON_NESTING_VOID_0(i && i || i); // expected-warning {{'&&' within '||'}} \ + // expected-note {{place parentheses around the '&&' expression to silence this warning}} + NON_NESTING_VOID_0((i && i) || i); // no warning. + + // NON_NESTING_VOID_1(&&, ||, i && i || i, i, i); + // will be expanded to: + // i && i || i && i || i + // ^---------^ arg position 2 (i && i || i) should be checked because `op ||` and `lhs (i && i)` from same arg position + // ^ arg position 0 (&&) + // ^ arg position 3 (i) + // ^ arg position 1 (||) should not be checked because `op ||` and nothing from same arg position + // ^ arg position 4 (i) + NON_NESTING_VOID_1(&&, ||, i && i || i, i, i); // expected-warning {{'&&' within '||'}} \ + // expected-note {{place parentheses around the '&&' expression to silence this warning}} + NON_NESTING_VOID_1(&&, ||, ((i && i) || i), i, i); // no warning. + + // same as this one + NESTING_VOID_WRAPPER(&&, ||, i && i || i, i, i); // expected-warning {{'&&' within '||'}} \ + // expected-note {{place parentheses around the '&&' expression to silence this warning}} + NESTING_VOID_WRAPPER(&&, ||, ((i && i) || i), i, i); // no waring. + + // NON_NESTING_VOID_1(&&, ||, i, i && i || i, i); + // will be expanded to: + // i && i && i || i || i + // ^ arg position 2 (i) + // ^ arg position 0 (&&) + // ^---------^ arg position 3 (i && i || i) should be checked because `op ||` and `lhs i && i` from same arg position + // ^ arg position 1 (||) should not be checked because `op ||` and nothing from same arg postion + // ^ arg position 4 (i) + NON_NESTING_VOID_1(&&, ||, i, i && i || i, i); // expected-warning {{'&&' within '||'}} \ + // expected-note {{place parentheses around the '&&' expression to silence this warning}} + NON_NESTING_VOID_1(&&, ||, i, (i && i) || i, i); // no warning. + + // same as this one + NESTING_VOID_WRAPPER(&&, ||, i, i && i || i, i); // expected-warning {{'&&' within '||'}} \ + // expected-note {{place parentheses around the '&&' expression to silence this warning}} + NESTING_VOID_WRAPPER(&&, ||, i, (i && i) || i, i); // no warning. + + // un-actionable expression tests + // NON_NESTING_VOID_1(&&, ||, i, i, i); + // will be expanded to: + // i && i || i + // ^ arg position 2 (i) + // ^ arg position 0 (&&) + // ^ arg position 3 (||) should not be checked becaues `op ||` and nothing from same arg position + // ^ arg position 1 (i) + // ^ arg position 4 (i) + NON_NESTING_VOID_1(&&, ||, i, i, i); // no warning. + NESTING_VOID_WRAPPER(&&, ||, i, i, i); // no warning. + + // NON_NESTING_VOID_1(&&, ||, i, i, i && i || i); + // will be expanded to: + // i && i || i && i || i + // ^ arg positon 2 (i) + // ^ arg position 0 (&&) + // ^ arg position 3 (i) + // ^ arg position 1 (||) should not be checked because `op ||` and nothing from same arg position + // ^---------^ arg position 4 (i && i || i) should not be checked because `op ||` and nothing from same arg position + NON_NESTING_VOID_1(&&, ||, i, i, i && i || i); // no warning. + NESTING_VOID_WRAPPER(&&, ||, i, i, i && i || i); // no warning. + + // same for something expression like (i || i && i); } _Bool someConditionFunc();