DevHeads.net

Review Request 119875: add support for Clang Static Analyzer and improve build configuration

Review request for KDE Base Apps.

Repository: kde-baseapps

Description
This patch add the support of the Clang Static Analyzer (<a href="http://clang-analyzer.llvm.org/" title="http://clang-analyzer.llvm.org/">http://clang-analyzer.llvm.org/</a>) implemented using a CMake target.
For each project in the main tree, it creates a <project_name>_static_analysis target which sets the c++-analyzer compiler
and then call scan-build. (see the cmake module for details)

As I couldn't find how to change the compiler for a specific target in CMake, I had to create another a CMake sub-build tree
named static_analysis_named in the main CMake build tree, and then reconfigure the project by forcing
the C++ compiler to c++-analyzer.

Also I added a configure.sh script that allow to set the build options very easily, using dialog.
So, new developers can change the compiler or set the install prefix without having to know
the specific CMake variables.

And finally i added a root Makefile, so you don't have to stay in the build directory to
compile your targets.

Diffs
CMakeLists.txt b06ba01fff7cee445fcc0b896cef041bd4c34fc8
Makefile PRE-CREATION
cmake/modules/clang_static_analysis.cmake PRE-CREATION
configure.sh PRE-CREATION

Diff: <a href="https://git.reviewboard.kde.org/r/119875/diff/" title="https://git.reviewboard.kde.org/r/119875/diff/">https://git.reviewboard.kde.org/r/119875/diff/</a>

Testing
For configure.sh :
./configure.sh
./configure.sh -c
./configure.sh -b
./configuer.sh -p

for clang static analyzer
./configure.sh
make dolphin_static_analysis

Thanks,

Mathieu Tarral

Comments

Re: Review Request 119875: add support for Clang Static Analyzer

By Albert Astals Cid at 08/20/2014 - 18:15

Why is this per repo? This should be in a place all repos get the nice stuff without having to do work.

A static Makefile and a configure.sh seem like very bad ideas to me. Why do you need those over cmake?

- Albert Astals Cid

On ago. 20, 2014, 10:11 p.m., Mathieu Tarral wrote:

Re: Review Request 119875: add support for Clang Static Analyzer

By Albert Astals Cid at 08/21/2014 - 13:20

You should aim as exposing it as buildtype or similar like milian says, one target that sets the proper CXX and linker flags for you.

- Albert

On ago. 20, 2014, 10:11 p.m., Mathieu Tarral wrote:

Re: Review Request 119875: add support for Clang Static Analyzer

By Milian Wolff at 08/21/2014 - 04:47

Oh and FTR: afaik one can also do something like this already:

mkdir build-asan
cd build-asan
CXX=clang++ -fsanitize=address cmake ..
make

But maybe it won't pass the proper linker flags then. But I'm sure there are ways to do that as well.

- Milian

On Aug. 20, 2014, 10:11 p.m., Mathieu Tarral wrote:

Re: Review Request 119875: add support for Clang Static Analyzer

By Milian Wolff at 08/21/2014 - 04:43

The makefile and configure script are a really bad idea. Maybe you think its simpler for newcomers, I find it highly confusing. You need to know CMake in KDE, so use CMake everywhere. Furthermore, this will otherwise not work on Windows etc.

Generally, I think such a feature should probably go into CMake itself, or be put into our ECM if possible. The different analysers (note: there are multiple, just one 'analysis' target is not enough) should be chosen by setting CMAKE_BUILD_TYPE to something like TSAN or ASAN or ...

- Milian

On Aug. 20, 2014, 10:11 p.m., Mathieu Tarral wrote:

Re: Review Request 119875: add support for Clang Static Analyzer

By Mathieu Tarral at 08/20/2014 - 18:31

Because i didn't know how to implement it in another way.
Another idea is to implement it as a plugin in the krazy framework,
so you can run it over any KDE repo without having them to implement it in their code.

For new developers, it avoids them to learn some CMake specific variables like CMAKE_INSTALL_PREFIX and CMAKE_CXX_COMPILER.
And for developers in general, it allows them to configure the project really quickly.
It's like a very little abstraction to the CMake interface.

- Mathieu

On août 20, 2014, 10:11 après-midi, Mathieu Tarral wrote: