[PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[Ted Kremenek via cfe-commits]

krememek added a comment.

LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D24010



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

[PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[Tim Shen via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL285657: [ReachableCode] Skip over ExprWithCleanups in 
isConfigurationValue (authored by timshen).

Changed prior to commit:
  https://reviews.llvm.org/D24010?vs=75644&id=76508#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24010

Files:
  cfe/trunk/include/clang/AST/Stmt.h
  cfe/trunk/lib/Analysis/ReachableCode.cpp
  cfe/trunk/test/SemaCXX/PR29152.cpp


Index: cfe/trunk/include/clang/AST/Stmt.h
===
--- cfe/trunk/include/clang/AST/Stmt.h
+++ cfe/trunk/include/clang/AST/Stmt.h
@@ -387,6 +387,9 @@
   /// Skip past any implicit AST nodes which might surround this
   /// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
   Stmt *IgnoreImplicit();
+  const Stmt *IgnoreImplicit() const {
+return const_cast(this)->IgnoreImplicit();
+  }
 
   /// \brief Skip no-op (attributed, compound) container stmts and skip 
captured
   /// stmt at the top, if \a IgnoreCaptured is true.
Index: cfe/trunk/test/SemaCXX/PR29152.cpp
===
--- cfe/trunk/test/SemaCXX/PR29152.cpp
+++ cfe/trunk/test/SemaCXX/PR29152.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunreachable-code -verify %s
+
+static const bool False = false;
+
+struct A {
+  ~A();
+  operator bool();
+};
+void Bar();
+
+void Foo() {
+  if (False && A()) {
+Bar(); // expected-no-diagnostics
+  }
+}
Index: cfe/trunk/lib/Analysis/ReachableCode.cpp
===
--- cfe/trunk/lib/Analysis/ReachableCode.cpp
+++ cfe/trunk/lib/Analysis/ReachableCode.cpp
@@ -164,6 +164,8 @@
   if (!S)
 return false;
 
+  S = S->IgnoreImplicit();
+
   if (const Expr *Ex = dyn_cast(S))
 S = Ex->IgnoreCasts();
 


Index: cfe/trunk/include/clang/AST/Stmt.h
===
--- cfe/trunk/include/clang/AST/Stmt.h
+++ cfe/trunk/include/clang/AST/Stmt.h
@@ -387,6 +387,9 @@
   /// Skip past any implicit AST nodes which might surround this
   /// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
   Stmt *IgnoreImplicit();
+  const Stmt *IgnoreImplicit() const {
+return const_cast(this)->IgnoreImplicit();
+  }
 
   /// \brief Skip no-op (attributed, compound) container stmts and skip captured
   /// stmt at the top, if \a IgnoreCaptured is true.
Index: cfe/trunk/test/SemaCXX/PR29152.cpp
===
--- cfe/trunk/test/SemaCXX/PR29152.cpp
+++ cfe/trunk/test/SemaCXX/PR29152.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunreachable-code -verify %s
+
+static const bool False = false;
+
+struct A {
+  ~A();
+  operator bool();
+};
+void Bar();
+
+void Foo() {
+  if (False && A()) {
+Bar(); // expected-no-diagnostics
+  }
+}
Index: cfe/trunk/lib/Analysis/ReachableCode.cpp
===
--- cfe/trunk/lib/Analysis/ReachableCode.cpp
+++ cfe/trunk/lib/Analysis/ReachableCode.cpp
@@ -164,6 +164,8 @@
   if (!S)
 return false;
 
+  S = S->IgnoreImplicit();
+
   if (const Expr *Ex = dyn_cast(S))
 S = Ex->IgnoreCasts();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[Stephen Hines via cfe-commits]

srhines added a comment.

Ping. Other developers (Firefox) are now starting to hit this issue.


https://reviews.llvm.org/D24010



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

Re: [PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[David Blaikie via cfe-commits]

Ah, right - thanks for reminding/explaining!

On Mon, Oct 24, 2016 at 2:42 PM Tim Shen  wrote:

> On Mon, Oct 24, 2016 at 2:38 PM David Blaikie  wrote:
>
> Simplify it further by replacing A() with just a function instead of a
> class? Or does that break the repro?
>
> bool Foo();
> void Bar();
> void Baz() {
>   if (False && Foo())
> Bar();
> }
>
>
> This actually breaks the repro. My test introduces a non-trivially
> destructible temporary object in the condition expression, which creates a
> ExprWithCleanups node at the full expression entry.
>
>
>
> On Mon, Oct 24, 2016 at 1:38 PM Tim Shen  wrote:
>
> On Mon, Oct 24, 2016 at 10:33 AM David Blaikie  wrote:
>
> On Mon, Aug 29, 2016 at 3:45 PM Tim Shen via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> timshen created this revision.
> timshen added reviewers: rsmith, pirama.
> timshen added a subscriber: cfe-commits.
>
> https://reviews.llvm.org/D24010
>
> Files:
>   clang/include/clang/AST/Stmt.h
>   clang/lib/Analysis/ReachableCode.cpp
>   clang/test/SemaCXX/PR29152.cpp
>
> Index: clang/test/SemaCXX/PR29152.cpp
> ===
> --- /dev/null
> +++ clang/test/SemaCXX/PR29152.cpp
> @@ -0,0 +1,19 @@
> +// RUN: %clang_cc1 -fsyntax-only -Wunreachable-code -verify %s
> +
> +static const bool False = false;
> +
> +struct Vector {
> +  struct iterator {
> +bool operator==(const iterator &) const;
> +  };
> +  iterator end();
> +};
> +
> +void Bar();
> +Vector::iterator Find(Vector &a);
> +
> +void Foo(Vector &a) {
> +  if (False && Find(a) == a.end()) {
> +Bar(); // expected-no-diagnostics
> +  }
> +}
>
>
> What are the relevant parts of this test for this change?
>
> Is it the static const bool False that's interesting?
>
> Is it the op==/other interesting side effects on the RHS that are
> interesting?
>
> I'd be surprised if it were both (& if it isn't both, I'm guessing it's
> the former rather than the latter - in which case the latter can probably
> be reduced to just an arbitrary function call to, say: bool Baz())
>
>
> It's the static const bool False. I actually managed to simplify the code
> to remove the operator==() call.
>
>
>
>
> Index: clang/lib/Analysis/ReachableCode.cpp
> ===
> --- clang/lib/Analysis/ReachableCode.cpp
> +++ clang/lib/Analysis/ReachableCode.cpp
> @@ -164,6 +164,8 @@
>if (!S)
>  return false;
>
> +  S = S->IgnoreImplicit();
> +
>if (const Expr *Ex = dyn_cast(S))
>  S = Ex->IgnoreCasts();
>
> Index: clang/include/clang/AST/Stmt.h
> ===
> --- clang/include/clang/AST/Stmt.h
> +++ clang/include/clang/AST/Stmt.h
> @@ -387,6 +387,9 @@
>/// Skip past any implicit AST nodes which might surround this
>/// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
>Stmt *IgnoreImplicit();
> +  const Stmt *IgnoreImplicit() const {
> +return const_cast(this)->IgnoreImplicit();
> +  }
>
>/// \brief Skip no-op (attributed, compound) container stmts and skip
> captured
>/// stmt at the top, if \a IgnoreCaptured is true.
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[Tim Shen via cfe-commits]

On Mon, Oct 24, 2016 at 2:38 PM David Blaikie  wrote:

> Simplify it further by replacing A() with just a function instead of a
> class? Or does that break the repro?
>
> bool Foo();
> void Bar();
> void Baz() {
>   if (False && Foo())
> Bar();
> }
>

This actually breaks the repro. My test introduces a non-trivially
destructible temporary object in the condition expression, which creates a
ExprWithCleanups node at the full expression entry.


>
> On Mon, Oct 24, 2016 at 1:38 PM Tim Shen  wrote:
>
> On Mon, Oct 24, 2016 at 10:33 AM David Blaikie  wrote:
>
> On Mon, Aug 29, 2016 at 3:45 PM Tim Shen via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> timshen created this revision.
> timshen added reviewers: rsmith, pirama.
> timshen added a subscriber: cfe-commits.
>
> https://reviews.llvm.org/D24010
>
> Files:
>   clang/include/clang/AST/Stmt.h
>   clang/lib/Analysis/ReachableCode.cpp
>   clang/test/SemaCXX/PR29152.cpp
>
> Index: clang/test/SemaCXX/PR29152.cpp
> ===
> --- /dev/null
> +++ clang/test/SemaCXX/PR29152.cpp
> @@ -0,0 +1,19 @@
> +// RUN: %clang_cc1 -fsyntax-only -Wunreachable-code -verify %s
> +
> +static const bool False = false;
> +
> +struct Vector {
> +  struct iterator {
> +bool operator==(const iterator &) const;
> +  };
> +  iterator end();
> +};
> +
> +void Bar();
> +Vector::iterator Find(Vector &a);
> +
> +void Foo(Vector &a) {
> +  if (False && Find(a) == a.end()) {
> +Bar(); // expected-no-diagnostics
> +  }
> +}
>
>
> What are the relevant parts of this test for this change?
>
> Is it the static const bool False that's interesting?
>
> Is it the op==/other interesting side effects on the RHS that are
> interesting?
>
> I'd be surprised if it were both (& if it isn't both, I'm guessing it's
> the former rather than the latter - in which case the latter can probably
> be reduced to just an arbitrary function call to, say: bool Baz())
>
>
> It's the static const bool False. I actually managed to simplify the code
> to remove the operator==() call.
>
>
>
>
> Index: clang/lib/Analysis/ReachableCode.cpp
> ===
> --- clang/lib/Analysis/ReachableCode.cpp
> +++ clang/lib/Analysis/ReachableCode.cpp
> @@ -164,6 +164,8 @@
>if (!S)
>  return false;
>
> +  S = S->IgnoreImplicit();
> +
>if (const Expr *Ex = dyn_cast(S))
>  S = Ex->IgnoreCasts();
>
> Index: clang/include/clang/AST/Stmt.h
> ===
> --- clang/include/clang/AST/Stmt.h
> +++ clang/include/clang/AST/Stmt.h
> @@ -387,6 +387,9 @@
>/// Skip past any implicit AST nodes which might surround this
>/// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
>Stmt *IgnoreImplicit();
> +  const Stmt *IgnoreImplicit() const {
> +return const_cast(this)->IgnoreImplicit();
> +  }
>
>/// \brief Skip no-op (attributed, compound) container stmts and skip
> captured
>/// stmt at the top, if \a IgnoreCaptured is true.
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[David Blaikie via cfe-commits]

Simplify it further by replacing A() with just a function instead of a
class? Or does that break the repro?

bool Foo();
void Bar();
void Baz() {
  if (False && Foo())
Bar();
}

On Mon, Oct 24, 2016 at 1:38 PM Tim Shen  wrote:

> On Mon, Oct 24, 2016 at 10:33 AM David Blaikie  wrote:
>
> On Mon, Aug 29, 2016 at 3:45 PM Tim Shen via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> timshen created this revision.
> timshen added reviewers: rsmith, pirama.
> timshen added a subscriber: cfe-commits.
>
> https://reviews.llvm.org/D24010
>
> Files:
>   clang/include/clang/AST/Stmt.h
>   clang/lib/Analysis/ReachableCode.cpp
>   clang/test/SemaCXX/PR29152.cpp
>
> Index: clang/test/SemaCXX/PR29152.cpp
> ===
> --- /dev/null
> +++ clang/test/SemaCXX/PR29152.cpp
> @@ -0,0 +1,19 @@
> +// RUN: %clang_cc1 -fsyntax-only -Wunreachable-code -verify %s
> +
> +static const bool False = false;
> +
> +struct Vector {
> +  struct iterator {
> +bool operator==(const iterator &) const;
> +  };
> +  iterator end();
> +};
> +
> +void Bar();
> +Vector::iterator Find(Vector &a);
> +
> +void Foo(Vector &a) {
> +  if (False && Find(a) == a.end()) {
> +Bar(); // expected-no-diagnostics
> +  }
> +}
>
>
> What are the relevant parts of this test for this change?
>
> Is it the static const bool False that's interesting?
>
> Is it the op==/other interesting side effects on the RHS that are
> interesting?
>
> I'd be surprised if it were both (& if it isn't both, I'm guessing it's
> the former rather than the latter - in which case the latter can probably
> be reduced to just an arbitrary function call to, say: bool Baz())
>
>
> It's the static const bool False. I actually managed to simplify the code
> to remove the operator==() call.
>
>
>
>
> Index: clang/lib/Analysis/ReachableCode.cpp
> ===
> --- clang/lib/Analysis/ReachableCode.cpp
> +++ clang/lib/Analysis/ReachableCode.cpp
> @@ -164,6 +164,8 @@
>if (!S)
>  return false;
>
> +  S = S->IgnoreImplicit();
> +
>if (const Expr *Ex = dyn_cast(S))
>  S = Ex->IgnoreCasts();
>
> Index: clang/include/clang/AST/Stmt.h
> ===
> --- clang/include/clang/AST/Stmt.h
> +++ clang/include/clang/AST/Stmt.h
> @@ -387,6 +387,9 @@
>/// Skip past any implicit AST nodes which might surround this
>/// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
>Stmt *IgnoreImplicit();
> +  const Stmt *IgnoreImplicit() const {
> +return const_cast(this)->IgnoreImplicit();
> +  }
>
>/// \brief Skip no-op (attributed, compound) container stmts and skip
> captured
>/// stmt at the top, if \a IgnoreCaptured is true.
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[Tim Shen via cfe-commits]

On Mon, Oct 24, 2016 at 10:33 AM David Blaikie  wrote:

> On Mon, Aug 29, 2016 at 3:45 PM Tim Shen via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> timshen created this revision.
> timshen added reviewers: rsmith, pirama.
> timshen added a subscriber: cfe-commits.
>
> https://reviews.llvm.org/D24010
>
> Files:
>   clang/include/clang/AST/Stmt.h
>   clang/lib/Analysis/ReachableCode.cpp
>   clang/test/SemaCXX/PR29152.cpp
>
> Index: clang/test/SemaCXX/PR29152.cpp
> ===
> --- /dev/null
> +++ clang/test/SemaCXX/PR29152.cpp
> @@ -0,0 +1,19 @@
> +// RUN: %clang_cc1 -fsyntax-only -Wunreachable-code -verify %s
> +
> +static const bool False = false;
> +
> +struct Vector {
> +  struct iterator {
> +bool operator==(const iterator &) const;
> +  };
> +  iterator end();
> +};
> +
> +void Bar();
> +Vector::iterator Find(Vector &a);
> +
> +void Foo(Vector &a) {
> +  if (False && Find(a) == a.end()) {
> +Bar(); // expected-no-diagnostics
> +  }
> +}
>
>
> What are the relevant parts of this test for this change?
>
> Is it the static const bool False that's interesting?
>
> Is it the op==/other interesting side effects on the RHS that are
> interesting?
>
> I'd be surprised if it were both (& if it isn't both, I'm guessing it's
> the former rather than the latter - in which case the latter can probably
> be reduced to just an arbitrary function call to, say: bool Baz())
>

