Page MenuHome

Code quality day: Clang-Tidy
Confirmed, NormalPublic

Description

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

  1. Put your name next to the warning once you start working on it.
  2. Remove corresponding line from the .clang-tidy file, make Blender compiled without this warning.
  3. 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

NOTE: This list is assembled based on thoughts "would be really nice to address those". Use common sense when it seems that addressing them goes too far. It might be best to ignore the warning rather than heavily modify codebase at this point.
NOTE: Some of the cleanup might discover actual bug in the code. Please separate fixing the bug from the cleanup commit.
  • -readability-else-after-return
  • -readability-braces-around-statements
  • -readability-inconsistent-declaration-parameter-name (@Sebastian Parborg (zeddb) working on it)
  • -readability-non-const-parameter (@Jacques Lucke (JacquesLucke))
  • -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 (@Sergey Sharybin (sergey) working on this)
  • -readability-const-return-type
  • -readability-container-size-empty
  • -readability-redundant-string-cstr
  • -readability-static-accessed-through-instance
  • -readability-redundant-declaration
  • -readability-qualified-auto
  • -bugprone-integer-division
  • -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

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.

Papercuts

  • Modifying .clang-tidy does not force re-compilation. For now run the clean target manually to make sure new warnings are enabled.

Event Timeline

Sergey Sharybin (sergey) changed the task status from Needs Triage to Confirmed.Jul 2 2020, 3:38 PM
Sergey Sharybin (sergey) created this task.