Re: [ccache] Speeding up builds with clang-tidy, clang-analyzer and iwyu

2020-12-06 Thread Christian Ledergerber via ccache
Hi Steffen,

I was in the same situation as you, porting / cleaning up a legacy code base to 
use clang-tidy. Except that the legacy code base was only a year old.

I ended up writing the following wrapper to cmake's add_executable and 
add_library:

function(add_executable_with_checks)
set(options
DISABLE_CLANG_TIDY_READABILITY
DISABLE_CLANG_TIDY_MODERNIZE
DISABLE_CLANG_TIDY_NAMING_CONVENTION
DISABLE_CLANG_TIDY_BUGPRONE
DISABLE_CLANG_ANALYZER
DISABLE_CLANG_TIDY_CPP_CORE_GUIDELINES
DISABLE_COMPILER_ERROR)
set(oneValueArgs PROJECT_NAME)
set(multiValueArgs
SOURCES
HEADERS)
cmake_parse_arguments(ARG "${options}" "${oneValueArgs}" 
"${multiValueArgs}" ${ARGN})

...
add_executable(${ARG_PROJECT_NAME} ${ARG_SOURCES})
...

endfunction()

this allows me to mix "ugly" code with "pretty" code in the same project in a 
very flexible manner on a module / library basis. Any module for which one of 
the things has been cleaned up we enable the automatic check and hence never go 
backwards. This way at least I got clang-format into place and most of the code 
is now using some form of clang-tidy checking.

Thanks for your idea on the wrapper script. Did you end up trying it? I still 
plan to attack this problem one day, but the wrapper script sounds a lot easier.
I guess the drawback is that things are not recompiled if you change the 
clang-tidy configuration. But maybe this can be fixed somehow or we could live 
with this.

Christian


From: ccache  on behalf of Steffen Dettmer via 
ccache 
Sent: Friday, December 4, 2020 17:00
To: ccache list
Subject: Re: [ccache] Speeding up builds with clang-tidy, clang-analyzer and 
iwyu

Hi,

On Sun, Aug 2, 2020 at 6:55 PM Christian Ledergerber via ccache <
ccache@lists.samba.org> wrote:
> for our builds currently clang-tidy is currently the limiting factor in
> terms of speed.
>
> There is one utility out there which tries to cache the output of
> clang-tidy, however it seems to be stale:
> https://github.com/ejfitzgerald/clang-tidy-cache
>
> Yet, I have the impression that my case is much simpler:

Do you have any new regarding caching clang-tidy?

Here we also occasionally discuss things and we found the following
opinions in our teams:
1) clang-tidy shall be in the IDE: it can apply fixes which needs
interaction
2) clang-tidy shall be a Jenkins job: When you are using it for a while,
number of warnings reduce a lot and it is not necessary to run it every time
3) clang-tidy shall be ran at every compilation: it is -Wall -Wextra -Wtidy
made correctly

For 3), which seems to be close to your use case, someone proposed in
context of a legacy project to run clang-tidy along with the compiler. The
discussed approach was to have a wrapper script to be used as a compiler,
but the wrapper script also called clang-tidy before it ran the compiler.

In this case, ccache would not call the compiler in case of a cache hit,
which here means it would not call the wrapper script and thus not
clang-tidy, so I think this could help you. Maybe.

(the idea was quickly discarded because there was absolutely no way to fix
the legacy code due to its ancient C++ and we joked about adding a
--museum-mode to clang-tidy :))

Steffen
___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache

___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] Speeding up builds with clang-tidy, clang-analyzer and iwyu

2020-12-04 Thread Steffen Dettmer via ccache
Hi,

On Sun, Aug 2, 2020 at 6:55 PM Christian Ledergerber via ccache <
ccache@lists.samba.org> wrote:
> for our builds currently clang-tidy is currently the limiting factor in
> terms of speed.
>
> There is one utility out there which tries to cache the output of
> clang-tidy, however it seems to be stale:
> https://github.com/ejfitzgerald/clang-tidy-cache
>
> Yet, I have the impression that my case is much simpler:

Do you have any new regarding caching clang-tidy?

Here we also occasionally discuss things and we found the following
opinions in our teams:
1) clang-tidy shall be in the IDE: it can apply fixes which needs
interaction
2) clang-tidy shall be a Jenkins job: When you are using it for a while,
number of warnings reduce a lot and it is not necessary to run it every time
3) clang-tidy shall be ran at every compilation: it is -Wall -Wextra -Wtidy
made correctly

For 3), which seems to be close to your use case, someone proposed in
context of a legacy project to run clang-tidy along with the compiler. The
discussed approach was to have a wrapper script to be used as a compiler,
but the wrapper script also called clang-tidy before it ran the compiler.

In this case, ccache would not call the compiler in case of a cache hit,
which here means it would not call the wrapper script and thus not
clang-tidy, so I think this could help you. Maybe.

(the idea was quickly discarded because there was absolutely no way to fix
the legacy code due to its ancient C++ and we joked about adding a
--museum-mode to clang-tidy :))

Steffen
___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] Speeding up builds with clang-tidy, clang-analyzer and iwyu

2020-08-18 Thread Joel Rosdahl via ccache
On Mon, 17 Aug 2020 at 15:34, Christian Ledergerber
 wrote:
> [...] The point is: I think this is not a coner-case, but people use this
> configuration abundantly.

I just wanted to stress the point that tools need to handle all configurations
correctly, regardless of whether they are popular or not.

> > 3. The method is also brittle since the clang-tidy step will be skipped if
> > the source is built (and therefore cached) by a normal non-clang-tidy
> > build.
>
> This I do not understand. Would this not be caught by the extra files to
> cache? So: if the ccache caches also the configuration of clang-tidy then
> there cannot be a cache hit due to binaries built without using clang-tidy?

The case I was thinking about is this: if a user sets CCACHE_EXTRAFILES (or
extra_files_to_hash in the configuration) to the .clang-tidy configuration file
and builds without running clang-tidy then user will have circumvented the
logic by placing a result in the cache without being stopped by clang-tidy.
This likely won't happen in practice; I'm just trying to find out if there are
edge cases where the assumptions can break down.

> Thanks for this comment! I feel like for easy integration into cmake (and
> other tools) one would need another command line flag which allows adding
> extra files to cache. E.g. --add-extra-files=file1;file2;file3.

That would be possible.

The main thing I think feels problematic with your idea is that it will require
tight coupling between CMake and ccache, something I wouldn't like to have if I
were a CMake maintainer. But since I'm not, I'll leave that discussion to you
and them.

-- Joel

___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] Speeding up builds with clang-tidy, clang-analyzer and iwyu

2020-08-17 Thread Christian Ledergerber via ccache

Hi Joel,

1. The solution will require a specific ccache version to work at all.
True, a good implementation should warn about a too old ccache version 
and fail or fall back to running clang-tidy always.

2. The method of using ccache to probe whether clang-tidy should be
run won't work for users who don't use warnings-as-errors. Also, how
would CMake know that warnings-as-errors is used? It could be part of
WarningsAsErrors in the config file or it could be on the command line
but only for some warnings, etc. Sounds brittle and complex.
I was thinking that the user should be able to choose whether clang-tidy 
should be run on a ccache hit or not. This he could specify by setting a 
cmake variable. E.g. CMAKE_CLANG_TIDY_ON_CCACHE_HIT. This variable would 
by default be false to guarantee backwards compatibility of cmake.


I personally feel like using warning-as-errors is the only reasonable 
way to use any linting tool. For the same reasons why you should use a 
compiler using -Wall -Werror. This is beside the discussion here. The 
point is: I think this is not a coner-case, but people use this 
configuration abundantly.



3. The method is also brittle since the clang-tidy step will be
skipped if the source is built (and therefore cached) by a normal
non-clang-tidy build.
This I do not understand. Would this not be caught by the extra files to 
cache? So: if the ccache caches also the configuration of clang-tidy 
then there cannot be a cache hit due to binaries built without using 
clang-tidy?

(But if you apply your changes to a local CMake version you can make
any assumptions you want, of course.)
I spontaneously think that it would be a better idea to either (a)
stop running clang-tidy for a normal build and instead run it as a
special build once in a while, or (b) improve
https://github.com/ejfitzgerald/clang-tidy-cache if you find it
deficient in some way (I know nothing about it), or (c) implement
clang-tidy support in ccache or another compiler cache program.


We do this already for clang-analyzer: it runs only on jenkins. But its 
quite easy to mess up something with clang-tidy. This means that you 
will most likely need to fix things in your PR. This then means you need 
to run clang-tidy on almost every commit anyway and it generates quite 
some work to clean up the git history.


As pointed in the title of this thread I believe that this approach 
scales to more than just clang-tidy. I wish to also add iwyu to our 
build system and will face the same problems: it will take almost the 
same amount of time as compiling the code because the code needs to go 
through the first few stages of clang (parsing, building an ast and the 
like). Hence, fixing clang-tidy-cache does not appeal to me as it 
doesn't scale. I feel like the algorithm discussed so far applies to any 
kind of static analysis tool / linter:


- use the tool with warnings-as-errors
- add the configuration of the tool (and the tool itself??) to the 
ccache extra files

- run it only if ccache has no hit
- run the compiler only if all static analysis passed



Just a comment on your CMake GitLab issue:


2. Add Class.clang-tidy to configuration of extra_files_to_hash of ccache using 
the command line syntax: --set-config=KEY=VALUE see 
https://ccache.dev/manual/latest.html

That's not a good idea. --set-config is just a convenience alternative
to manually editing the ccache configuration file. If you run it once
for each compilation you will have race conditions between invocations
and you will mess up the user's ccache configuration. To enable some
ccache setting for a specific compilation you should set the
corresponding environment variable, i.e. CCACHE_EXTRAFILES in this
case.
Thanks for this comment! I feel like for easy integration into cmake 
(and other tools) one would  need another command line flag which allows 
adding extra files to cache. E.g. --add-extra-files=file1;file2;file3.



-- Joel


___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] Speeding up builds with clang-tidy, clang-analyzer and iwyu

2020-08-14 Thread Joel Rosdahl via ccache
On Fri, 14 Aug 2020 at 08:39, Christian Ledergerber
 wrote:
>
> I have created the following issue for cmake to launch the discussion:
> https://gitlab.kitware.com/cmake/cmake/-/issues/21092

When you described your idea I didn't think you meant making changes
for incorporation in the upstream CMake software but to some local
CMake version. I think that it will be hard to implement it in a
generic enough way for CMake since

1. The solution will require a specific ccache version to work at all.
2. The method of using ccache to probe whether clang-tidy should be
run won't work for users who don't use warnings-as-errors. Also, how
would CMake know that warnings-as-errors is used? It could be part of
WarningsAsErrors in the config file or it could be on the command line
but only for some warnings, etc. Sounds brittle and complex.
3. The method is also brittle since the clang-tidy step will be
skipped if the source is built (and therefore cached) by a normal
non-clang-tidy build.

(But if you apply your changes to a local CMake version you can make
any assumptions you want, of course.)

I spontaneously think that it would be a better idea to either (a)
stop running clang-tidy for a normal build and instead run it as a
special build once in a while, or (b) improve
https://github.com/ejfitzgerald/clang-tidy-cache if you find it
deficient in some way (I know nothing about it), or (c) implement
clang-tidy support in ccache or another compiler cache program.

Just a comment on your CMake GitLab issue:

> 2. Add Class.clang-tidy to configuration of extra_files_to_hash of ccache 
> using the command line syntax: --set-config=KEY=VALUE see 
> https://ccache.dev/manual/latest.html

That's not a good idea. --set-config is just a convenience alternative
to manually editing the ccache configuration file. If you run it once
for each compilation you will have race conditions between invocations
and you will mess up the user's ccache configuration. To enable some
ccache setting for a specific compilation you should set the
corresponding environment variable, i.e. CCACHE_EXTRAFILES in this
case.

-- Joel

___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] Speeding up builds with clang-tidy, clang-analyzer and iwyu

2020-08-14 Thread Christian Ledergerber via ccache

I have created the following issue for cmake to launch the discussion:
https://gitlab.kitware.com/cmake/cmake/-/issues/21092

On 04.08.20 19:46, Joel Rosdahl wrote:

On Sun, 2 Aug 2020 at 19:49, Christian Ledergerber via ccache
 wrote:

In the mean-time I have been thinking some more and what I am afraid of
is that the result will not be correct if the flags to clang-tidy change.

A typical command run by make looks like this:

