From f9854f8e36c3bd5c3c18cb0e3a9c6d3c537768ba Mon Sep 17 00:00:00 2001 From: Ben Deane Date: Tue, 22 Oct 2024 09:11:38 -0600 Subject: [PATCH 1/3] :bug: :rotating_light: Fix clang-tidy Problem: - The `AnalyzeTemporaryDtors` option was **removed** in clang-tidy-18, which means that clang-tidy was failing to parse the .clang-tidy file. - Incredibly, clang-tidy exits with **success** if it fails to parse the .clang-tidy file! Therefore all clang-tidy runs were passing without actually tidying anything. Solution: - Remove `AnalyzeTemporaryDtors` option from .clang-tidy --- .clang-tidy | 1 - 1 file changed, 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index d209905..ae53f47 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -36,6 +36,5 @@ Checks: > -readability-uppercase-literal-suffix WarningsAsErrors: '*' HeaderFilterRegex: '' -AnalyzeTemporaryDtors: false FormatStyle: file ... From f1e4f3b1bc911f3905f5e5c93eed6e71e1a04015 Mon Sep 17 00:00:00 2001 From: Ben Deane Date: Tue, 22 Oct 2024 09:12:00 -0600 Subject: [PATCH 2/3] :rotating_light: Update `clang-tidy` checks Problem: - Some checks are aliases for other checks. `clang-tidy` does not deduplicate such checks; it just runs the same check twice - Some things have changed since last updating `clang-tidy` checks. - The `clang-tidy` checks that are configured are not documented. Solution: - Remove alias checks. - Update a few other checks. - Add documentation for configured checks. --- .clang-tidy | 24 ++++++++---- docs/quality.adoc | 95 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 8 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index ae53f47..f69cb6c 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -3,26 +3,33 @@ Checks: > boost-use-to-string, bugprone-*, -bugprone-easily-swappable-parameters, + -bugprone-narrowing-conversions, clang-diagnostic-*, clang-analyzer-*, cppcoreguidelines-*, + -cppcoreguidelines-avoid-c-arrays, -cppcoreguidelines-avoid-magic-numbers, -cppcoreguidelines-avoid-non-const-global-variables, - -cppcoreguidelines-macro-usage, - -cppcoreguidelines-pro-bounds-pointer-arithmetic, + -cppcoreguidelines-c-copy-assignment-signature, + -cppcoreguidelines-explicit-virtual-functions, + -cppcoreguidelines-macro-to-enum, -cppcoreguidelines-missing-std-forward, + -cppcoreguidelines-noexcept-destructor, + -cppcoreguidelines-noexcept-move-operations, + -cppcoreguidelines-noexcept-swap, + -cppcoreguidelines-non-private-member-variables-in-classes, + -cppcoreguidelines-pro-bounds-pointer-arithmetic, + -cppcoreguidelines-use-default-member-init, google-build-explicit-make-pair, + google-build-namespaces, google-default-arguments, google-explicit-constructor, google-readability-casting, google-runtime-int, hicpp-signed-bitwise, - misc-misplaced-const, - misc-non-copyable-objects, - misc-redundant-expression, - misc-static-assert, - misc-throw-by-value-catch-by-reference, - misc-uniqueptr-reset-release, + misc-*, + -misc-include-cleaner, + -misc-non-private-member-variables-in-classes, modernize-*, -modernize-concat-nested-namespaces, performance-*, @@ -33,6 +40,7 @@ Checks: > -readability-magic-numbers, -readability-named-parameter, -readability-qualified-auto, + -readability-redundant-inline-specifier, -readability-uppercase-literal-suffix WarningsAsErrors: '*' HeaderFilterRegex: '' diff --git a/docs/quality.adoc b/docs/quality.adoc index 4dbf523..1f07694 100644 --- a/docs/quality.adoc +++ b/docs/quality.adoc @@ -119,6 +119,101 @@ If you are not building with clang, the `clang-tidy` target will do nothing. NOTE: As with `clang-format`, for preference, we'll find the `clang-tidy` that exists alongside the compiler being used. +=== Enabled clang-tidy checks + +The following https://clang.llvm.org/extra/clang-tidy/checks/list.html[`clang-tidy` check] categories are enabled: + +* bugprone-* +* clang-diagnostic-* +* clang-analyzer-* +* cppcoreguidelines-* +* misc-* +* modernize-* +* performance-* +* portability-* +* readability-* + +In addition, the following specific checks are enabled: + +* https://clang.llvm.org/extra/clang-tidy/checks/boost/use-to-string.html[boost-use-to-string] +* https://clang.llvm.org/extra/clang-tidy/checks/google/build-explicit-make-pair.html[google-build-explicit-make-pair] +* https://clang.llvm.org/extra/clang-tidy/checks/google/build-namespaces.html[google-build-namespaces] +* https://clang.llvm.org/extra/clang-tidy/checks/google/default-arguments.html[google-default-arguments] +* https://clang.llvm.org/extra/clang-tidy/checks/google/explicit-constructor.html[google-explicit-constructor] +* https://clang.llvm.org/extra/clang-tidy/checks/google/readability-casting.html[google-readability-casting] +* https://clang.llvm.org/extra/clang-tidy/checks/google/runtime-int.html[google-runtime-int] +* https://clang.llvm.org/extra/clang-tidy/checks/hicpp/signed-bitwise.html[hicpp-signed-bitwise] + +The following specific checks are _disabled_ because they are aliases for other +checks, and clang-tidy does not deduplicate them: + +* https://clang.llvm.org/extra/clang-tidy/checks/bugprone/narrowing-conversions.html[bugprone-narrowing-conversions] + aliases + https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/narrowing-conversions.html[cpp-coreguidelines-narrowing-conversions] +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-c-arrays.html[cppcoreguidelines-avoid-c-arrays] + aliases + https://clang.llvm.org/extra/clang-tidy/checks/modernize/avoid-c-arrays.html[modernize-avoid-c-arrays] +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-magic-numbers.html[cppcoreguidelines-avoid-magic-numbers] + aliases + https://clang.llvm.org/extra/clang-tidy/checks/readability/magic-numbers.html[readability-magic-numbers] +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/c-copy-assignment-signature.html[cppcoreguidelines-c-copy-assignment-signature] + aliases + https://clang.llvm.org/extra/clang-tidy/checks/misc/unconventional-assign-operator.html[misc-unconventional-assignment-operator] +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/explicit-virtual-functions.html[cppcoreguidelines-explicit-virtual-functions] + aliases + https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-override.html[modernize-use-override] +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/macro-to-enum.html[cppcoreguidelines-macro-to-enum] + aliases + https://clang.llvm.org/extra/clang-tidy/checks/modernize/macro-to-enum.html[modernize-macro-to-enum] +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/noexcept-destructor.html[cppcoreguidelines-noexcept-destructor] + aliases + https://clang.llvm.org/extra/clang-tidy/checks/performance/noexcept-destructor.html[performance-noexcept-destructor] +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/noexcept-move-operations.html[cppcoreguidelines-noexcept-move-operations] + aliases + https://clang.llvm.org/extra/clang-tidy/checks/performance/noexcept-move-constructor.html[performance-noexcept-move-constructor] +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/noexcept-swap.html[cppcoreguidelines-noexcept-swap] + aliases + https://clang.llvm.org/extra/clang-tidy/checks/performance/noexcept-swap.html[performance-noexcept-swap] +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/non-private-member-variables-in-classes.html[cppcoreguidelines-non-private-member-variables-in-classes] + aliases + https://clang.llvm.org/extra/clang-tidy/checks/misc/non-private-member-variables-in-classes.html[misc-non-private-member-variables-in-classes] +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/use-default-member-init.html[cppcoreguidelines-use-default-member-init] + aliases + https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html[modernize-use-default-member-init] + +The following checks are disabled for specific reasons: + +* https://clang.llvm.org/extra/clang-tidy/checks/bugprone/easily-swappable-parameters.html[bugprone-easily-swappable-parameters] - + may be enabled someday, but currently too onerous. +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables.html[cppcoreguidelines-avoid-non-const-global-variables] - + the nature of embedded work makes this check ill-conceived. +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/missing-std-forward.html[cppcoreguidelines-missing-std-forward] - + this check misdiagnoses some common things. +* https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-bounds-pointer-arithmetic.html[cppcoreguidelines-pro-bounds-pointer-arithmetic] - + may be enabled someday, but currently too onerous. +* https://clang.llvm.org/extra/clang-tidy/checks/misc/include-cleaner.html[misc-include-cleaner] - + warns on omnibus headers. +* https://clang.llvm.org/extra/clang-tidy/checks/misc/non-private-member-variables-in-classes.html[misc-non-private-member-variables-in-classes] - + public variables don't contribute to class invariants. +* https://clang.llvm.org/extra/clang-tidy/checks/modernize/concat-nested-namespaces.html[modernize-concat-nested-namespaces] - + it's a style choice. +* https://clang.llvm.org/extra/clang-tidy/checks/readability/identifier-length.html[readability-identifier-length] - + generic code uses lots of short identifiers. +* https://clang.llvm.org/extra/clang-tidy/checks/readability/identifier-naming.html[readability-identifier-naming] - + one of the most expensive checks; not worth the cost. +* https://clang.llvm.org/extra/clang-tidy/checks/readability/magic-numbers.html[readability-magic-numbers] - + the nature of embedded work makes this too onerous. +* https://clang.llvm.org/extra/clang-tidy/checks/readability/named-parameter.html[readability-named-parameter] - + it's a style choice. +* https://clang.llvm.org/extra/clang-tidy/checks/readability/qualified-auto.html[readability-qualified-auto] - + it's a style choice. +* https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-inline-specifier.html[readability-redundant-inline-specifier] - + `inline` is mostly, but not _only_ for the linker. +* https://clang.llvm.org/extra/clang-tidy/checks/readability/uppercase-literal-suffix.html[readability-uppercase-literal-suffix] - + it's a style choice. + +It is likely in the future that more clang-tidy checks will be enabled. + === `mypy` Python linting is available using https://mypy-lang.org/[`mypy`]. To lint python From 5afeef488332635f23a00ccbb3c3379166237213 Mon Sep 17 00:00:00 2001 From: Ben Deane Date: Tue, 22 Oct 2024 10:12:57 -0600 Subject: [PATCH 3/3] :construction_worker: Add clang-tidy canary Problem: - `clang-tidy` exits with success if the `.clang-tidy` file is ill-formed. Solution: - Add a clang-tidy-canary target which tests if `clang-tidy` has any error output when run with `--verify-config`. --- cmake/clang-tidy.cmake | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/cmake/clang-tidy.cmake b/cmake/clang-tidy.cmake index 311a937..539fd83 100644 --- a/cmake/clang-tidy.cmake +++ b/cmake/clang-tidy.cmake @@ -128,3 +128,16 @@ function(clang_tidy_interface) compute_branch_diff(clang-tidy ".hpp") endfunction() + +if(NOT TARGET clang-tidy-canary) + message(STATUS "Adding clang-tidy-canary target for ${CMAKE_SOURCE_DIR}") + add_custom_command( + OUTPUT clang_tidy_canary.alive + COMMAND ${CLANG_TIDY_PROGRAM} "--verify-config" 2>clang_tidy.log + COMMAND "!" "[" "-s" "clang_tidy.log" "]" + COMMAND ${CMAKE_COMMAND} "-E" "touch" "clang_tidy_canary.alive" + DEPENDS ${CMAKE_SOURCE_DIR}/.clang-tidy) + add_custom_target(clang-tidy-canary DEPENDS clang_tidy_canary.alive) + add_dependencies(clang-tidy clang-tidy-canary) + add_dependencies(clang-tidy-branch-diff clang-tidy-canary) +endif()