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. > >> >>> 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
