impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Apple <jbap...@cloudera.com>
Subject Re: clang-tidy: sweeping linter changes?
Date Mon, 10 Oct 2016 03:56:37 GMT
Thanks - this is working for me!

I'm going to try and curate some config settings. I'll update dev@ if
and when I have something useful.

On Sat, Oct 8, 2016 at 1:03 PM, Todd Lipcon <todd@cloudera.com> wrote:
> Check out clang-tidy-diff which can limit warnings based on a patch. You
> can control the amount of context given to it so that people get warnings
> about surrounding code or really just strictly the changed lines.
>
> We are using this in kudu and it's moderately helpful.
>
> Todd
>
> On Oct 8, 2016 12:13 PM, "Jim Apple" <jbapple@cloudera.com> wrote:
>
>> Should we make large (but mostly cosmetic) changes to our source code to
>> help new contributors write clean C++?
>>
>> We now have a .clang-format file to help new contributors write code with
>> correct whitespace formatting. Many other parts of our C++ style can be
>> enforced by clang-tidy, a stronger tool included in our toolchain that
>> understands much more than whitespace.
>>
>> There is a cost, however -- clang-tidy is not nearly as good at targeting
>> its warnings at newly-written code. To use it well(*), we will need to fix
>> a lot of warnings about our existing code.
>>
>> Should we fix the existing code to make clang-tidy accept it? This would be
>> a big change from what we did with clang-format, in which we ask that new
>> code be clang-formatted, but we allow old code to remain unformatted unless
>> it is touched again.
>>
>> Some randomly-selected examples of the kind of things clang-tidy can
>> enforce (**):
>>
>> * TypeNamesAreCamelCase
>> * "if (b == true)" should be written "if (b)"
>> * Function prototypes should have the same parameter names as function
>> definitions
>> * Single-argument constructors should be "explicit"
>>
>> We can configure clang-tidy, just like clang-format, to be more or less
>> strict for any given check.
>>
>> (*) without building up a lot of ad-hoc tooling to suppress warnings about
>> existing code
>>
>> (**) partial list here:
>> http://clang.llvm.org/extra/clang-tidy/checks/list.html. This list does
>> not
>> include some diagnostics that are part of clang proper and that clang-tidy
>> can warn about, though we can also acquire a list of those and tune them
>> for our needs.
>>

Mime
View raw message