[PATCH] D28231: -Wunreachable-code: Avoid multiple diagnostics that are triggered by the same source range and fix the unary operator fixit source range

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL291757: Avoid multiple -Wunreachable-code diagnostics that 
are triggered by (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D28231?vs=83819=84094#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28231

Files:
  cfe/trunk/lib/Analysis/ReachableCode.cpp
  cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
  cfe/trunk/test/Sema/warn-unreachable.c

Index: cfe/trunk/test/Sema/warn-unreachable.c
===
--- cfe/trunk/test/Sema/warn-unreachable.c
+++ cfe/trunk/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 @@
   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}}
+}
Index: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
===
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
@@ -56,14 +56,24 @@
 namespace {
   class UnreachableCodeHandler : public reachable_code::Callback {
 Sema 
+SourceRange PreviousSilenceableCondVal;
+
   public:
 UnreachableCodeHandler(Sema ) : S(s) {}
 
 void HandleUnreachable(reachable_code::UnreachableKind UK,
SourceLocation L,
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:
Index: cfe/trunk/lib/Analysis/ReachableCode.cpp
===
--- cfe/trunk/lib/Analysis/ReachableCode.cpp
+++ cfe/trunk/lib/Analysis/ReachableCode.cpp
@@ -218,11 +218,21 @@
 }
 case Stmt::UnaryOperatorClass: {
   const UnaryOperator *UO = cast(S);
- 

[PATCH] D28231: -Wunreachable-code: Avoid multiple diagnostics that are triggered by the same source range and fix the unary operator fixit source range

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak accepted this revision.
ahatanak added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM


Repository:
  rL LLVM

https://reviews.llvm.org/D28231



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28231: -Wunreachable-code: Avoid multiple diagnostics that are triggered by the same source range and fix the unary operator fixit source range

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Analysis/ReachableCode.cpp:229
+  if (SilenceableCondValNotSet && SilenceableCondVal->getBegin().isValid())
+*SilenceableCondVal = UO->getSourceRange();
+  return UO->getOpcode() == UO_LNot && IsSubExprConfigValue;

ahatanak wrote:
> Thanks, that fixed the incorrect fixit. The patch looks good to me, but there 
> are still cases in which the suggestions clang makes are not accurate. 
> Perhaps you can leave a FIXME somewhere so that we don't forget to fix it 
> later?
> 
> For example, if we change the condition in the test case to this,
> 
> ```
> if (!(s->p && 0))
> ```
> 
> , clang suggests enclosing the entire expression with a parenthesis (caret 
> points to "!"), but the warning will not go away unless the parenthesis is 
> around "0".
> 
> The second example is in function unaryOpFixit in warn-unreachable.c. clang 
> suggests enclosing "-1" with a parenthesis, but it still warns after putting 
> the parenthesis around "-1" or "1".
Thanks for pointing out these issues, I've decided to fix them in this patch. 
Since only the '!' operator can wrap a silenced expression, the updated patch 
ensures that fixits for unary operators can only be used with '!' (this fixes 
your second example). I fixed the first example by ensuring that the unary 
operator source range can be used for fixit only if the previous fixit source 
range comes from its direct child.


Repository:
  rL LLVM

https://reviews.llvm.org/D28231



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28231: -Wunreachable-code: Avoid multiple diagnostics that are triggered by the same source range and fix the unary operator fixit source range

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 83819.
arphaman added a comment.

Fix more issues with FIXIT for unary operators.


Repository:
  rL LLVM

https://reviews.llvm.org/D28231

Files:
  lib/Analysis/ReachableCode.cpp
  lib/Sema/AnalysisBasedWarnings.cpp
  test/Sema/warn-unreachable.c

Index: test/Sema/warn-unreachable.c
===
--- test/Sema/warn-unreachable.c
+++ 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 @@
   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}}
+}
Index: lib/Sema/AnalysisBasedWarnings.cpp
===
--- lib/Sema/AnalysisBasedWarnings.cpp
+++ lib/Sema/AnalysisBasedWarnings.cpp
@@ -56,14 +56,24 @@
 namespace {
   class UnreachableCodeHandler : public reachable_code::Callback {
 Sema 
+SourceRange PreviousSilenceableCondVal;
+
   public:
 UnreachableCodeHandler(Sema ) : S(s) {}
 
 void HandleUnreachable(reachable_code::UnreachableKind UK,
SourceLocation L,
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:
Index: lib/Analysis/ReachableCode.cpp
===
--- lib/Analysis/ReachableCode.cpp
+++ lib/Analysis/ReachableCode.cpp
@@ -218,11 +218,21 @@
 }
 case Stmt::UnaryOperatorClass: {
   const UnaryOperator *UO = cast(S);
-  if (SilenceableCondVal) 
-*SilenceableCondVal = UO->getSourceRange();  
-  return UO->getOpcode() == UO_LNot &&
- isConfigurationValue(UO->getSubExpr(), PP, SilenceableCondVal,
-  IncludeIntegers, 

[PATCH] D28231: -Wunreachable-code: Avoid multiple diagnostics that are triggered by the same source range and fix the unary operator fixit source range

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: lib/Analysis/ReachableCode.cpp:229
+  if (SilenceableCondValNotSet && SilenceableCondVal->getBegin().isValid())
+*SilenceableCondVal = UO->getSourceRange();
+  return UO->getOpcode() == UO_LNot && IsSubExprConfigValue;

Thanks, that fixed the incorrect fixit. The patch looks good to me, but there 
are still cases in which the suggestions clang makes are not accurate. Perhaps 
you can leave a FIXME somewhere so that we don't forget to fix it later?

For example, if we change the condition in the test case to this,

```
if (!(s->p && 0))
```

, clang suggests enclosing the entire expression with a parenthesis (caret 
points to "!"), but the warning will not go away unless the parenthesis is 
around "0".

The second example is in function unaryOpFixit in warn-unreachable.c. clang 
suggests enclosing "-1" with a parenthesis, but it still warns after putting 
the parenthesis around "-1" or "1".


Repository:
  rL LLVM

https://reviews.llvm.org/D28231



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28231: -Wunreachable-code: Avoid multiple diagnostics that are triggered by the same source range and fix the unary operator fixit source range

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 83032.
arphaman added a comment.

You're right, the fixit for `if (!s->p || 1)` is wrong, but that was another 
existing bug. The updated patch fixes this issue.


Repository:
  rL LLVM

https://reviews.llvm.org/D28231

Files:
  lib/Analysis/ReachableCode.cpp
  lib/Sema/AnalysisBasedWarnings.cpp
  test/Sema/warn-unreachable.c

Index: test/Sema/warn-unreachable.c
===
--- test/Sema/warn-unreachable.c
+++ 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,44 @@
   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 unaryOpFixit() {
+  if (- 1)  // 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]]:10-[[@LINE-2]]:10}:")"
+  unaryOpFixit(); // expected-warning {{code will never be executed}}
+}
Index: lib/Sema/AnalysisBasedWarnings.cpp
===
--- lib/Sema/AnalysisBasedWarnings.cpp
+++ lib/Sema/AnalysisBasedWarnings.cpp
@@ -56,14 +56,24 @@
 namespace {
   class UnreachableCodeHandler : public reachable_code::Callback {
 Sema 
+SourceRange PreviousSilenceableCondVal;
+
   public:
 UnreachableCodeHandler(Sema ) : S(s) {}
 
 void HandleUnreachable(reachable_code::UnreachableKind UK,
SourceLocation L,
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:
Index: lib/Analysis/ReachableCode.cpp
===
--- lib/Analysis/ReachableCode.cpp
+++ lib/Analysis/ReachableCode.cpp
@@ -218,11 +218,16 @@
 }
 case Stmt::UnaryOperatorClass: {
   const UnaryOperator *UO = cast(S);
-  if (SilenceableCondVal) 
-*SilenceableCondVal = UO->getSourceRange();  
-  return UO->getOpcode() == UO_LNot &&
- isConfigurationValue(UO->getSubExpr(), PP, SilenceableCondVal,
-  IncludeIntegers, WrappedInParens);
+  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 in the child expression.
+  if (SilenceableCondValNotSet && SilenceableCondVal->getBegin().isValid())
+*SilenceableCondVal = 

[PATCH] D28231: -Wunreachable-code: Avoid multiple diagnostics that are triggered by the same source range and fix the unary operator fixit source range

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

If I change the condition to the following,

  if (!s->p || 1)

clang suggests enclosing !s->p with a parenthesis, but the comment in 
ReachableCode.cpp says the parenthesis should enclose the integer literal. It 
seems like there is a contradiction here?


Repository:
  rL LLVM

https://reviews.llvm.org/D28231



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28231: -Wunreachable-code: Avoid multiple diagnostics that are triggered by the same source range and fix the unary operator fixit source range

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: rsmith, bruno, ahatanak.
arphaman added a subscriber: cfe-commits.
arphaman set the repository for this revision to rL LLVM.

This patch fixes an issue with -Wunreachable-code diagnostic that happens with 
the following code sample:

  struct s {
void *p;
  };
  
  void fn(struct s *s) {
if (1 || !s->p)
  return;
fn(s);
  }

Currently, clang will emit two warnings (at `fn(s)` and at `!s->p`. As well as 
that, the fixits for the first warning (for `fn(s)`) has an incorrect source 
range: it suggests to wrap `!s->p` in parentheses when it should suggest to 
warp `1` instead.

The attached patch ensures that the fixits for the `fn(s)` warning suggest to 
wrap the correct expression (`1`). It also avoids the second warning that was 
previously shown for `!s->p` because that warning is caused by the same 
expression (`1`).

Thanks for taking a look.


Repository:
  rL LLVM

https://reviews.llvm.org/D28231

Files:
  lib/Analysis/ReachableCode.cpp
  lib/Sema/AnalysisBasedWarnings.cpp
  test/Sema/warn-unreachable.c


Index: test/Sema/warn-unreachable.c
===
--- test/Sema/warn-unreachable.c
+++ 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,30 @@
   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}:")"
+}
Index: lib/Sema/AnalysisBasedWarnings.cpp
===
--- lib/Sema/AnalysisBasedWarnings.cpp
+++ lib/Sema/AnalysisBasedWarnings.cpp
@@ -56,14 +56,24 @@
 namespace {
   class UnreachableCodeHandler : public reachable_code::Callback {
 Sema 
+SourceRange PreviousSilenceableCondVal;
+
   public:
 UnreachableCodeHandler(Sema ) : S(s) {}
 
 void HandleUnreachable(reachable_code::UnreachableKind UK,
SourceLocation L,
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:
Index: lib/Analysis/ReachableCode.cpp
===
--- lib/Analysis/ReachableCode.cpp
+++ lib/Analysis/ReachableCode.cpp
@@ -218,8 +218,8 @@
 }
 case Stmt::UnaryOperatorClass: {
   const UnaryOperator *UO = cast(S);
-  if (SilenceableCondVal) 
-*SilenceableCondVal = UO->getSourceRange();  
+  if (SilenceableCondVal && !SilenceableCondVal->getBegin().isValid())
+*SilenceableCondVal = UO->getSourceRange();
   return UO->getOpcode() == UO_LNot &&
  isConfigurationValue(UO->getSubExpr(), PP, SilenceableCondVal,
   IncludeIntegers, WrappedInParens);


Index: test/Sema/warn-unreachable.c
===
--- test/Sema/warn-unreachable.c
+++ 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