Higuoxing updated this revision to Diff 150517.

https://reviews.llvm.org/D47687

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/Sema/logical-op-parentheses-in-macros.c

Index: test/Sema/logical-op-parentheses-in-macros.c
===================================================================
--- test/Sema/logical-op-parentheses-in-macros.c
+++ test/Sema/logical-op-parentheses-in-macros.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -Wparentheses -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -Wlogical-op-parentheses-in-macros -fsyntax-only \
+// RUN:            -verify=logical-op-parentheses %s
+// RUN: %clang_cc1 -Wlogical-op-parentheses -fsyntax-only \
+// RUN:            -verify=silence %s
+
+#define macro_op_parentheses_check(x) ( \
+  ( (void)(x) ) \
+)
+
+// silence-no-diagnostics
+void logical_op_in_macros_test(unsigned i) {
+  macro_op_parentheses_check(i || i && i); // logical-op-parentheses-warning {{'&&' within '||'}} \
+                                           // logical-op-parentheses-note {{place parentheses around the '&&' expression to silence this warning}}
+
+  macro_op_parentheses_check(i || i && "I love LLVM"); // no warning.
+  macro_op_parentheses_check("I love LLVM" && i || i); // no warning.
+
+  macro_op_parentheses_check(i || i && "I love LLVM" || i); // logical-op-parentheses-warning {{'&&' within '||'}} \
+                                                            // logical-op-parentheses-note {{place parentheses around the '&&' expression to silence this warning}}
+
+  macro_op_parentheses_check(i || "I love LLVM" && i || i); // logical-op-parentheses-warning {{'&&' within '||'}} \
+                                                            // logical-op-parentheses-note {{place parentheses around the '&&' expression to silence this warning}}
+
+  macro_op_parentheses_check(i && i || 0); // no warning.
+  macro_op_parentheses_check(0 || i && i); // no warning.
+}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -12172,8 +12172,16 @@
 EmitDiagnosticForLogicalAndInLogicalOr(Sema &Self, SourceLocation OpLoc,
                                        BinaryOperator *Bop) {
   assert(Bop->getOpcode() == BO_LAnd);
-  Self.Diag(Bop->getOperatorLoc(), diag::warn_logical_and_in_logical_or)
-      << Bop->getSourceRange() << OpLoc;
+  Self.Diag(Bop->getOperatorLoc(), !OpLoc.isMacroID() ?
+    // if this warning is in a normal context
+    diag::warn_logical_and_in_logical_or :
+    // else this warning is in a macro context
+    // currently, this will not warn in macros by default.
+    // add [-Wlogical-op-parentheses-in-macros]
+    // to enable this warning.
+    diag::warn_logical_and_in_logical_or_in_macros
+  ) << Bop->getSourceRange() << OpLoc;
+  
   SuggestParentheses(Self, Bop->getOperatorLoc(),
     Self.PDiag(diag::note_precedence_silence)
       << Bop->getOpcodeStr(),
@@ -12205,6 +12213,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 +12236,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 +12319,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: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5483,6 +5483,10 @@
 def warn_logical_and_in_logical_or : Warning<
   "'&&' within '||'">, InGroup<LogicalOpParentheses>;
 
+def warn_logical_and_in_logical_or_in_macros: 
+  Warning<warn_logical_and_in_logical_or.Text>, 
+  InGroup<LogicalOpParenthesesInMacros>, DefaultIgnore;
+
 def warn_overloaded_shift_in_comparison :Warning<
   "overloaded operator %select{>>|<<}0 has higher precedence than "
   "comparison operator">,
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -261,6 +261,8 @@
 def GlobalConstructors : DiagGroup<"global-constructors">;
 def BitwiseOpParentheses: DiagGroup<"bitwise-op-parentheses">;
 def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">;
+def LogicalOpParenthesesInMacros: DiagGroup<"logical-op-parentheses-in-macros", 
+                                            [LogicalOpParentheses]>;
 def LogicalNotParentheses: DiagGroup<"logical-not-parentheses">;
 def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">;
 def OverloadedShiftOpParentheses: DiagGroup<"overloaded-shift-op-parentheses">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D47687: fi... Xing via Phabricator via cfe-commits

Reply via email to