Higuoxing updated this revision to Diff 151604.
Higuoxing added a comment.
Ping, thanks
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,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,78 @@
(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}} \
+ // 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}} \
+ // 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();
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 == LHSExpansionLoc) {
+ DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr);
+ }
+ }
+ }
}
if ((Opc == BO_Shl && LHSExpr->getType()->isIntegralType(Self.getASTContext()))
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
