[cfe-dev] -Wunreachable-code and templates

[Ted Kremenek]

On Nov 30, 2011, at 10:09 PM, David Blaikie wrote:

> Does this mean we can't analyze any function involving macros? That
> would seem overly limiting. Should we try to record more detail about
> the graph again, such as where a branch (or lack of one) came from?
> How practical is that?

This is all a bit of art, but it is about hitting the sweet spot between false positives and false negatives, complexity of implementation (and cleverness) versus payoff.  This isn't about soundness; -Wunreachable-code was correctly warning in template instantiations that some code is unreachable.  The problem is that there are cases where we don't care, because it doesn't indicate a problem in the original code (e.g., the original template body). The goal of the warning is to find bugs, so we should concentrate on stylistically what kind of deadcode doesn't look intentional.

For templates, my concern is that the template instantiation can routinely impact control-flow in bizarre ways.  We could itemize the common ways that unreachable code occurs in template instantiations, and see if we can do something clever.  I think some of this could be done using the CFG approach I described (e.g., recording that specific pruned branches were "instantiation-dependent").  That might be acceptable for a large number of cases, but the analysis isn't trivial.  I'm very skeptical that the payoff would be worth it versus the complexity of the implementation.

Macros I think are more nuanced.  Consider the following two cases:

   if (MACRO_INSTANTIATION)
     unreachable

and

   MACRO_INSTANTIATION_THAT_CONTAINS_UNREACHABLE_CODE

For the latter, my gut is to never issue a -Wunreachable-code warning.  It's basically the same as templates; the instantiated code could be anything (and it frequently does).  Many people use macro instantiations understanding that the macro will generate a ton of goop, only to get optimized away from the compiler.  I think warning about unreachable code within macros is going to have marginal utility.  However, if we saw something like this:

  MACRO_INSTANTIATION_THAT_BLOCKS_FURTHER_EXECUTION
  deadcode

should we warn about 'deadcode'?  Probably not.  It's probably no different than using llvm_unreachable.

Then there is also the case of:

   if (COMPILE_TIME_CONSTANT)
     unreachable

There, I think it depends on how the compile-time constant was computed.  If the constant involved macros, e.g.:

   if (MACRO_A + 1)

then I'd say this is the same as:

   if (MACRO_INSTANTIATION)

If it was something like:

   if (some_constant_dependent_on_langoptions_or_triple)

then I'd also treat it like "if (MACRO_INSTANTIATION)".  This is basically 'sizeof(long) == sizeof(int)".  It's basically meta-programming of a sort, and we don't know that for a different program "instantiation" (e.g., on a different architecture) that the code would have behaved differently.

If, however, it looks something like:

  if (2 * 0)
    unreachable

I think we should definitely warn.  There is nothing here that relies on "meta-programming".  It's all concrete, and clearly the developer made a mistake.

So the bottom line is that I think it comes down to a grab bag of heuristics.  With the CFG approach I described, I'm basically saying we retain "ghost" edges for the edges we would have pruned.  Heck, we don't even need to record why they were pruned, just that they were pruned.  -Wunreachable-code, if it cares, could go an infer the specific cases of why an edge was pruned (or what was involved, e.g. a macro instantiation in the condition).

One nice thing of leaving the heuristics up to -Wunreachable-code is that it doesn't complicate CFG construction.  CFG construction just records the ghost edges, and it is up to specific analyses to decide what to do with them (if anything).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20111130/e0b91ce0/attachment.html>