Higuoxing updated this revision to Diff 151654. Higuoxing marked an inline comment as done.
https://reviews.llvm.org/D47687 Files: lib/Sema/SemaExpr.cpp test/Sema/parentheses.c Index: test/Sema/parentheses.c =================================================================== --- test/Sema/parentheses.c +++ test/Sema/parentheses.c @@ -14,6 +14,10 @@ if ((i = 4)) {} } +#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) { (void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} \ // expected-note{{place parentheses around the '==' expression to silence this warning}} \ @@ -96,6 +100,13 @@ (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}} + VOID_CAST((i && i) || i); // no warning. + + APPLY_OPS_DIRECTLY(&&, ||, i, i, i); // no warning. + APPLY_OPS(&&, ||, i, i, i); // no warning. } _Bool someConditionFunc(); Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -12233,6 +12233,35 @@ } } +/// 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); + return; + } else { + // 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) && + !SM.getImmediateMacroCallerLoc(LHSExpr->getExprLoc()).isMacroID() && + OpExpansionLoc == ExprExpansionLoc) + DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr); + + if (SM.isMacroArgExpansion(RHSExpr->getExprLoc(), &ExprExpansionLoc) && + !SM.getImmediateMacroCallerLoc(LHSExpr->getExprLoc()).isMacroID() && + 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 +12339,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,10 @@ if ((i = 4)) {} } +#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) { (void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} \ // expected-note{{place parentheses around the '==' expression to silence this warning}} \ @@ -96,6 +100,13 @@ (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}} + VOID_CAST((i && i) || i); // no warning. + + APPLY_OPS_DIRECTLY(&&, ||, i, i, i); // no warning. + APPLY_OPS(&&, ||, i, i, i); // no warning. } _Bool someConditionFunc(); Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -12233,6 +12233,35 @@ } } +/// 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); + return; + } else { + // 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) && + !SM.getImmediateMacroCallerLoc(LHSExpr->getExprLoc()).isMacroID() && + OpExpansionLoc == ExprExpansionLoc) + DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr); + + if (SM.isMacroArgExpansion(RHSExpr->getExprLoc(), &ExprExpansionLoc) && + !SM.getImmediateMacroCallerLoc(LHSExpr->getExprLoc()).isMacroID() && + 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 +12339,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) {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits