Add and Test Clang-tidy

[haytham918]

Addressing the issue in #1060. Added clang-tidy check on files inside "libopenage" directory, focusing on "clang-analyzer", "bugprone", "concurrency", and "performance" issues. @heinezen

[haytham918]

I think make checkall doesn't work for your workflow because it didn't detect installed clang-tidy

[heinezen]

@TheJJ kevin needs clang-tidy to be installed

[heinezen]

@haytham918 You should add clang-tidy as a sanity check dependency in https://github.com/SFTtech/openage/blob/master/doc/building.md#dependencies

[haytham918]

@heinezen Thank you for the notice. I have updated it accordingly.

[heinezen]

@TheJJ Did you add clang-tidy already?

[haytham918]

@heinezen @TheJJ , I hope you are doing well with the new year :), any updates on this request?

[heinezen]

@haytham918 Happy new year! I'm still waiting for @TheJJ to add clang-tidy to the CI machine.

[TheJJ]

It's in the image now :)

[haytham918]

-- The CXX compiler identification is unknown
CMake Error at CMakeLists.txt:34 (project):
  No CMAKE_CXX_COMPILER could be found.

  Tell CMake where to find the compiler by setting either the environment
  variable "CXX" or the CMake cache entry CMAKE_CXX_COMPILER to the full path
  to the compiler, or to the compiler name if it is in the PATH.


-- Configuring incomplete, errors occurred!
Error: Process completed with exit code 1.

This is what I am seeing from the build output, not sure what exactly is going wrong with the configuration? I don't think I have ever modified the CMake files.

[TheJJ]

Kevin is now able to run clang-tidy, but produces many errors.
The cause of this seems to be the invocation of clang-tidy, which must be integrated into the buildsystem (cmake), because only there the correct compiler invocations are available :)

[haytham918]

Now clang-tidy seems to be working and generating warnings?

[simonsan]

I think the compile command database needs to be added like this: https://stackoverflow.com/questions/63944447/how-to-specify-compilation-database-for-clang-tidy#71826250

-p ${CMAKE_BUILD_DIR}, also not -DCMAKE_EXPORT_COMPILE_COMMANDS=ON needs to be set to get the compile_commands.json if not already set.

-p <build-path> is used to read a compile command database.

        For example, it can be a CMake build directory in which a file named
        compile_commands.json exists (use -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
        CMake option to get this output). When no build path is specified,
        a search for compile_commands.json will be attempted through all
        parent paths of the first input file . See:
        https://clang.llvm.org/docs/HowToSetupToolingForLLVM.html for an
        example of setting up Clang Tooling on a source tree.

[TheJJ]

we have the compile command export already. but we need to make clang-tidy to use it, i.e. by integrating it in the buildsystem. it can't run without cmake running before it.

the warnings it currently generates are not helpful since they fail at things like unknown includes, but once it's integrated into the real compile environment, real output is generated :)

[mikonse]

Fyi cmake natively supports running clang tidy as part of the normal build.
If the increased compile time is not a problem this is probably the easiest solution.

See https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_CLANG_TIDY.html

[TheJJ]

Thx, indeed. And now I remember: We actually added this already:

openage/configure

Line 416 in 6597b09

cli.add_argument("--clang-tidy", action="store_true",

openage/CMakeLists.txt

Line 152 in 6597b09

set(CMAKE_CXX_CLANG_TIDY "clang-tidy;-checks=-*,readability-*")

https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_CLANG_TIDY.html
https://cmake.org/cmake/help/latest/prop_tgt/LANG_CLANG_TIDY.html