On Tue Jan 13 2015 at 1:54:43 PM Daniel Jasper <[email protected]> wrote:
> There are many reasons to not put a lot of checks that clang-tidy has into > the compiler: > a) Checks might be too expensive. Maybe not the case now for this check, > but actually removing the braces safely might need quite a bit of > additional computation. > b) Not interesting to many developers. People can have rather strong and > diverse opinions on specific style options. In contrast to many of the > other Clang warnings, such style issues might increase readability but > aren't likely to be bugs. > c) Doing the checks on every compile is a waste of time. And I don't just > mean compilation time. Sometimes when developing code, commenting out > sections, etc. checks can suddenly be triggered. Having to fix them (or > suppress specific warnings) is additional unnecessary effort. This e.g. is > also what bugs some people about -Wunused-*. All we want to do is ensure > that these are executed/fixed before submit. > d) At least for now, checks are way easier to write concisely in > clang-tidy. Doing the same work somewhere in Sema can be quite complex. We > probably can implement a better abstraction, but that is a lot of work to > be done. > > In short, I think we will want to keep many of these checks in clang-tidy > even if there isn't a strong technical reason not to move them into the > compiler itself. Nonetheless, we should improve the development workflow > integration so that we get all of the benefits that we would get from a > compiler warning. > It still seems like having the ability to compile clang-tidy checks into clang (optionally) and having to run only clang would be a use-case many people might benefit from... > > On Tue, Jan 13, 2015 at 1:12 AM, David Blaikie <[email protected]> wrote: > >> >> >> On Mon, Jan 12, 2015 at 4:07 PM, Sean Silva <[email protected]> >> wrote: >> >>> >>> >>> On Mon, Jan 12, 2015 at 4:00 PM, David Blaikie <[email protected]> >>> wrote: >>> >>>> >>>> >>>> On Mon, Jan 12, 2015 at 3:46 PM, Sean Silva <[email protected]> >>>> wrote: >>>> >>>>> In http://reviews.llvm.org/D6927#107453, @dblaikie wrote: >>>>> >>>>> > Not your problem, but I'm wondering: If/when/how we'll be able to >>>>> integrate clang-tidy checks into the compile step for developers. This >>>>> warning and many others in clang-tidy ought to be cheap enough to run at >>>>> compile time and as hard errors just like many real clang warnings (the >>>>> only reason they're not is that they're stylistic in nature and so don't >>>>> meet that bar for the compiler warnings - not because they aren't cheap, >>>>> low false positive, etc). It'd be nice not to have these as asynchronous >>>>> results but as errors during the build. >>>>> >>>>> >>>>> Maybe there's scope for a way to add warnings/errors which are run as >>>>> a separate AST consumer, rather than integrated into the parsing/lexing >>>>> code path. That way, you don't pay for them if you don't use them (also >>>>> you >>>>> don't pay for them in added complexity within the parsing/lexing code). I >>>>> think most AST-based clang-tidy checks would fit that model. We also have >>>>> a >>>>> warning we added internally that would be very nice to cleanly segregate >>>>> out of the main code path like that. >>>>> >>>> >>>> Perhaps - I'm not sure if "compiled in, but designed to not add >>>> complexity to the main codepaths" would meet the bar of the Powers That Be >>>> (Richard Smith, mostly, maybe Doug, etc). >>>> >>>> Does anyone care about the size of the clang binary? (I know people >>>> care about the size of LLVM so as to be able to use it as a library in web >>>> browsers, etc) If not, it seems pretty harmless to put some AST checkers in >>>> that way. >>>> >>> >>> It might be possible to have them as a separate lib that can optionally >>> be linked in based on a configure-time option. >>> >> >> Yep, would be a possibility, if necessary/desirable. >> >> >>> I suspect that we may have some warnings already that could be excised >>> from the main codepath and put into here, so the net effect might be to >>> enable a slimmer clang for those wanting to reduce clang binary size. >>> >> >> True. >> >> It'd be interesting to get a feel for the performance of clang-tidy-esque >> checks, too. I wonder how much of a perf hit it would be to rephrase some >> existing warnings in terms of AST matching. >> >> - David >> >> >>> >>> -- Sean Silva >>> >>> >>>> >>>> >>>> - David >>>> >>>> >>>>> >>>>> >>>>> http://reviews.llvm.org/D6927 >>>>> >>>>> EMAIL PREFERENCES >>>>> http://reviews.llvm.org/settings/panel/emailpreferences/ >>>>> >>>>> >>>>> >>>> >>> >> > _______________________________________________ > 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
