The goal of this little project is to allow developers to leverage Clang-Tidy to help spotting typical bugprone code and typical readability problems. The plan of attack is simple: a developer picks up a warning from the list, modifies the code base to address it, commits the work to the master Git branch.
Required setup:
- Linux (unfortunately, it is not trivial to have Clang-Tidy outside of Linux)
- Clang-Tidy installed (use package manager of your Linux distro)
- Configure Blender's CMake using WITH_CLANG_TIDY=ON (optionally specify CLANG_TIDY_EXECUTABLE if automatic detection fails).
Both Clang and GCC toolchains are supposed to work. However, GCC+Clang-Tidy might need some polishing.
There is a command which works for me (tm): make developer debug BUILD_CMAKE_ARGS='-DWITH_CLANG_TIDY=ON'
Clang-Tidy is run as an extra compilation step, so it's warning are emitted as regular build process.
Workflow
- Put your name next to the warning once you start working on it.
- Remove corresponding line from the .clang-tidy file, make Blender compiled without this warning.
- Commit result of your work (if the warning is fully tackled, commit the change to .clang-tidy as well).
Some warnings happens in many areas, and many people can work on it. Use blender-chat to coordinate this.
List of warnings to be addressed
Documentation can be found in the Clang-Tidy v10 docs.
- -readability-else-after-return
- -readability-braces-around-statements
- -readability-inconsistent-declaration-parameter-name
- -readability-non-const-parameter
- -readability-redundant-preprocessor
- -readability-redundant-control-flow
- -readability-named-parameter
- -readability-function-size (rule is enabled, but future parameter tweaks will likely reveal more areas of improvement)
- -readability-static-definition-in-anonymous-namespace
- -readability-delete-null-pointer
- -readability-redundant-string-init
- -readability-redundant-member-init
- -readability-const-return-type
- -readability-container-size-empty
- -readability-redundant-string-cstr
- -readability-static-accessed-through-instance
- -readability-redundant-declaration
- -readability-qualified-auto
- -readability-function-cognitive-complexity
- -readability-suspicious-call-argument
- -bugprone-incorrect-roundings
- -bugprone-suspicious-string-compare
- -bugprone-too-small-loop-variable
- -bugprone-misplaced-widening-cast
- -bugprone-not-null-terminated-result
- -bugprone-suspicious-missing-comma
- -bugprone-argument-comment
- -bugprone-assert-side-effect
- -bugprone-parent-virtual-call
- -bugprone-infinite-loop
- -bugprone-copy-constructor-init
- -bugprone-lambda-function-name
- -bugprone-exception-escape (does not happen on Linux + clang-tidy-10) (Fixed with D9497)
- -bugprone-redundant-branch-condition
- -bugprone-easily-swappable-parameters
- -bugprone-implicit-widening-of-multiplication-result
- -modernize-deprecated-headers
- -modernize-use-equals-default D9494
- -modernize-use-nullptr
- -modernize-concat-nested-namespaces
- -modernize-use-emplace
- -modernize-use-using
- -modernize-redundant-void-arg
- -modernize-use-bool-literals
- -modernize-loop-convert (partially done in rB11c4066159e1)
- -modernize-pass-by-value (P1755)
- -modernize-make-unique
- -modernize-use-override
- -modernize-use-transparent-functors D10323
- -modernize-raw-string-literal D10322
- -modernize-avoid-bind D10320
- -modernize-use-default-member-init D9501
- -modernize-return-braced-init-list
There is -readability-misleading-indentation explicitly silenced. Seems to be incompatible with Blender's clang-format rules.
Ignored for now:
- -bugprone-sizeof-expression: It does not allow us to do the following: T **ptr_array = MEM_mallocN(sizeof(*ptr_array), AT). Also see this.
- -bugprone-integer-division: It causes too many false-positives and fixing adds more noise.
Papercuts
-] Modifying .clang-tidy does not force re-compilation. For now run the clean target manually to make sure new warnings are enabled. This is something what was annoying initially, but is not that different from .clang-format. Lets not overthink/overcomplicate build process here and just leave it as is.