It's the static const bool False. I actually managed to simplify the code
to remove the operator==() call.


>
>
> Index: clang/lib/Analysis/ReachableCode.cpp
> ===
> --- clang/lib/Analysis/ReachableCode.cpp
> +++ clang/lib/Analysis/ReachableCode.cpp
> @@ -164,6 +164,8 @@
>if (!S)
>  return false;
>
> +  S = S->IgnoreImplicit();
> +
>if (const Expr *Ex = dyn_cast(S))
>  S = Ex->IgnoreCasts();
>
> Index: clang/include/clang/AST/Stmt.h
> ===
> --- clang/include/clang/AST/Stmt.h
> +++ clang/include/clang/AST/Stmt.h
> @@ -387,6 +387,9 @@
>/// Skip past any implicit AST nodes which might surround this
>/// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
>Stmt *IgnoreImplicit();
> +  const Stmt *IgnoreImplicit() const {
> +return const_cast(this)->IgnoreImplicit();
> +  }
>
>/// \brief Skip no-op (attributed, compound) container stmts and skip
> captured
>/// stmt at the top, if \a IgnoreCaptured is true.
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[Tim Shen via cfe-commits]

timshen updated this revision to Diff 75644.
timshen added a comment.

Simplified the test.


https://reviews.llvm.org/D24010

