[PATCH] clang-tidy: Add initial check for "Don't use else after return".

Manuel Klimek klimek at google.com
Tue Jan 13 08:47:19 PST 2015


On Tue Jan 13 2015 at 1:54:43 PM Daniel Jasper <djasper at google.com> 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 <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Mon, Jan 12, 2015 at 4:07 PM, Sean Silva <chisophugis at gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Mon, Jan 12, 2015 at 4:00 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Mon, Jan 12, 2015 at 3:46 PM, Sean Silva <chisophugis at gmail.com>
>>>> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150113/edf7521d/attachment.html>


More information about the cfe-commits mailing list