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

[Xing via Phabricator via cfe-commits]

Higuoxing updated this revision to Diff 151736.
Higuoxing added a comment.

test case has been passed


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,22 @@
 
   (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}}
+  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();
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 side 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;
+  }
+
+  // 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.
@@ -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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D47687.151736.patch
Type: text/x-patch
Size: 3854 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180618/cf3d7dc5/attachment-0001.bin>