Higuoxing updated this revision to Diff 150253.
Higuoxing added a reviewer: echristo.
Higuoxing added a comment.

update with comments & remove some useless blank lines.


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,11 @@
   if ((i = 4)) {}
 }
 
+#define macro_parentheses_check(x) \
+  ( \
+    ( (void)(x) ) \
+  )
+
 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 +101,21 @@
 
   (void)(i && i || 0); // no warning.
   (void)(0 || i && i); // no warning.
+
+  macro_parentheses_check(i || i && i); // expected-warning {{'&&' within 
'||'}} \
+                                        // expected-note {{place parentheses 
around the '&&' expression to silence this warning}}
+  
+  macro_parentheses_check(i || i && "I love LLVM"); // no warning.
+  macro_parentheses_check("I love LLVM" && i || i); // no warning.
+
+  macro_parentheses_check(i || i && "I love LLVM" || i); // expected-warning 
{{'&&' within '||'}} \
+                                                         // expected-note 
{{place parentheses around the '&&' expression to silence this warning}}
+  
+  macro_parentheses_check(i || "I love LLVM" && i || i); // expected-warning 
{{'&&' within '||'}} \
+                                                         // expected-note 
{{place parentheses around the '&&' expression to silence this warning}}
+  
+  macro_parentheses_check(i && i || 0); // no warning.
+  macro_parentheses_check(0 || i && i); // no warning.
 }
 
 _Bool someConditionFunc();
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -12205,6 +12205,7 @@
       if (EvaluatesAsFalse(S, RHSExpr))
         return;
       // If it's "1 && a || b" don't warn since the precedence doesn't matter.
+      // And 'assert("some message" && a || b)' don't warn as well.
       if (!EvaluatesAsTrue(S, Bop->getLHS()))
         return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop);
     } else if (Bop->getOpcode() == BO_LOr) {
@@ -12227,6 +12228,7 @@
       if (EvaluatesAsFalse(S, LHSExpr))
         return;
       // If it's "a || b && 1" don't warn since the precedence doesn't matter.
+      // And 'assert(a || b && "some message")' don't warn as well.
       if (!EvaluatesAsTrue(S, Bop->getRHS()))
         return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop);
     }
@@ -12309,8 +12311,11 @@
   }
 
   // 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. */) {
+  // Here we will not skip 'logical and in logical or' checking
+  // in macros, since 'assert(a || b && "some message")' is equal
+  // to '(a || b && 1)' and 'assert("some message" && a || b)' is
+  // equal to '(1 && a || b)'.
+  if (Opc == BO_LOr) {
     DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr);
     DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr);
   }


Index: test/Sema/parentheses.c
===================================================================
--- test/Sema/parentheses.c
+++ test/Sema/parentheses.c
@@ -14,6 +14,11 @@
   if ((i = 4)) {}
 }
 
+#define macro_parentheses_check(x) \
+  ( \
+    ( (void)(x) ) \
+  )
+
 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 +101,21 @@
 
   (void)(i && i || 0); // no warning.
   (void)(0 || i && i); // no warning.
+
+  macro_parentheses_check(i || i && i); // expected-warning {{'&&' within '||'}} \
+                                        // expected-note {{place parentheses around the '&&' expression to silence this warning}}
+  
+  macro_parentheses_check(i || i && "I love LLVM"); // no warning.
+  macro_parentheses_check("I love LLVM" && i || i); // no warning.
+
+  macro_parentheses_check(i || i && "I love LLVM" || i); // expected-warning {{'&&' within '||'}} \
+                                                         // expected-note {{place parentheses around the '&&' expression to silence this warning}}
+  
+  macro_parentheses_check(i || "I love LLVM" && i || i); // expected-warning {{'&&' within '||'}} \
+                                                         // expected-note {{place parentheses around the '&&' expression to silence this warning}}
+  
+  macro_parentheses_check(i && i || 0); // no warning.
+  macro_parentheses_check(0 || i && i); // no warning.
 }
 
 _Bool someConditionFunc();
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -12205,6 +12205,7 @@
       if (EvaluatesAsFalse(S, RHSExpr))
         return;
       // If it's "1 && a || b" don't warn since the precedence doesn't matter.
+      // And 'assert("some message" && a || b)' don't warn as well.
       if (!EvaluatesAsTrue(S, Bop->getLHS()))
         return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop);
     } else if (Bop->getOpcode() == BO_LOr) {
@@ -12227,6 +12228,7 @@
       if (EvaluatesAsFalse(S, LHSExpr))
         return;
       // If it's "a || b && 1" don't warn since the precedence doesn't matter.
+      // And 'assert(a || b && "some message")' don't warn as well.
       if (!EvaluatesAsTrue(S, Bop->getRHS()))
         return EmitDiagnosticForLogicalAndInLogicalOr(S, OpLoc, Bop);
     }
@@ -12309,8 +12311,11 @@
   }
 
   // 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. */) {
+  // Here we will not skip 'logical and in logical or' checking
+  // in macros, since 'assert(a || b && "some message")' is equal
+  // to '(a || b && 1)' and 'assert("some message" && a || b)' is
+  // equal to '(1 && a || b)'.
+  if (Opc == BO_LOr) {
     DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr);
     DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr);
   }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to