Files:
  clang/include/clang/AST/Stmt.h
  clang/lib/Analysis/ReachableCode.cpp
  clang/test/SemaCXX/PR29152.cpp


Index: clang/test/SemaCXX/PR29152.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/PR29152.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunreachable-code -verify %s
+
+static const bool False = false;
+
+struct A {
+  ~A();
+  operator bool();
+};
+void Bar();
+
+void Foo() {
+  if (False && A()) {
+Bar(); // expected-no-diagnostics
+  }
+}
Index: clang/lib/Analysis/ReachableCode.cpp
===
--- clang/lib/Analysis/ReachableCode.cpp
+++ clang/lib/Analysis/ReachableCode.cpp
@@ -164,6 +164,8 @@
   if (!S)
 return false;
 
+  S = S->IgnoreImplicit();
+
   if (const Expr *Ex = dyn_cast(S))
 S = Ex->IgnoreCasts();
 
Index: clang/include/clang/AST/Stmt.h
===
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -387,6 +387,9 @@
   /// Skip past any implicit AST nodes which might surround this
   /// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
   Stmt *IgnoreImplicit();
+  const Stmt *IgnoreImplicit() const {
+return const_cast(this)->IgnoreImplicit();
+  }
 
   /// \brief Skip no-op (attributed, compound) container stmts and skip 
captured
   /// stmt at the top, if \a IgnoreCaptured is true.


Index: clang/test/SemaCXX/PR29152.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/PR29152.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunreachable-code -verify %s
+
+static const bool False = false;
+
+struct A {
+  ~A();
+  operator bool();
+};
+void Bar();
+
+void Foo() {
+  if (False && A()) {
+Bar(); // expected-no-diagnostics
+  }
+}
Index: clang/lib/Analysis/ReachableCode.cpp
===
--- clang/lib/Analysis/ReachableCode.cpp
+++ clang/lib/Analysis/ReachableCode.cpp
@@ -164,6 +164,8 @@
   if (!S)
 return false;
 
+  S = S->IgnoreImplicit();
+
   if (const Expr *Ex = dyn_cast(S))
 S = Ex->IgnoreCasts();
 
Index: clang/include/clang/AST/Stmt.h
===
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -387,6 +387,9 @@
   /// Skip past any implicit AST nodes which might surround this
   /// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
   Stmt *IgnoreImplicit();
+  const Stmt *IgnoreImplicit() const {
+return const_cast(this)->IgnoreImplicit();
+  }
 
   /// \brief Skip no-op (attributed, compound) container stmts and skip captured
   /// stmt at the top, if \a IgnoreCaptured is true.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[David Blaikie via cfe-commits]

On Mon, Aug 29, 2016 at 3:45 PM Tim Shen via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> timshen created this revision.
> timshen added reviewers: rsmith, pirama.
> timshen added a subscriber: cfe-commits.
>
> https://reviews.llvm.org/D24010
>
> Files:
>   clang/include/clang/AST/Stmt.h
>   clang/lib/Analysis/ReachableCode.cpp
>   clang/test/SemaCXX/PR29152.cpp
>
> Index: clang/test/SemaCXX/PR29152.cpp
> ===
> --- /dev/null
> +++ clang/test/SemaCXX/PR29152.cpp
> @@ -0,0 +1,19 @@
> +// RUN: %clang_cc1 -fsyntax-only -Wunreachable-code -verify %s
> +
> +static const bool False = false;
> +
> +struct Vector {
> +  struct iterator {
> +bool operator==(const iterator &) const;
> +  };
> +  iterator end();
> +};
> +
> +void Bar();
> +Vector::iterator Find(Vector &a);
> +
> +void Foo(Vector &a) {
> +  if (False && Find(a) == a.end()) {
> +Bar(); // expected-no-diagnostics
> +  }
> +}
>

What are the relevant parts of this test for this change?

Is it the static const bool False that's interesting?

Is it the op==/other interesting side effects on the RHS that are
interesting?

I'd be surprised if it were both (& if it isn't both, I'm guessing it's the
former rather than the latter - in which case the latter can probably be
reduced to just an arbitrary function call to, say: bool Baz())


> Index: clang/lib/Analysis/ReachableCode.cpp
> ===
> --- clang/lib/Analysis/ReachableCode.cpp
> +++ clang/lib/Analysis/ReachableCode.cpp
> @@ -164,6 +164,8 @@
>if (!S)
>  return false;
>
> +  S = S->IgnoreImplicit();
> +
>if (const Expr *Ex = dyn_cast(S))
>  S = Ex->IgnoreCasts();
>
> Index: clang/include/clang/AST/Stmt.h
> ===
> --- clang/include/clang/AST/Stmt.h
> +++ clang/include/clang/AST/Stmt.h
> @@ -387,6 +387,9 @@
>/// Skip past any implicit AST nodes which might surround this
>/// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
>Stmt *IgnoreImplicit();
> +  const Stmt *IgnoreImplicit() const {
> +return const_cast(this)->IgnoreImplicit();
> +  }
>
>/// \brief Skip no-op (attributed, compound) container stmts and skip
> captured
>/// stmt at the top, if \a IgnoreCaptured is true.
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[Stephen Hines via cfe-commits]

srhines added a comment.

This looks good to me, but I would prefer if one of the more experienced Clang 
folks could officially LGTM this. Thanks.


https://reviews.llvm.org/D24010



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

Re: [PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[Pirama Arumuga Nainar via cfe-commits]

pirama added a comment.

Ping... I am not the author of this patch, but am interested in seeing the 
issue fixed :)


https://reviews.llvm.org/D24010



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

Re: [PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[Pirama Arumuga Nainar via cfe-commits]

pirama added a subscriber: srhines.
pirama added a comment.

Tim: Thanks for looking into this.  It's definitely a simpler fix than I 
thought it would be :)


https://reviews.llvm.org/D24010



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

[PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

[Tim Shen via cfe-commits]

timshen created this revision.
timshen added reviewers: rsmith, pirama.
timshen added a subscriber: cfe-commits.

https://reviews.llvm.org/D24010

Files:
  clang/include/clang/AST/Stmt.h
  clang/lib/Analysis/ReachableCode.cpp
  clang/test/SemaCXX/PR29152.cpp

Index: clang/test/SemaCXX/PR29152.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/PR29152.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunreachable-code -verify %s
+
+static const bool False = false;
+
+struct Vector {
+  struct iterator {
+bool operator==(const iterator &) const;
+  };
+  iterator end();
+};
+
+void Bar();
+Vector::iterator Find(Vector &a);
+
+void Foo(Vector &a) {
+  if (False && Find(a) == a.end()) {
+Bar(); // expected-no-diagnostics
+  }
+}
Index: clang/lib/Analysis/ReachableCode.cpp
===
--- clang/lib/Analysis/ReachableCode.cpp
+++ clang/lib/Analysis/ReachableCode.cpp
@@ -164,6 +164,8 @@
   if (!S)
 return false;
 
+  S = S->IgnoreImplicit();
+
   if (const Expr *Ex = dyn_cast(S))
 S = Ex->IgnoreCasts();
 
Index: clang/include/clang/AST/Stmt.h
===
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -387,6 +387,9 @@
   /// Skip past any implicit AST nodes which might surround this
   /// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
   Stmt *IgnoreImplicit();
+  const Stmt *IgnoreImplicit() const {
+return const_cast(this)->IgnoreImplicit();
+  }
 
   /// \brief Skip no-op (attributed, compound) container stmts and skip 
captured
   /// stmt at the top, if \a IgnoreCaptured is true.


Index: clang/test/SemaCXX/PR29152.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/PR29152.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunreachable-code -verify %s
+
+static const bool False = false;
+
+struct Vector {
+  struct iterator {
+bool operator==(const iterator &) const;
+  };
+  iterator end();
+};
+
+void Bar();
+Vector::iterator Find(Vector &a);
+
+void Foo(Vector &a) {
+  if (False && Find(a) == a.end()) {
+Bar(); // expected-no-diagnostics
+  }
+}
Index: clang/lib/Analysis/ReachableCode.cpp
===
--- clang/lib/Analysis/ReachableCode.cpp
+++ clang/lib/Analysis/ReachableCode.cpp
@@ -164,6 +164,8 @@
   if (!S)
 return false;
 
+  S = S->IgnoreImplicit();
+
   if (const Expr *Ex = dyn_cast(S))
 S = Ex->IgnoreCasts();
 
Index: clang/include/clang/AST/Stmt.h
===
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -387,6 +387,9 @@
   /// Skip past any implicit AST nodes which might surround this
   /// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
   Stmt *IgnoreImplicit();
+  const Stmt *IgnoreImplicit() const {
+return const_cast(this)->IgnoreImplicit();
+  }
 
   /// \brief Skip no-op (attributed, compound) container stmts and skip captured
   /// stmt at the top, if \a IgnoreCaptured is true.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits