Avoid multiple -Wunreachable-code diagnostics that are triggered by

the same source range and use the unary operator fixit only when it
actually silences the warning.

rdar://24570531

Differential Revision: https://reviews.llvm.org/D28231

llvm-svn: 291757

clang/lib/Analysis/ReachableCode.cpp

@@ -218,11 +218,21 @@ static bool isConfigurationValue(const Stmt *S,
     }
     case Stmt::UnaryOperatorClass: {
       const UnaryOperator *UO = cast<UnaryOperator>(S);
-      if (SilenceableCondVal) 
-        *SilenceableCondVal = UO->getSourceRange();      
-      return UO->getOpcode() == UO_LNot &&
-             isConfigurationValue(UO->getSubExpr(), PP, SilenceableCondVal,
-                                  IncludeIntegers, WrappedInParens);
+      if (UO->getOpcode() != UO_LNot)
+        return false;
+      bool SilenceableCondValNotSet =
+          SilenceableCondVal && SilenceableCondVal->getBegin().isInvalid();
+      bool IsSubExprConfigValue =
+          isConfigurationValue(UO->getSubExpr(), PP, SilenceableCondVal,
+                               IncludeIntegers, WrappedInParens);
+      // Update the silenceable condition value source range only if the range
+      // was set directly by the child expression.
+      if (SilenceableCondValNotSet &&
+          SilenceableCondVal->getBegin().isValid() &&
+          *SilenceableCondVal ==
+              UO->getSubExpr()->IgnoreCasts()->getSourceRange())
+        *SilenceableCondVal = UO->getSourceRange();
+      return IsSubExprConfigValue;
     }
     default:
       return false;

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -56,6 +56,8 @@ using namespace clang;
 namespace {
   class UnreachableCodeHandler : public reachable_code::Callback {
     Sema &S;
+    SourceRange PreviousSilenceableCondVal;
+
   public:
     UnreachableCodeHandler(Sema &s) : S(s) {}
 
@@ -64,6 +66,14 @@ namespace {
                            SourceRange SilenceableCondVal,
                            SourceRange R1,
                            SourceRange R2) override {
+      // Avoid reporting multiple unreachable code diagnostics that are
+      // triggered by the same conditional value.
+      if (PreviousSilenceableCondVal.isValid() &&
+          SilenceableCondVal.isValid() &&
+          PreviousSilenceableCondVal == SilenceableCondVal)
+        return;
+      PreviousSilenceableCondVal = SilenceableCondVal;
+
       unsigned diag = diag::warn_unreachable;
       switch (UK) {
         case reachable_code::UK_Break:

clang/test/Sema/warn-unreachable.c

@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -fblocks -Wunreachable-code-aggressive -Wno-unused-value -Wno-covered-switch-default -I %S/Inputs
+// RUN: %clang_cc1 -fsyntax-only -fblocks -Wunreachable-code-aggressive -Wno-unused-value -Wno-covered-switch-default -fdiagnostics-parseable-fixits -I %S/Inputs %s 2>&1 | FileCheck %s
 
 #include "warn-unreachable.h"
 
@@ -396,3 +397,57 @@ void test_with_paren_silencing(int x) {
   else
     calledFun();
 }
+
+// rdar://24570531
+
+struct StructWithPointer {
+  void *p;
+};
+
+void emitJustOneWarningForOr(struct StructWithPointer *s) {
+  if (1 || !s->p) // expected-note {{silence by adding parentheses to mark code as explicitly dead}}
+    return; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:"/* DISABLES CODE */ ("
+            // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:8-[[@LINE-2]]:8}:")"
+  emitJustOneWarningForOr(s); // expected-warning {{code will never be executed}}
+}
+
+void emitJustOneWarningForOrSilenced(struct StructWithPointer *s) {
+  if ((1) || !s->p)
+    return;
+
+  emitJustOneWarningForOrSilenced(s); // no warning
+}
+
+void emitJustOneWarningForOr2(struct StructWithPointer *s) {
+  if (1 || !s->p) // expected-warning {{code will never be executed}}
+    return; // expected-note@-1 {{silence by adding parentheses to mark code as explicitly dead}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:7}:"/* DISABLES CODE */ ("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:8-[[@LINE-3]]:8}:")"
+}
+
+void wrapOneInFixit(struct StructWithPointer *s) {
+  if (!s->p || 1) // expected-note {{silence by adding parentheses to mark code as explicitly dead}}
+    return; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"/* DISABLES CODE */ ("
+            // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:17-[[@LINE-2]]:17}:")"
+  wrapOneInFixit(s); // expected-warning {{code will never be executed}}
+}
+
+void unaryOpNoFixit() {
+  if (- 1)
+    return; // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+  unaryOpNoFixit(); // expected-warning {{code will never be executed}}
+}
+
+void unaryOpStrictFixit(struct StructWithPointer *s) {
+  if (!(s->p && 0)) // expected-note {{silence by adding parentheses to mark code as explicitly dead}}
+    return; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:17-[[@LINE-1]]:17}:"/* DISABLES CODE */ ("
+            // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:18-[[@LINE-2]]:18}:")"
+  unaryOpStrictFixit(s); // expected-warning {{code will never be executed}}
+}
+
+void unaryOpFixitCastSubExpr(int x) {
+  if (! (int)0) // expected-note {{silence by adding parentheses to mark code as explicitly dead}}
+    return; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:"/* DISABLES CODE */ ("
+            // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:15}:")"
+  unaryOpFixitCastSubExpr(x); // expected-warning {{code will never be executed}}
+}