r215128 - Mark successors as reachable/unreachable instead of changing the CFG.

[Manuel Klimek]

On Thu, Aug 7, 2014 at 9:30 PM, Ted Kremenek <kremenek at apple.com> wrote:

>
> On Aug 7, 2014, at 12:16 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
> I'm a bit confused as to why /this/ warning would go away when you add
> the optimstic/pessimistic edges. This warning should still fire, as,
> when 'value' is false, control does reach the end of a non-void
> function.
>
> The warnings I would expect to not fire with the kind of change Ted
> was suggesting (and I don't know if this is a pre-existing problem, an
> issue with this patch, or an issue with my understanding) would be
> things like -Wunreachable-code, probably in cases like:
>
> true ? NoReturn() : true;
> func(); // -Wunreachable-code should fire here
>
> ARCH_CONST ? NoReturn() : true;
> func(); // don't warn that this is unreachable, because ARCH_CONST
> could vary by build
>
> Or something like that - I forget which particular heuristics Ted put
> in to suppress build-varying constants.
>
>
> Right.
>
> There are a variety of heuristics that we used:
>
> - The expression condition has 'sizeof' some type or expression.
> - The expression condition has 'alignof' some type or expression.
>
> There is also some code that is intentionally "commented out", but meant
> to still compile, e.g.:
>
> if (false) { ... }
>
> We allow the developer to wrap the condition in extra layer of '()' to
> silence the issue, explicitly marking the code as dead.  Here's some
> examples from our tests:
>
>   if (YES) // expected-note {{silence by adding parentheses to mark code
> as explicitly dead}}
>     calledFun();
>   else
>     calledFun(); // expected-warning {{will never be executed}}
>
>   if ((YES))
>     calledFun();
>   else
>     calledFun(); // no-warning
>
> Note that -Wunreachable-code has knowledge of these heuristics; it's the
> one that applies them.  What's important in the CFG is to capture that some
> edges are conditionally unreachable, letting must analyses prune it out
> automatically.  The default "view" of the CFG is that these edges are
> unreachable.  The logic for -Wunreachable-code needs to do extra work to
> query what the original edge would have been.
>
> Specifically for -Wreturn-type, the change I proposed should not impact
> that warning at all.
>

If the change didn't affect this at all, we'd have loads of false positives
for Wreturn-type, because it uses the reachability analysis to figure out
whether anything falls out at the end of the function...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140807/0db0699a/attachment.html>