Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -7103,9 +7103,11 @@ static void SuggestParentheses(Sema &Self, SourceLocation Loc, const PartialDiagnostic &Note, SourceRange ParenRange) { - SourceLocation EndLoc = Self.getLocForEndOfToken(ParenRange.getEnd()); - if (ParenRange.getBegin().isFileID() && ParenRange.getEnd().isFileID() && - EndLoc.isValid()) { + SourceManager &SM = Self.getSourceManager(); + SourceLocation spellLoc = SM.getSpellingLoc(ParenRange.getEnd()); + unsigned tokLen = Lexer::MeasureTokenLength(spellLoc, SM, Self.getLangOpts()); + SourceLocation EndLoc = spellLoc.getLocWithOffset(tokLen); + if (EndLoc.isValid()) { Self.Diag(Loc, Note) << FixItHint::CreateInsertion(ParenRange.getBegin(), "(") << FixItHint::CreateInsertion(EndLoc, ")"); @@ -12331,6 +12333,34 @@ } } +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); + return; + } + + // Diagnose parentheses, if and only if operator and its LHS, RHS + // are from the same argument position of first level macros + SourceLocation OpExpansionLoc; + if (!SM.isMacroArgExpansion(OpLoc, &OpExpansionLoc) || + SM.getImmediateMacroCallerLoc(OpLoc).isMacroID()) + return; + + SourceLocation ExprExpansionLoc; + if (SM.isMacroArgExpansion(LHSExpr->getExprLoc(), &ExprExpansionLoc) && + OpExpansionLoc == ExprExpansionLoc && + !SM.getImmediateMacroCallerLoc(LHSExpr->getExprLoc()).isMacroID()) + DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr); + + if (SM.isMacroArgExpansion(RHSExpr->getExprLoc(), &ExprExpansionLoc) && + OpExpansionLoc == ExprExpansionLoc && + !SM.getImmediateMacroCallerLoc(RHSExpr->getExprLoc()).isMacroID()) + 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. @@ -12407,10 +12437,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,7 +14,11 @@ if ((i = 4)) {} } -void bitwise_rel(unsigned i) { +#define VOID_CAST(cond) ( (void)(cond) ) +#define APPLY_OPS(op0, op1, x, y, z) ( VOID_CAST(x op0 y op1 z) ) +#define APPLY_OPS_DIRECTLY(op0, op1, x, y, z) ( (void)(x op0 y op1 z) ) + +void bitwise_rel(unsigned i, unsigned iiii) { (void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} \ // expected-note{{place parentheses around the '==' expression to silence this warning}} \ // expected-note{{place parentheses around the & expression to evaluate it first}} @@ -96,6 +100,31 @@ (void)(i && i || 0); // no warning. (void)(0 || i && i); // no warning. + + VOID_CAST(i && i || i); // expected-warning {{'&&' within '||'}} \ + // expected-note {{place parentheses around the '&&' expression to silence this warning}} + + APPLY_OPS_DIRECTLY(&&, ||, i, i, i); // no warning. + APPLY_OPS(&&, ||, i, i, i); // no warning. + + APPLY_OPS_DIRECTLY(&&, ||, i, i, i || i && i); // expected-warning {{'&&' within '||'}} \ + // expected-note {{place parentheses around the '&&' expression to silence this warning}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:41-[[@LINE-2]]:41}:"(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:47-[[@LINE-3]]:47}:")" + + APPLY_OPS_DIRECTLY(&&, ||, i, i, i || i && iiii); // expected-warning {{'&&' within '||'}} \ + // expected-note {{place parentheses around the '&&' expression to silence this warning}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:41-[[@LINE-2]]:41}:"(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:50-[[@LINE-3]]:50}:")" + + APPLY_OPS(&&, ||, i, i, i || i && i); // no warning. + + APPLY_OPS_DIRECTLY(&&, ||, i, i, i && i); // no warning. + APPLY_OPS_DIRECTLY(&&, ||, i, i, i || i); // no warning. + + APPLY_OPS(&&, ||, i, i, i && i); // no warning. + APPLY_OPS(&&, ||, i, i, i || i); // no warning. + } _Bool someConditionFunc();