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

[Ted Kremenek]

> 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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140807/9cc2f174/attachment.html>