cd /home/dev/erx-soft/cmake-build-debug/modules/utils && ccache
/home/dev/clion-2020.1.3/bin/cmake/linux/bin/cmake -E __run_co_compile [...]

If I interpret the example command correctly ccache is used as a
prefix to cmake? That won't have any effect; ccache can't cache the
result of cmake so it will just fall back to executing cmake with the
given arguments (and increase some statistics counter).


--launcher=ccache

I guess this means that cmake will execute "ccache $compiler ..." later.


[...] In other words: it seems like ccache could also hash the flags to
clang-tidy. The question: does it?

No, since ccache doesn't see any clang-tidy arguments when cmake
executes "ccache $compiler ..." as mentioned above.


Now that I am looking at the above command line: Will this require to
also modify the cmake binary to generate other command lines?

I have no idea, but it sounds likely.

Here's an idea to consider: You could write the clang-tidy arguments
to a file and set CCACHE_EXTRAFILES to the file. Or just set it to
preexisting .clang-tidy files? Then the ccache results will be
invalidated when the clang-tidy arguments change. That in combination
with the hypothetical mode where ccache would exit with failure on a
cache miss would maybe enough for your initial idea.

-- Joel


___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] Speeding up builds with clang-tidy, clang-analyzer and iwyu

2020-08-04 Thread Joel Rosdahl via ccache
On Sun, 2 Aug 2020 at 19:49, Christian Ledergerber via ccache
 wrote:
> In the mean-time I have been thinking some more and what I am afraid of
> is that the result will not be correct if the flags to clang-tidy change.
>
> A typical command run by make looks like this:
>
> cd /home/dev/erx-soft/cmake-build-debug/modules/utils && ccache
> /home/dev/clion-2020.1.3/bin/cmake/linux/bin/cmake -E __run_co_compile [...]

If I interpret the example command correctly ccache is used as a
prefix to cmake? That won't have any effect; ccache can't cache the
result of cmake so it will just fall back to executing cmake with the
given arguments (and increase some statistics counter).

> --launcher=ccache

I guess this means that cmake will execute "ccache $compiler ..." later.

> [...] In other words: it seems like ccache could also hash the flags to
> clang-tidy. The question: does it?

No, since ccache doesn't see any clang-tidy arguments when cmake
executes "ccache $compiler ..." as mentioned above.

> Now that I am looking at the above command line: Will this require to
> also modify the cmake binary to generate other command lines?

I have no idea, but it sounds likely.

Here's an idea to consider: You could write the clang-tidy arguments
to a file and set CCACHE_EXTRAFILES to the file. Or just set it to
preexisting .clang-tidy files? Then the ccache results will be
invalidated when the clang-tidy arguments change. That in combination
with the hypothetical mode where ccache would exit with failure on a
cache miss would maybe enough for your initial idea.

-- Joel

___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] Speeding up builds with clang-tidy, clang-analyzer and iwyu

2020-08-02 Thread Christian Ledergerber via ccache

Thank you for your answer.

In the mean-time I have been thinking some more and what I am afraid of 
is that the result will not be correct if the flags to clang-tidy change.


A typical command run by make looks like this:

