Higuoxing updated this revision to Diff 151633.

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,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();
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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to