Windows: Add clang-tidy support in Visual Studio
ClosedPublic
Actions

Authored by Ray Molenkamp (LazyDodo) on Aug 10 2020, 1:14 AM.

Details

Reviewers

Brecht Van Lommel (brecht)
Sergey Sharybin (sergey)

Summary

While clang-tidy support is broken for cmake/ninja (see D8503 for details) the built in support in the VS IDE actually works remarkably well.

This diff adds support for clang-tidy support inside the Visual Studio IDE. It currently does *NOT* run automatically when you build blender, you have to right click the project you want to check and run the analyser manually.

The reason for this is two fold:

By default we setup clang-tidy to fail on warning and there are *A LOT* of warnings.
It adds an unreasonably long time to a full build.

Requirements: VS 16.4+

Diff Detail

Repository

rB Blender

Branch

tmp_clang_tidy_ide_support (branched from master)

Build Status

Buildable 9431
Build 9431: arc lint + arc unit

Event Timeline

Ray Molenkamp (LazyDodo) requested review of this revision.Aug 10 2020, 1:14 AM

Ray Molenkamp (LazyDodo) created this revision.

Harbormaster completed remote builds in B9431: Diff 27553.Aug 10 2020, 1:14 AM

By default we setup clang-tidy to fail on warning and there are *A LOT* of warnings.

The current .clang-tidy configuration is meant to be fully passable on an entire source/. Are those warnings coming from ifdef-ed code which is only compiled on windows/msvc?

In D8512#207566, @Sergey Sharybin (sergey) wrote:

By default we setup clang-tidy to fail on warning and there are *A LOT* of warnings.

The current .clang-tidy configuration is meant to be fully passable on an entire source/. Are those warnings coming from ifdef-ed code which is only compiled on windows/msvc?

There's a bunch of them inside WIN32 ifdefs (ie source\blender\blenlib\intern\fnmatch.c, source\blender\blenlib\intern\winstuff_dir.c ) and probably a bunch more i have not looked at in detail, also it doesn't seem to be constrained to just source/ the .clang-tidy file is in the located in the root source folder so it is also throwing of issues from /intern ie

1>------ Build started: Project: bf_intern_sky, Configuration: Debug x64 ------
1>sky_model.cpp
K:\BlenderGit\blender\intern\sky\source\sky_model.cpp(181,27): error GA67C5309: statement should be inside braces [readability-braces-around-statements,-warnings-as-errors]
  if (int_turbidity == 10)
                          ^
                           {
Suppressed 1 warnings (1 with check filters).
5 warnings treated as errors
K:\BlenderGit\blender\intern\sky\source\sky_model.cpp(246,27): error GA67C5309: statement should be inside braces [readability-braces-around-statements,-warnings-as-errors]
  if (int_turbidity == 10)
                          ^
                           {
K:\BlenderGit\blender\intern\sky\source\sky_model.cpp(273,84): error G6427A3D2: pointer parameter 'configuration' can be pointer to const [readability-non-const-parameter,-warnings-as-errors]
static double ArHosekSkyModel_GetRadianceInternal(SKY_ArHosekSkyModelConfiguration configuration,
                                                                                   ^
                                                  const 
K:\BlenderGit\blender\intern\sky\source\sky_model.cpp(301,34): error GA67C5309: statement should be inside braces [readability-braces-around-statements,-warnings-as-errors]
  if (low_wl < 0 || low_wl >= 11)
                                 ^
                                  {
K:\BlenderGit\blender\intern\sky\source\sky_model.cpp(309,21): error GA67C5309: statement should be inside braces [readability-braces-around-statements,-warnings-as-errors]
  if (interp < 1e-6)
                    ^
                     {
1>Done building project "bf_intern_sky.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

none of them seem hard to fix ( statement should be inside braces seems to be the most common one) but there are quite a few of them.

*prod*

As long as Clang-Tidy is not enabled by default (aka does not break regular developers builds) think it's fine.

This revision is now accepted and ready to land.Dec 3 2020, 5:35 PM

committed in rBf637b4706423: MSVC: Enable clang-tidy analyser but forgot the magic words in the commit msg so it didn't auto close