Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Complete the Introduction of Clang-Tidy #5520

Open
3 tasks done
mojomex opened this issue Dec 2, 2024 · 1 comment
Open
3 tasks done

Complete the Introduction of Clang-Tidy #5520

mojomex opened this issue Dec 2, 2024 · 1 comment
Assignees
Labels
status:help-wanted Assistance or contributors needed.

Comments

@mojomex
Copy link
Contributor

mojomex commented Dec 2, 2024

Checklist

  • I've read the contribution guidelines.
  • I've searched other issues and no duplicate issues were found.
  • I've agreed with the maintainers that I can plan this task.

Description

Enable as many Clang-Tidy checks as reasonable and applicable to the required Clang-Tidy CI check discussed here:

Reasonable, as in, there is a clear benefit to enabling the check, and the benefit outweighs he cost of fixing the offending code.

Applicable, as in, the check is compatible with our goal of highly performant automotive code. Things like replacing reinterpret_cast by explicit but slow memcpy could fall under 'non applicable' for performance reasons, but should definitely be discussed. Checks for frameworks we are not using should of course be disabled.

Please discuss about checks here:

Purpose

Static analysis is a standard practice in industry and helps to enforce clean coding, reduces succeptability to hard-to-catch bugs, and highlights many of the foot-guns C++ has to offer.

Possible approaches

As there are quite many Clang-Tidy checks, which up until now have not been enforced, it is not feasible to enable them all at once for Autoware. Instead, working in a distributed fashion and first fixing, then enforcing checks seems like the most efficient way forward.

The workflow per check could be along these lines:

  1. triage - is the check applicable, reasonable and important?
  2. assignment - an assignee or group of assignees is chosen to work on the check
  3. fix errors - create PRs against offending repos, get them merged
  4. make required - once the check passes on Autoware, make it required in CI
  5. report - Update the list below sothat other collaborators know the status of the rule

Definition of done

The transition is done when all of the below Clang-Tidy checks have either

  • been made required, or
  • have been discussed and disabled with a valid reason.

Accepted Checks (In Progress or Done)

Check Assignee Link to merged PR when done
bugprone-dangling-handle @veqcc #5465
bugprone-argument-comment @veqcc #5543
bugprone-assert-side-effect @veqcc #5543
bugprone-bad-signal-to-kill-thread @veqcc #5543
bugprone-bool-pointer-implicit-conversion @veqcc #5543
bugprone-copy-constructor-init @veqcc #5543
bugprone-dangling-handle @veqcc #5543
bugprone-dynamic-static-initializers @veqcc #5543
bugprone-fold-init-type @veqcc #5543
bugprone-forward-declaration-namespace @veqcc #5543
bugprone-forwarding-reference-overload @veqcc #5543
bugprone-inaccurate-erase @veqcc #5543
bugprone-incorrect-roundings @veqcc #5543
bugprone-lambda-function-name @veqcc #5543
bugprone-macro-repeated-side-effects @veqcc #5543
bugprone-misplaced-operator-in-strlen-in-alloc @veqcc #5543
bugprone-misplaced-pointer-arithmetic-in-alloc @veqcc #5543
bugprone-misplaced-widening-cast @veqcc #5543
bugprone-move-forwarding-reference @veqcc #5543
bugprone-multiple-statement-macro @veqcc #5543
bugprone-no-escape @veqcc #5543
bugprone-not-null-terminated-result @veqcc #5543
bugprone-posix-return @veqcc #5543
bugprone-redundant-branch-condition @veqcc #5543
bugprone-signal-handler @veqcc #5543
bugprone-sizeof-container @veqcc #5543
bugprone-sizeof-expression @veqcc #5543
bugprone-spuriously-wake-up-functions @veqcc #5543
bugprone-string-constructor @veqcc #5543
bugprone-string-integer-assignment @veqcc #5543
bugprone-string-literal-with-embedded-nul @veqcc #5543
bugprone-stringview-nullptr @veqcc #5543
bugprone-suspicious-enum-usage @veqcc #5543
bugprone-suspicious-include @veqcc #5543
bugprone-suspicious-memory-comparison @veqcc #5543
bugprone-suspicious-memset-usage @veqcc #5543
bugprone-suspicious-missing-comma @veqcc #5543
bugprone-suspicious-semicolon @veqcc #5543
bugprone-suspicious-string-compare @veqcc #5543
bugprone-swapped-arguments @veqcc #5543
bugprone-terminating-continue @veqcc #5543
bugprone-throw-keyword-missing @veqcc #5543
bugprone-too-small-loop-variable @veqcc #5543
bugprone-undefined-memory-manipulation @veqcc #5543
bugprone-undelegated-constructor @veqcc #5543
bugprone-unhandled-exception-at-new @veqcc #5543
bugprone-unhandled-self-assignment @veqcc #5543
bugprone-unused-raii @veqcc #5543
bugprone-unused-return-value @veqcc #5543
bugprone-use-after-move @veqcc #5543
bugprone-virtual-near-miss @veqcc #5543

Disabled Checks

Check Discussion thread
modernize-pass-by-value here

Backlog

⚠️ The below list is currently ordered alphabetically, not by priority. Will fix in the near future.

Check
boost-use-to-string
bugprone-branch-clone
bugprone-easily-swappable-parameters
bugprone-exception-escape
bugprone-implicit-widening-of-multiplication-result
bugprone-infinite-loop
bugprone-integer-division
bugprone-macro-parentheses
bugprone-parent-virtual-call
bugprone-reserved-identifier
bugprone-signed-char-misuse
cert-dcl21-cpp
cert-dcl50-cpp
cert-dcl58-cpp
cert-env33-c
cert-err33-c
cert-err34-c
cert-err52-cpp
cert-err58-cpp
cert-err60-cpp
cert-flp30-c
cert-mem57-cpp
cert-msc50-cpp
cert-msc51-cpp
cert-oop57-cpp
cert-oop58-cpp
concurrency-mt-unsafe
concurrency-thread-canceltype-asynchronous
cppcoreguidelines-avoid-goto
cppcoreguidelines-avoid-non-const-global-variables
cppcoreguidelines-init-variables
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-macro-usage
cppcoreguidelines-narrowing-conversions
cppcoreguidelines-no-malloc
cppcoreguidelines-owning-memory
cppcoreguidelines-prefer-member-initializer
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
cppcoreguidelines-pro-type-const-cast
cppcoreguidelines-pro-type-cstyle-cast
cppcoreguidelines-pro-type-member-init
cppcoreguidelines-pro-type-reinterpret-cast
cppcoreguidelines-pro-type-static-cast-downcast
cppcoreguidelines-pro-type-union-access
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
cppcoreguidelines-virtual-class-destructor
google-build-explicit-make-pair
google-build-namespaces
google-build-using-namespace
google-default-arguments
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-nsobject-new
google-objc-avoid-throwing-exception
google-objc-function-naming
google-objc-global-variable-declaration
google-readability-avoid-underscore-in-googletest-name
google-readability-casting
google-readability-todo
google-runtime-int
google-runtime-operator
google-upgrade-googletest-case
hicpp-avoid-goto
hicpp-exception-baseclass
hicpp-multiway-paths-covered
hicpp-no-assembler
hicpp-signed-bitwise
misc-definitions-in-headers
misc-misleading-bidirectional
misc-misleading-identifier
misc-misplaced-const
misc-new-delete-overloads
misc-no-recursion
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
misc-redundant-expression
misc-static-assert
misc-throw-by-value-catch-by-reference
misc-unconventional-assign-operator
misc-uniqueptr-reset-release
misc-unused-alias-decls
misc-unused-parameters
misc-unused-using-decls
modernize-avoid-bind
modernize-avoid-c-arrays
modernize-concat-nested-namespaces
modernize-deprecated-headers
modernize-deprecated-ios-base-aliases
modernize-loop-convert
modernize-make-shared
modernize-make-unique
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
modernize-replace-disallow-copy-and-assign-macro
modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
modernize-unary-static-assert
modernize-use-auto
modernize-use-bool-literals
modernize-use-default-member-init
modernize-use-emplace
modernize-use-equals-default
modernize-use-equals-delete
modernize-use-nodiscard
modernize-use-noexcept
modernize-use-nullptr
modernize-use-override
modernize-use-trailing-return-type
modernize-use-transparent-functors
modernize-use-uncaught-exceptions
modernize-use-using
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
performance-for-range-copy
performance-implicit-conversion-in-loop
performance-inefficient-algorithm
performance-inefficient-string-concatenation
performance-inefficient-vector-operation
performance-move-const-arg
performance-move-constructor-init
performance-no-automatic-move
performance-no-int-to-ptr
performance-noexcept-move-constructor
performance-trivially-destructible
performance-type-promotion-in-math-fn
performance-unnecessary-copy-initialization
performance-unnecessary-value-param
portability-restrict-system-includes
portability-simd-intrinsics
readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-const-return-type
readability-container-contains
readability-container-data-pointer
readability-container-size-empty
readability-convert-member-functions-to-static
readability-delete-null-pointer
readability-duplicate-include
readability-else-after-return
readability-function-cognitive-complexity
readability-function-size
readability-identifier-length
readability-identifier-naming
readability-implicit-bool-conversion
readability-inconsistent-declaration-parameter-name
readability-isolate-declaration
readability-magic-numbers
readability-make-member-function-const
readability-misleading-indentation
readability-misplaced-array-index
readability-named-parameter
readability-non-const-parameter
readability-qualified-auto
readability-redundant-access-specifiers
readability-redundant-control-flow
readability-redundant-declaration
readability-redundant-function-ptr-dereference
readability-redundant-member-init
readability-redundant-preprocessor
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-redundant-string-init
readability-simplify-boolean-expr
readability-simplify-subscript-expr
readability-static-accessed-through-instance
readability-static-definition-in-anonymous-namespace
readability-string-compare
readability-suspicious-call-argument
readability-uniqueptr-delete-release
readability-uppercase-literal-suffix
readability-use-anyofallof
@mojomex mojomex added the status:help-wanted Assistance or contributors needed. label Dec 2, 2024
@veqcc
Copy link
Contributor

veqcc commented Dec 9, 2024

@mojomex
I have made a PR to add required checks of clang-tidy in terms of bugprone prefix. 👀
#5543

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:help-wanted Assistance or contributors needed.
Projects
Status: Todo
Development

No branches or pull requests

3 participants