[Development] Qt API review with clazy

Ch'Gans chgans at gna.org
Mon Sep 12 11:56:50 CEST 2016


On 12 September 2016 at 21:13, Mathias Hasselmann
<mathias at taschenorakel.de> wrote:
> Hello Chris,
>
>> On 12 September 2016 at 19:06, Mathias Hasselmann
>> <mathias at taschenorakel.de> wrote:
>>>
>>> Just that the approach of clang-tidy is fundamentally wrong:
>>
>>
>> Hi Mathias,
>>
>> No offense, but you arguments are fundamentally wrong too. Your
>> workflow is not everyone's workflow!
>>
>>>
>>> You simply don't do static checks as a after thought, at random times
>>> when
>>> sun, mars and moon are in proper constellation.
>>
>>
>> I don't see the problem, people are free to choose if, when and which
>> checks they fancy run.
>>
>>> Why? Because when running this checks occasionally too much cruft will
>>> have
>>> accumulated that it is worth and reasonable to fix those issues: Too big
>>> chunk of boring and still expensive work. Too big risk to introduce
>>> regressions.
>>
>>
>> Running clang-tidy is way slower than compiling code, a nightly check
>> might be enough (or not, again this is a personal choice), or you
>> could decide to run the checks systematically on pull requests because
>> you don't trust the authors (for whatever reason, inc the usual "oops,
>> forgot to do that", human do errors).
>
>> Sanity checks ran by clang-tidy can be of arbitrary complexity, and as
>> such can take really long time. You might not want to be slow down by
>> this while you're actively developing.
>
> Well, with clang-tidy being slow you just gave another reason to have a
> compiler plugin for this quick, very obvious tests. In my experience clazy
> has no measurable impact to compile time.

I never said that clazy was not useful, I was just saying that you
should use the right tool for the right job. Both tools are
complimentary, people have different use-cases.

>
>> If you go this way, then why don't you always run your applications in
>> profiling mode to make sure you don't introduce bottle-necks or memory
>> leaks "as-you-go"? Well we all know the answer: Because it is way too
>> slow.
>
>
> Besides clazy being blasting fast you are comparing apples with oranges. The
> issues clazy blames are plain Qt usage bugs. Just the same kind of issues
> the compiler reports for regular C++ already. Ideally the compilers would
> checks those issues out of the box already. Obviously they can't because Qt
> layers stuff on top of C++.

Effectively, I'm not sure we're talking about the same things here.
Maybe clazy plugin could be even use by Qtc clang code model if it's
that fast (I really like the "compile errors/warnings" as you type,
altho not perfect). I was talking about "deeper" checks, like the ones
listed in the "group of checks" table as per
http://clang.llvm.org/extra/clang-tidy/index.html#using-clang-tidy
They are really more than what clazy proposes (for now). And you
definitely don't want them to be run each time you build your app.

So, all in all, it looks to me that the clazy plugin is the way to go
for fast checks, and clang-tidy for the more thoughtfully ones. Hence
my message about being able to use these new checks both form the
compiler command and from an external tool such as clang-tidy. Deeper,
time-consuming, Qt-specific checks would be welcome as a new
clang-tidy check group.
The checks clang-tidy propose are not limited to "potential bugs", it
does cover as well coding guideline and styles and AFAIR Qt use custom
perl scripts for style and basic errors checking (might have changed
since last time i checked).

Cherry on the cake, with clang-tidy (actually a llvm feature used by
clang-tidy) is that it can generates the fixes as well, and even apply
them in one go. A feature which I believe is used by QtC to propose
you the "hot fix" accessible with Alt-Return (On Linux).

"clang-tidy -checks=* -list-checks" here gives me a list of 184 checks
(LLVM v3.8.0). Not all are useful and some of them are not always
applicable or sometimes give false positive, but it should give you a
fair idea of what does already exists in there.

Chris

>
> Ciao,
> Mathias



More information about the Development mailing list