celerity-runtime icon indicating copy to clipboard operation
celerity-runtime copied to clipboard

clang-tidy: Enable more checks

Open psalz opened this issue 2 years ago • 1 comments

So far we've used clang-tidy only to enforce naming conventions. This PR enables a whole set of additional checks as suggested in #127.

Notably this includes misc-const-correctness, a check that was recently added in clang-tidy 15. While we don't yet use clang-tidy 15 in CI, this patch includes a (semi-automatic) fixup pass that was done for all const-correctness violations found through that check. Unfortunately the check seems to be a little buggy still (producing false positives in certain situations) which is why I've decided to not turn it into an error for now.

Here's a summary of the remaining clang-tidy warnings we produce with the set of checks enabled in this patch:

Found 465 diagnostics (out of 2880 parsed; 2415 were duplicates).
   96: readability-qualified-auto
   51: readability-function-cognitive-complexity
   46: bugprone-unchecked-optional-access
   33: cppcoreguidelines-special-member-functions
   21: cppcoreguidelines-init-variables
   18: readability-implicit-bool-conversion
   16: bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions
   16: readability-named-parameter
   15: performance-unnecessary-value-param
   14: cppcoreguidelines-avoid-non-const-global-variables
   13: cppcoreguidelines-pro-bounds-constant-array-index
   12: clang-analyzer-deadcode.DeadStores
   12: cppcoreguidelines-pro-bounds-array-to-pointer-decay
   11: readability-isolate-declaration
   10: cppcoreguidelines-pro-type-member-init
    9: cppcoreguidelines-pro-type-static-cast-downcast
    8: bugprone-exception-escape
    6: cppcoreguidelines-pro-type-vararg
    6: readability-else-after-return
    6: cppcoreguidelines-non-private-member-variables-in-classes
    5: cppcoreguidelines-pro-type-reinterpret-cast
    4: readability-redundant-access-specifiers
    3: performance-faster-string-find
    3: cppcoreguidelines-pro-type-const-cast
    3: cppcoreguidelines-owning-memory
    3: clang-diagnostic-error
    2: bugprone-implicit-widening-of-multiplication-result
    2: bugprone-reserved-identifier
    2: readability-inconsistent-declaration-parameter-name
    2: misc-const-correctness
    2: readability-static-accessed-through-instance
    2: performance-noexcept-move-constructor
    2: clang-analyzer-optin.mpi.MPI-Checker
    2: cppcoreguidelines-prefer-member-initializer
    2: readability-convert-member-functions-to-static
    1: readability-redundant-smartptr-get
    1: readability-suspicious-call-argument
    1: clang-analyzer-core.NonNullParamChecker
    1: performance-move-const-arg
    1: performance-for-range-copy
    1: clang-analyzer-core.CallAndMessage
    1: readability-make-member-function-const

Remember that our CI setup only runs checks on diffs, so I think it is reasonable to address these whenever we touch those respective parts of the code.

Resolves #127.

psalz avatar Sep 27 '22 09:09 psalz

Turns out I was missing two commas in the list of checks...

psalz avatar Sep 27 '22 15:09 psalz