From dff2b4b7d13cf043e2515da0a5de39a768ad0fcb Mon Sep 17 00:00:00 2001 From: Dennis Klein Date: Sat, 26 Jun 2021 02:57:26 +0200 Subject: [PATCH] feat(tidy): Add new FairMQTidy.cmake module --- CMakeLists.txt | 5 ++ README.md | 3 ++ cmake/FairMQSummary.cmake | 10 +++- cmake/FairMQTidy.cmake | 103 ++++++++++++++++++++++++++++++++++++++ docs/Development.md | 47 ++++++++++++++++- fairmq/tidy/Tool.h | 18 +++++-- fairmq/tidy/runTool.cpp | 2 +- 7 files changed, 181 insertions(+), 7 deletions(-) create mode 100644 cmake/FairMQTidy.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 13c97c22e..f74937fc5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -135,6 +135,11 @@ if(BUILD_DOCS) DESTINATION ${PROJECT_INSTALL_DATADIR}/docs ) endif() +if(BUILD_TIDY_TOOL) + install(FILES cmake/FairMQTidy.cmake + DESTINATION ${PROJECT_INSTALL_CMAKEMODDIR} + ) +endif() include(FairMQPackage) install_cmake_package() diff --git a/README.md b/README.md index 206687f12..6e83d4659 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,9 @@ After the `find_package(FairMQ)` call the following CMake variables are defined: 3. [Introspection](docs/Configuration.md#33-introspection) 4. [Development](docs/Development.md#4-development) 1. [Testing](docs/Development.md#41-testing) + 2. [Static Analysis](docs/Development.md#42-static-analysis) + 1. [CMake Integration](docs/Development.md#421-cmake-integration) + 2. [Extra Compiler Arguments](docs/Development.md#422-extra-compiler-arguments) 5. [Logging](docs/Logging.md#5-logging) 1. [Log severity](docs/Logging.md#51-log-severity) 2. [Log verbosity](docs/Logging.md#52-log-verbosity) diff --git a/cmake/FairMQSummary.cmake b/cmake/FairMQSummary.cmake index b1dc78bad..2eb4d8e96 100644 --- a/cmake/FairMQSummary.cmake +++ b/cmake/FairMQSummary.cmake @@ -97,9 +97,17 @@ macro(fairmq_summary_static_analysis) endforeach() set(static_ana_summary "${BWhite}(${CR}${analyser_list}${BWhite})${CR} (disable with ${BMagenta}-DRUN_STATIC_ANALYSIS=OFF${CR})") else() - set(static_ana_summary "${BRed}OFF${CR} (default, enable with ${BMagenta}-DRUN_STATIC_ANALYSIS=ON${CR})") + set(static_ana_summary "${BRed} NO${CR} (default, enable with ${BMagenta}-DRUN_STATIC_ANALYSIS=ON${CR})") endif() message(STATUS " ${Cyan}RUN STATIC ANALYSIS ${static_ana_summary}") + if(BUILD_TIDY_TOOL) + if(RUN_FAIRMQ_TIDY) + set(tidy_summary "${BGreen}YES${CR} (disable with ${BMagenta}-DRUN_FAIRMQ_TIDY=OFF${CR})") + else() + set(tidy_summary "${BRed} NO${CR} (default, enable with ${BMagenta}-DRUN_FAIRMQ_TIDY=ON${CR})") + endif() + message(STATUS " ${Cyan}RUN fairmq-tidy ${tidy_summary}") + endif() endmacro() macro(fairmq_summary_install_prefix) diff --git a/cmake/FairMQTidy.cmake b/cmake/FairMQTidy.cmake new file mode 100644 index 000000000..2893c8c46 --- /dev/null +++ b/cmake/FairMQTidy.cmake @@ -0,0 +1,103 @@ +################################################################################ +# Copyright (C) 2021 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH # +# # +# This software is distributed under the terms of the # +# GNU Lesser General Public Licence (LGPL) version 3, # +# copied verbatim in the file "LICENSE" # +################################################################################ + +include_guard(GLOBAL) + +#[=======================================================================[.rst: + +``fairmq_target_tidy()`` +======================== + +Runs the FairMQ static analyzer ``fairmq-tidy`` on source files in given +target. + +.. code-block:: cmake + + fairmq_target_tidy(TARGET [EXTRA_ARGS ] + [CLANG_EXECUTABLE ]) + +Registers a custom command that depends on ```` which runs ``fairmq-tidy`` +on the source files that belong to ````. Optional extra arguments +```` are passed to the ``fairmq-tidy`` command-line. + +Requires ``CMAKE_EXPORT_COMPILE_COMMANDS`` to be enabled. + +#]=======================================================================] + +function(fairmq_target_tidy) + cmake_parse_arguments(PARSE_ARGV 0 ARG "" "TARGET;EXTRA_ARGS;CLANG_EXECUTABLE" "") + + if(NOT CMAKE_EXPORT_COMPILE_COMMANDS) + message(AUTHOR_WARNING "CMAKE_EXPORT_COMPILE_COMMANDS is not enabled. Skipping.") + return() + endif() + + if(NOT ARG_TARGET) + message(AUTHOR_WARNING "TARGET argument is required. Skipping.") + return() + endif() + + if(NOT TARGET ${ARG_TARGET}) + message(AUTHOR_WARNING "Given TARGET argument `${ARG_TARGET}` is not a target. Skipping.") + return() + endif() + + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + if(NOT fairmq_tidy_clang_resource_dir) + if(ARG_CLANG_EXECUTABLE) + set(clang_exe ${ARG_CLANG_EXECUTABLE}) + else() + if(TARGET clang) + get_property(clang_exe TARGET clang PROPERTY LOCATION) + else() + set(clang_exe "clang") + endif() + endif() + execute_process( + COMMAND ${clang_exe} -print-resource-dir + OUTPUT_VARIABLE clang_resource_dir + OUTPUT_STRIP_TRAILING_WHITESPACE + ) + set(fairmq_tidy_clang_resource_dir ${clang_resource_dir} + CACHE PATH "fairmq_target_tidy() internal state" FORCE) + endif() + list(APPEND ARG_EXTRA_ARGS + "--extra-arg-before=-I${fairmq_tidy_clang_resource_dir}/include") + endif() + + if(ARG_EXTRA_ARGS) + set(extra 1) + else() + set(extra 0) + endif() + + get_target_property(sources ${ARG_TARGET} SOURCES) + list(FILTER sources INCLUDE REGEX "\.(cpp|cxx)$") + string(REPLACE ":" "-" target_nocolon "${ARG_TARGET}") + + set(src_deps) + foreach(source IN LISTS sources) + string(REPLACE "\/" "-" source_noslash "${source}") + set(src_stamp "${CMAKE_CURRENT_BINARY_DIR}/${target_nocolon}-${source_noslash}.fairmq-tidy") + add_custom_command( + OUTPUT ${src_stamp} + COMMAND $ -p=${CMAKE_BINARY_DIR} + $<${extra}:${ARG_EXTRA_ARGS}> ${source} + COMMAND ${CMAKE_COMMAND} -E touch ${src_stamp} + COMMENT "fairmq-tidy: Analyzing source file '${source}' of target '${ARG_TARGET}'" + DEPENDS ${ARG_TARGET} + WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} + COMMAND_EXPAND_LISTS VERBATIM + ) + list(APPEND src_deps ${src_stamp}) + endforeach() + + if(src_deps) + add_custom_target("fairmq-tidy-${target_nocolon}" ALL DEPENDS ${src_deps}) + endif() +endfunction() diff --git a/docs/Development.md b/docs/Development.md index 52dd2df55..44a09bd7d 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -8,6 +8,51 @@ For unit testing it is often not feasible to boot up a full-blown distributed sy In some scenarios it is useful to not even instantiate a `FairMQDevice` at all. Please see [this example](../test/protocols/_push_pull_multipart.cxx) for single and multi threaded unit test without a device instance. If you store your transport factories and channels on the heap, pls make sure, you destroy the channels before you destroy the related transport factory for proper shutdown. Channels provide all the `Send/Receive` and `New*Message/New*Poller` APIs provided by the device too. -TODO Multiple devices in one process. +## 4.2 Static Analysis + +With `-DBUILD_TIDY_TOOL=ON` you can build the `fairmq-tidy` tool that implements static checks on your source code. To use it, enable the generation of a [compilation database](https://clang.llvm.org/docs/JSONCompilationDatabase.html) in your project via `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON` (generates a file `/compile_commands.json`): + +``` +fairmq-tidy -p mysourcefile.cpp +``` + +If you find any issue (e.g. false positives) with this tool, please tell us by opening an issue on github. + +### 4.2.1 CMake Integration + +When discovering a FairMQ installation in your project, explicitely state, that you want one with the `fairmq-tidy` tool included: + +``` +find_package(FairMQ COMPONENTS tidy_tool) +``` + +Now the CMake module [`FairMQTidy.cmake`](../cmake/FairMQTidy.cmake) is available for inclusion: + +``` +include(FairMQTidy) +``` + +It provides the CMake function `fairmq_target_tidy()` which attaches appropriate `fairmq-tidy` build rules to each source file contained in the passed library or executable target, e.g. if you have an executable that uses FairMQ: + +``` +add_executable(myexe mysource1.cpp mysource2.cpp) +target_link_libraries(myexe PRIVATE FairMQ::FairMQ) +fairmq_target_tidy(TARGET myexe) +``` + +### 4.2.2 Extra Compiler Arguments + +On most Linux distros you are likely to use GCC to compile your projects, so the resulting `compile_commands.json` contains the command-line tuned for GCC which might be missing options needed to successfully invoke the Clang frontend which is used internally by `fairmq-tidy`. In general, you can pass extra `clang` options via the following options: + +``` + --extra-arg= - Additional argument to append to the compiler command line + --extra-arg-before= - Additional argument to prepend to the compiler command line +``` + +E.g. if standard headers are not found, you can hint the location like this: + +``` +fairmq-top -p --extra-arg-before=-I$(clang -print-resource-dir)/include mysourcefile.cpp +``` ← [Back](../README.md) diff --git a/fairmq/tidy/Tool.h b/fairmq/tidy/Tool.h index 00308162e..19f140205 100644 --- a/fairmq/tidy/Tool.h +++ b/fairmq/tidy/Tool.h @@ -9,6 +9,7 @@ #ifndef FAIR_MQ_TIDY_TOOL #define FAIR_MQ_TIDY_TOOL +#include #include #include #include @@ -58,11 +59,20 @@ struct Tool tool.appendArgumentsAdjuster( [](tooling::CommandLineArguments const &_args, StringRef /*file*/) { tooling::CommandLineArguments args(_args); - // TODO add only if cdb was generated with GCC - args.emplace(args.begin() + 1, "-I/usr/lib64/clang/12.0.0/include"); - // TODO add only if missing - args.emplace(args.begin() + 1, "-std=c++17"); + + auto const no_std_arg_present = + std::find_if(cbegin(args), + cend(args), + [](std::string const &arg) { + return arg.find("-std=") != std::string::npos; + }) + == std::cend(args); + if (no_std_arg_present) { + args.emplace(std::cbegin(args) + 1, "-std=c++17"); + } + args.emplace_back("-Wno-everything"); + return args; }); diff --git a/fairmq/tidy/runTool.cpp b/fairmq/tidy/runTool.cpp index d30a63517..2bb082b02 100644 --- a/fairmq/tidy/runTool.cpp +++ b/fairmq/tidy/runTool.cpp @@ -19,7 +19,7 @@ int main(int argc, const char** argv) { // TODO Replace command line parser with CLI11 auto parser(clang::tooling::CommonOptionsParser::create( - argc, argv, ToolCategory, llvm::cl::NumOccurrencesFlag::Optional, "")); + argc, argv, ToolCategory, llvm::cl::NumOccurrencesFlag::ZeroOrMore, "")); if (!parser) { llvm::errs() << parser.takeError(); return EXIT_FAILURE;