Static analyser clang-tidy: which checks should be used?

Beman Libraries
1

clang-tidy is really helpful and we should use it.
It is available on every OS and may be use in debug builds.

Use clang-tidy in Debug builds if available

see i.e. Too many warnings with clang-tidy · Issue #37 · bemanproject/inplace_vector · GitHub

Also the naming conventions may be checked / fixed if wanted with readability-identifier-* checks.

Checks:
  '-*,
      boost-*,
      bugprone-*,
      -bugprone-empty-catch,
      cert-*,
      cert-dcl58-cpp,
      clang-analyzer-*,
      concurrency-*,
      -cppcoreguidelines-*,
      -google-*,
      hicpp-*,
      -hicpp-avoid-c-arrays,
      misc-*,
      misc-const-correctness,
      -misc-include-cleaner,
      -misc-use-internal-linkage,
      -modernize-*,
      -modernize-use-nodiscard,
      performance-*,
      portability-*,
      -readability-*,
      -readability-identifier-*,
      -readability-implicit-bool-conversion,
      -readability-magic-numbers,
      -*-named-parameter,
      -*-uppercase-literal-suffix,
      -*-use-equals-default,
      -*-braces-around-statements
  '
HeaderFilterRegex: '.*/inplace_vector/(include|src|example|tests)/.*\.(hpp)$'
WarningsAsErrors: 'clang*'
FormatStyle: file

CheckOptions:
  - { key: readability-identifier-naming.NamespaceCase,             value: CamelCase  }
  - { key: readability-identifier-naming.ClassCase,                 value: CamelCase  }
  - { key: readability-identifier-naming.EnumCase,                  value: CamelCase  }
  - { key: readability-identifier-naming.MemberCase,                value: camelBack  }
  - { key: readability-identifier-naming.MemberPrefix,              value: m_         }
  - { key: readability-identifier-naming.StructCase,                value: lower_case }
  - { key: readability-identifier-naming.UnionCase,                 value: lower_case }
  - { key: readability-identifier-naming.TypedefCase,               value: lower_case }
  - { key: readability-identifier-naming.TypedefSuffix,             value: _type      }
  - { key: readability-identifier-naming.FunctionCase,              value: camelBack  }
  - { key: readability-identifier-naming.VariableCase,              value: camelBack  }
  - { key: readability-identifier-naming.ParameterCase,             value: camelBack  }
  - { key: readability-identifier-naming.LocalVariableCase,         value: camelBack  }
  - { key: readability-identifier-naming.ConstexprFunctionCase,     value: camelBack  }
  - { key: readability-identifier-naming.ConstexprMethodCase,       value: camelBack  }
  - { key: readability-identifier-naming.ConstexprVariableCase,     value: UPPER_CASE }
  - { key: readability-identifier-naming.ClassConstantCase,         value: UPPER_CASE }
  - { key: readability-identifier-naming.EnumConstantCase,          value: UPPER_CASE }
  - { key: readability-identifier-naming.GlobalConstantCase,        value: UPPER_CASE }
  - { key: readability-identifier-naming.GlobalConstantPointerCase, value: UPPER_CASE }
  - { key: readability-identifier-naming.LocalConstantPointerCase,  value: UPPER_CASE }
  - { key: readability-identifier-naming.ScopedEnumConstantCase,    value: UPPER_CASE }
  - { key: readability-identifier-naming.StaticConstantCase,        value: UPPER_CASE }

All possible checks and fixes

2

I am working on introducing clang-tidy to exemplar.
And this is the set of checks I currently think could be in scope.

  • bugprone-*
  • clang-analyzer-core*,
  • clang-analyzer-cplusplus*
  • concurrency-*
  • cppcoreguidelines-*
  • modernize-*
  • performance-*
  • portability-*
  • readability-*

Though I do want to bring up that clang-tidy is very pedantic, with sometimes questionable suggestions that is good out of principle but annoying (e.g. magic number, use trailing return type).

I honestly don’t think we should enforce clang-tidy use unless the project is being built from the ground up, I don’t think there’s that much value in

3

Please add misc-*, cert-*, and hicpp-* too.

Some cppcoreguidelines-* are annoying and others redundant with hiccp and bugprone!

4

Yeah that could happen.

I don’t see the value of clang-tidy that much.

From personal experience clang-tidy is more of a hustle than a savior.

5

I see it as an add-on to the compiler warnings and as a helper to refactor / modernise the code!

see i.e.: Modernise your source code using clang-tidy

1 Like
6

clausklein> see i.e.: Modernise your source code using clang-tidy

We need to be a little careful here – I’ve seen clang-tidy break working code – and really for no good reason. That said, my information from about 5 years ago – so much older version. I’d also note that in the context of Beman, all the code should already be modern – the project is basically 6 months old.

My sense here is that we should be pretty conservative with the rules we apply. Start with a small and build. It would be interesting to see a report of the various repos: optional, execution, iterator_interface, and utf_view to see what tidy complains about with various options. That would allow us to see for the projects what issues it’s flagging.

7

Sure, the run-clang-tidy -fix option must be use carefully!

Normally I fix only one check, but over all files in project.

Then compile, test, commit, maybe push.

Than next check…

8

A current live result may be found here: Quickhacks to check CXX_MODULE fmt without gtest · ClausKlein/fmt-module@ad6d2fa · GitHub

Most of warnings may be fixed, but with a CXX_MODULE I have a problem with clang-tidy at the moment :woozy_face: