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

[Tim Shen via cfe-commits]

On Mon, Oct 24, 2016 at 2:38 PM David Blaikie <dblaikie at gmail.com> 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 <timshen at google.com> wrote:
>
> On Mon, Oct 24, 2016 at 10:33 AM David Blaikie <dblaikie at gmail.com> wrote:
>
> On Mon, Aug 29, 2016 at 3:45 PM Tim Shen via cfe-commits <
> cfe-commits at 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<Expr>(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<Stmt *>(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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161024/4265eb68/attachment.html>