cd /home/dev/erx-soft/cmake-build-debug/modules/utils && ccache 
/home/dev/clion-2020.1.3/bin/cmake/linux/bin/cmake -E __run_co_compile 
--launcher=ccache 
--tidy="clang-tidy-10;-warnings-as-errors=*;-header-filter=/home/dev/erx-soft/modules/utils/*;--checks=-*,bugprone-argument-comment,bugprone-assert-side-effect,bugprone-bad-signal-to-kill-thread,bugprone-bool-pointer-implicit-conversion,bugprone-branch-clone,bugprone-copy-constructor-init,bugprone-dangling-handle,bugprone-dynamic-static-initializers,bugprone-exception-escape,bugprone-fold-init-type,bugprone-forward-declaration-namespace,bugprone-forwarding-reference-overload,bugprone-inaccurate-erase,bugprone-incorrect-roundings,bugprone-infinite-loop,bugprone-integer-division,bugprone-lambda-function-name,bugprone-macro-parentheses,bugprone-macro-repeated-side-effects,bugprone-misplaced-operator-in-strlen-in-alloc,bugprone-misplaced-pointer-arithmetic-in-alloc,bugprone-misplaced-widening-cast,bugprone-move-forwarding-reference,bugprone-multiple-statement-macro,bugprone-not-null-terminated-result,bugprone-parent-virtual-call,bugprone-posix-return,bugprone-reserved-identifier,bugprone-signed-char-misuse,bugprone-sizeof-container,bugprone-sizeof-expression,bugprone-spuriously-wake-up-functions,bugprone-string-constructor,bugprone-string-integer-assignment,bugprone-string-literal-with-embedded-nul,bugprone-suspicious-enum-usage,bugprone-suspicious-include,bugprone-suspicious-memset-usage,bugprone-suspicious-missing-comma,bugprone-suspicious-semicolon,bugprone-suspicious-string-compare,bugprone-swapped-arguments,bugprone-terminating-continue,bugprone-throw-keyword-missing,bugprone-too-small-loop-variable,bugprone-undefined-memory-manipulation,bugprone-undelegated-constructor,bugprone-unhandled-self-assignment,bugprone-unused-raii,bugprone-unused-return-value,bugprone-use-after-move,bugprone-virtual-near-miss" 
--source=/home/dev/erx-soft/modules/utils/src/DevSerialHelper.cpp -- 
/usr/bin/clang++-10 -DASSETS_LOCATION_DIR=\"/home/dev/erx-soft/assets/\" 
-DCPU_ONLY -DLOG_DIR=\"/mnt/erxssd/runs/current/\" 
-DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_TRACE -DSPDLOG_COMPILED_LIB 
-DSPDLOG_FMT_EXTERNAL -I/home/dev/erx-soft/cmake-build-debug 
-I/home/dev/erx-soft/modules/utils/include 
-I/home/dev/erx-soft/modules/utils/src 
-I/home/dev/erx-soft/libs/serializable_objects/cpp/include 
-I/home/dev/erx-soft/cmake-build-debug/libs/serializable_objects/cpp/generated_fbs 
-I/home/dev/erx-soft/external/jsoncpp/src/lib_json/../../include 
-I/home/dev/erx-soft/cmake-build-debug/external/jsoncpp/include/json 
-I/home/dev/erx-soft/external/spdlog/include 
-I/home/dev/erx-soft/external/fmt/include 
-I/home/dev/erx-soft/modules/logger/include -isystem /opt/erx/include 
-isystem /opt/erx/include/opencv  -Werror -Wall -Wshadow-all 
-Wunreachable-code -Wno-error=undefined-inline 
-Wno-error=#pragma-messages -fno-limit-debug-info -fcolor-diagnostics 
-g   -Werror -pthread -std=gnu++17 -o 
CMakeFiles/utils.dir/src/DevSerialHelper.cpp.o -c 
/home/dev/erx-soft/modules/utils/src/DevSerialHelper.cpp



In other words: it seems like ccache could also hash the flags to 
clang-tidy. The question: does it?


Now that I am looking at the above command line: Will this require to 
also modify the cmake binary to generate other command lines?


Maybe I'll find the time to look more into this in two weeks.

Regards,
Christian

On 02.08.20 19:28, Joel Rosdahl wrote:

On Sun, 2 Aug 2020 at 18:55, Christian Ledergerber via ccache
 wrote:

[...]
To me it seems like the following should work:

1. try to use ccache

2. if no hit:

- run clang-tidy

- run clang

For this I would need to know whether ccache generated a hit - lets say
as return code of the executable.

I don't think there's a good way of accomplishing this without adding
code to ccache to optionally exit with failure on a cache miss. But it
would be easy to add it.

Regards,
Joel



___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] Speeding up builds with clang-tidy, clang-analyzer and iwyu

2020-08-02 Thread Joel Rosdahl via ccache
On Sun, 2 Aug 2020 at 18:55, Christian Ledergerber via ccache
 wrote:
> [...]
> To me it seems like the following should work:
>
> 1. try to use ccache
>
> 2. if no hit:
>
> - run clang-tidy
>
> - run clang
>
> For this I would need to know whether ccache generated a hit - lets say
> as return code of the executable.

I don't think there's a good way of accomplishing this without adding
code to ccache to optionally exit with failure on a cache miss. But it
would be easy to add it.

Regards,
Joel

___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache