On Tue, Nov 12, 2013 at 10:37 PM, David Blaikie <[email protected]> wrote:
> On Tue, Nov 12, 2013 at 10:30 PM, Richard Trieu <[email protected]> wrote: > >> On Tue, Nov 12, 2013 at 9:56 PM, Richard Smith <[email protected]>wrote: >> >>> 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? >>> >> Portion of Clang driver code. >> >>> Is this some randomly-chosen file or is it selected to be the worst case >>> for this warning somehow? >>> >> Randomly chosen. >> >>> 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)? >>> >> I used the standard Clang invocation. I am not sure if that constructs >> the CFG. Turning -Wuninitialized on and off produces similar timing >> differences to this warning. >> > > The important question is what's the timing difference between: > -Wuninitialized -Wyour-new-warning and just -Wuninitialized > > That way you won't be comparing the timing of CFG building and no CFG > building, but CFG building with your warning and CFG building without your > warning. > Right. If this warning doesn't add measurable compile time in a build that's already constructing CFGs, then that's OK, but it needs to be off by default, like the other CFG-based warnings. 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 >> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
