[PATCH] D47687: [Sema] Missing -Wlogical-op-parentheses warnings in macros (PR18971)

[Xing via Phabricator via cfe-commits]

Higuoxing updated this revision to Diff 151599.

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,33 @@
 
   (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); // 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.
+
+  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.
+
+  // FIXME: strange things, but I think this could be tolerated
+  NON_NESTING_VOID_1(&&, ||, i && i || i, i, i); // no warning. but expected. because op and LHS are not from same arg position
+  NON_NESTING_VOID_1(&&, ||, i, i, i && i || i); // no warning. but expected. because op and RHS are not from same arg position
+  NESTING_VOID_WRAPPER(&&, ||, i && i || i, i, i); // no warning. but expected. because op and LHS are not from same arg position
+  NESTING_VOID_WRAPPER(&&, ||, i, i, i && i || i); // no warning. but expected. because op and RHS are not from same arg position
+
+  // un-actionable expression tests
+  NESTING_VOID_WRAPPER(&&, ||, i, i, i); // no warning.
+  NON_NESTING_VOID_1(&&, ||, i, i, i);   // no warning.
 }
 
 _Bool someConditionFunc();
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -12310,9 +12310,26 @@
 
   // 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(RHSExpr->getExprLoc(), 
+                                                      &RHSExpansionLoc)) {
+        if (OpExpansionLoc == LHSExpansionLoc && OpExpansionLoc == RHSExpansionLoc) {
+          DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr);
+          DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr);
+        }
+      }
+    }
   }
 
   if ((Opc == BO_Shl && LHSExpr->getType()->isIntegralType(Self.getASTContext()))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D47687.151599.patch
Type: text/x-patch
Size: 4404 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180616/71509d0a/attachment.bin>