[PATCH] Add new warning to Clang to detect when all code paths in a function has a call back to the function.

[Richard Trieu]

Timing details:

21.03s for Clang
21.27s for Patched Clang with warning

This is a difference of .24s or less than 1.5% slowdown.

Times are averaged over 20 runs.  Input is the same pre-processed file.
 Both Clang binaries were built from the same revision.  -fsyntax-only was
also used.

On Fri, Nov 8, 2013 at 10:47 PM, Sean Silva <silvas at purdue.edu> wrote:

>
>
>
> On Fri, Nov 8, 2013 at 8:52 PM, Chandler Carruth <chandlerc at google.com>wrote:
>
>> Random thoughts...
>>
>> On Fri, Nov 8, 2013 at 3:20 PM, Sean Silva <silvas at purdue.edu> wrote:
>>
>>> At a higher level, is this really needed as a compiler warning? I mean,
>>> it's nice and all to detect these things statically, but is this really
>>> something that needs to be happening on every build?
>>
>>
>> Note that if this is a significant compile time issue, that's relevant.
>> Everything else is assuming that it isn't. =]
>>
>> Could we just do this in the static analyzer? In practice, I can't
>>> imagine any of these being hard to debug "while developing", since they
>>> will always result in a stack overflow the second they are called (and then
>>> you just look at the core file (or the debugger that your IDE attached) to
>>> see which function it was)
>>
>>
>> Your description alone is the evidence for why developers should have a
>> warning. ;] First off, stack overflows are notoriously annoying to debug.
>> Core files are often missing. Running under the debugger is yet another
>> step to do. I would much rather the compiler just tell me about it.
>>
>
> Absolutely; a static warning is always preferable. All my comments were in
> light of a perception that this was maybe not cheap enough to justify
> running at every compile (of course, need to wait for Richard to respond
> with some real measurements of the compile time impact).
>
> -- Sean Silva
>
>
>>
>> Almost all of our warnings "aren't needed" because they could be tested
>> and debugged. That doesn't remove their value.
>>
>>
>>> So essentially it seems like this is finding bugs in code that has no
>>> test coverage and has never been executed in practice; that kind of
>>> "cleaning out crusty unused parts of the codebase" seems like it would be
>>> better left to the static analyzer.
>>
>>
>> No, it's shortening the cycle time for developers that hit this bug.
>>
>> It also is cleaning out bugs in crufty code, which is always nice. Very
>> few people will run the static analyzer over all of their crufty code
>> because if they aren't changing it and it is working in production, they
>> aren't going to want to sift through the false positives of "maybe that's a
>> bug". Static analyzers run much more over code under active development.
>>
>> And fundamentally, we *routinely* do all of the static analysis that is
>> sufficiently inexpensive and has sufficiently rare false positives at
>> compile time. So I think we should focus on those two criteria, the latter
>> one being the most interesting here.
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131112/97c983a9/attachment.html>