Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -12233,6 +12233,31 @@ } } +/// Look for '&&' in the righ or left hand of a '||' expr +static void DiagnoseLogicalAndInLogicalOr(Sema &Self, SourceLocation OpLoc, + Expr *LHSExpr, Expr *RHSExpr) { + SourceManager &SM = Self.getSourceManager(); + if (!OpLoc.isMacroID()) { + DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr); + DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr); + } else { + SourceLocation OpExpansionLoc; + if (!SM.isMacroArgExpansion(OpLoc, &OpExpansionLoc)) + return; + + SourceLocation ExprExpansionLoc; + // LHS and operator are from same arg position of macro function + if (SM.isMacroArgExpansion(LHSExpr->getExprLoc(), &ExprExpansionLoc) && + OpExpansionLoc == ExprExpansionLoc) + DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr); + + // RHS and operator are from same arg position of macro function + if (SM.isMacroArgExpansion(RHSExpr->getExprLoc(), &ExprExpansionLoc) && + OpExpansionLoc == ExprExpansionLoc) + DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr); + } +} + /// Look for bitwise op in the left or right hand of a bitwise op with /// lower precedence and emit a diagnostic together with a fixit hint that wraps /// the '&' expression in parentheses. @@ -12310,10 +12335,8 @@ // 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) + DiagnoseLogicalAndInLogicalOr(Self, OpLoc, LHSExpr, RHSExpr); if ((Opc == BO_Shl && LHSExpr->getType()->isIntegralType(Self.getASTContext())) || Opc == BO_Shr) { 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,115 @@ (void)(i && i || 0); // no warning. (void)(0 || i && i); // no warning. + + //===---------------------------------------------------------------------- + // Logical operator in macros + //===---------------------------------------------------------------------- + + 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); + // 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 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 warning. + + // 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. + + // 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. + + // 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 not be checked because `op ||` and its rhs are not from same arg position + // ^ arg position 0 (&&) + // ^ arg position 3 (i) + // ^ arg position 1 (||) should not be checked because `op ||` and its lhs are not from same arg position + // ^ arg position 4 (i) + NON_NESTING_VOID_1(&&, ||, i || i && i, i, i); // no warning. + NESTING_VOID_WRAPPER(&&, ||, i || i && 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) + // ^ arg position 0 (&&) + // ^---------^ arg position 3 (i || i && i) should be checked because `op ||` and its rhs are from same arg position + // ^ arg position 1 (||) shoud not be checked because `op ||` and its lhs are not 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 warning. + + // 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) + // ^ arg position 1 (||) should not be checked because `op ||` and its rhs are not from same arg position + // ^----------^ arg position 4 (i || i && i) should be checked because `op ||` and its rhs are from same arg position + 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. } _Bool someConditionFunc();