On Tue, Nov 12, 2013 at 9:44 PM, Richard Trieu <[email protected]> wrote:
> 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. > What's your test case? Is this some randomly-chosen file or is it selected to be the worst case for this warning somehow? A 1.5% slowdown on average seems like far too much for this. Was this in a setup where the CFG was being built regardless (for instance, with -Wuninitialized enabled)? > 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 > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
