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 <[email protected]> wrote: > > > > On Fri, Nov 8, 2013 at 8:52 PM, Chandler Carruth <[email protected]>wrote: > >> Random thoughts... >> >> On Fri, Nov 8, 2013 at 3:20 PM, Sean Silva <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
