[Bf-committers] Clang-Tidy: Request for Feedback

Sybren A. Stüvel sybren at blender.org
Thu Mar 4 11:08:15 CET 2021


On Thu, 4 Mar 2021 at 09:53, Sergey Sharybin via Bf-committers <
bf-committers at blender.org> wrote:

> - Do you have a strong feeling about leaving a .clang-tidy file as it is
> now (where file modification requires manual re-compilation) ?
>

No, not really. Of course it would be nice if changes are picked up
automatically, but I don't have a problem with an occasional clean +
rebuild.


> - What do you think we still need to do before we can call the project
> done?
>

There are a few rules mentioned in T78535 that haven't been crossed off
yet, but also have no clear status:
- readability-non-const-parameter and modernize-use-equals-default: seem to
still be in progress
- modernize-loop-convert: I worked on that before, but probably won't have
time to continue on it this Friday. I have some work for the studio that
has a clear deadline; I'll catch up on code quality work after that's been
delivered.
- modernize-pass-by-value: needs a decision on whether we want it or not.

I think these need a finalizing decision, to either continue & finish the
work, or a decision that it won't be done. After that I think we can call
the project finished.

- Shall we enable it by default? Maybe for `make developer` ?
>

To see the impact of that, I did a quick (N=1) test on my machine. I'm
using GCC 9.3 with CCache on the master branch. Both tests have had a full
build with this combo before measuring the build timing, so the CCache
cache is up to date. This to me is resembling my personal development
situation more than doing clean builds without caching.

Building without clang-tidy took 152 sec (wallclock time).
Building with clang-tidy took 275 sec (wallclock time).

This means that it's very roughly twice as slow to build with clang-tidy
enabled. To me this means that I won't be using it for regular builds in my
code-build-test cycle. If other developers feel the same way, that I expect
them to disable it right after they run `make developer`, in which case
it's probably better to just disable it by default.

- Any other relevant feedback?
>

IMO we should configure the buildbots to run with clang-tidy enabled. That
way violations of the rules are noted, even when most devs work with
clang-tidy disabled. Case in point: I had to fix one error before I could
run the above timing test.

Sybren


More information about the Bf-committers mailing list