Use a clang-tidy configuration file

[IAmNotHanni]

Discussion: #484

This uses a .clang-tidy configuration file to specify which warnings to ignore and which files to analyze.
Please check the result of the CI log before merging this, I can't test it in my fork.

[adam-sawicki-a]

Works fine.

I would also like to disable:

invalid case style for variable 'VMA_SUBALLOCATION_TYPE_NAMES' [readability-identifier-naming]

warning: parameter name 'v' is too short, expected at least 3 characters [readability-identifier-length]

[adam-sawicki-a]

I can see file /home/runner/work/VulkanMemoryAllocator/VulkanMemoryAllocator/build/CMakeFiles/3.31.6/CompilerIdCXX/CMakeCXXCompilerId.cpp is still analyzed.

[IAmNotHanni]

I can see file /home/runner/work/VulkanMemoryAllocator/VulkanMemoryAllocator/build/CMakeFiles/3.31.6/CompilerIdCXX/CMakeCXXCompilerId.cpp is still analyzed.

In b5a5703 I changed it so it ignores the build directory. It should work now.

[IAmNotHanni]

It works, but I get the impression that it does not analyze the files of the sample app, only vk_mem_alloc.h.

[IAmNotHanni]

(I will have time to fix this sunday evening, need to go)

[adam-sawicki-a]

Now we have a lot of warnings coming from analysis inside of Vulkan SDK, like "/home/runner/work/VulkanMemoryAllocator/VulkanMemoryAllocator/vulkan_sdk/1.4.309.0/x86_64/include/slang/slang-com-ptr.h" 😟

[adam-sawicki-a]

I can't test it in my fork.

Why can't you test GitHub Actions on your fork? It seems to work for me. I created a fork on my GitHub account:
https://github.com/adam-sawicki-a/VulkanMemoryAllocator/tree/master
And also on my another (personal) GitHub account that has no write access to the original VMA repo:
https://github.com/sawickiap/VulkanMemoryAllocator
I can see all actions executed on both of them.

[IAmNotHanni]

Now we have a lot of warnings coming from analysis inside of Vulkan SDK, like "/home/runner/work/VulkanMemoryAllocator/VulkanMemoryAllocator/vulkan_sdk/1.4.309.0/x86_64/include/slang/slang-com-ptr.h" 😟

Interesting 😄

I can't test it in my fork.

Why can't you test GitHub Actions on your fork? It seems to work for me. I created a fork on my GitHub account: https://github.com/adam-sawicki-a/VulkanMemoryAllocator/tree/master And also on my another (personal) GitHub account that has no write access to the original VMA repo: https://github.com/sawickiap/VulkanMemoryAllocator I can see all actions executed on both of them.

I honestly don't know. I will try to get it to work first before pushing further commits.

[IAmNotHanni]

I can't test it in my fork.

Why can't you test GitHub Actions on your fork? It seems to work for me. I created a fork on my GitHub account: https://github.com/adam-sawicki-a/VulkanMemoryAllocator/tree/master And also on my another (personal) GitHub account that has no write access to the original VMA repo: https://github.com/sawickiap/VulkanMemoryAllocator I can see all actions executed on both of them.

What you see in your fork there is the CI from the last merge. Since this account of you is not owner of VMA repository, it is not allowed to run the CI I think. If you would make a commit into the fork, I think GitHub won't permit you to run the CI in that fork. Since it's even with master branch, it just shows the last CI run from VMA repo's master branch.

[sawickiap]

I still think it works. On my personal GitHub account I created a branch of my fork:
https://github.com/sawickiap/VulkanMemoryAllocator/tree/CI-test
I introduced a bug:
sawickiap@98c26cb
And I can see the CI build failed:
https://github.com/sawickiap/VulkanMemoryAllocator/actions/runs/14497422259/job/40668587636

[IAmNotHanni]

Interesting. I will try to get it to work for my fork.

[IAmNotHanni]

Apologies for the force-push, but I think I have solved it now.

Changes:

• clang-tidy only runs on the project files, not third-party libraries
• The workflow prints the files which will be passed to clang-tidy
• The workflow will group the warnings by type and numer of occurrences, making it easy to add to ignore list.
• Use a cache for the Vulkan SDK, so we don't have to download it every time. This fix should be applied to the build workflows as well. Sadly, there seems to be no such easy fix for the apt packages.
• The current ignore list needs further work. You can decide what to add to the ignore list.

So please tell me which ones to add to the ignore list:

170 readability-braces-around-statements
120 misc-const-correctness
113 misc-static-assert
 91 readability-implicit-bool-conversion
 41 cppcoreguidelines-pro-type-cstyle-cast
 22 cppcoreguidelines-owning-memory
 17 readability-static-accessed-through-instance
 17 bugprone-multi-level-implicit-pointer-conversion
 17 bugprone-easily-swappable-parameters
 16 misc-unused-parameters
 12 performance-enum-size
 12 misc-misplaced-const
 12 misc-include-cleaner
 12 cppcoreguidelines-special-member-functions
 11 misc-non-private-member-variables-in-classes
 11 cppcoreguidelines-pro-type-member-init
 11 cppcoreguidelines-pro-bounds-array-to-pointer-decay
  9 modernize-loop-convert
  8 readability-const-return-type
  7 cppcoreguidelines-pro-type-reinterpret-cast
  7 cppcoreguidelines-pro-type-const-cast
  6 cppcoreguidelines-init-variables
  5 readability-simplify-boolean-expr
  5 readability-duplicate-include
  5 bugprone-macro-parentheses
  4 performance-no-int-to-ptr
  4 bugprone-sizeof-expression
  3 misc-no-recursion
  3 cppcoreguidelines-pro-type-vararg
  2 readability-suspicious-call-argument
  2 readability-qualified-auto
  2 readability-else-after-return
  1 readability-convert-member-functions-to-static
  1 modernize-use-nullptr
  1 cppcoreguidelines-no-malloc
  1 cppcoreguidelines-avoid-const-or-ref-data-members
  1 bugprone-switch-missing-default-case

[adam-sawicki-a]

Thank you for getting back to this topic.

170 readability-braces-around-statements    - PLEASE ADD TO IGNORE
120 misc-const-correctness                  - PLEASE ADD TO IGNORE
113 misc-static-assert                      - PLEASE ADD TO IGNORE
 91 readability-implicit-bool-conversion    - PLEASE ADD TO IGNORE
 41 cppcoreguidelines-pro-type-cstyle-cast  - we can leave it as warnings
 22 cppcoreguidelines-owning-memory         - PLEASE ADD TO IGNORE
 17 readability-static-accessed-through-instance - PLEASE ADD TO IGNORE
 17 bugprone-multi-level-implicit-pointer-conversion - we can leave it as warnings
 17 bugprone-easily-swappable-parameters    - PLEASE ADD TO IGNORE
 16 misc-unused-parameters                  - PLEASE ADD TO IGNORE
 12 performance-enum-size                   - PLEASE ADD TO IGNORE
 12 misc-misplaced-const                    - we can leave it as warnings
 12 misc-include-cleaner                    - we can leave it as warnings
 12 cppcoreguidelines-special-member-functions - we can leave it as warnings
 11 misc-non-private-member-variables-in-classes - we can leave it as warnings
 11 cppcoreguidelines-pro-type-member-init  - we can leave it as warnings
 11 cppcoreguidelines-pro-bounds-array-to-pointer-decay - PLEASE ADD TO IGNORE
  9 modernize-loop-convert                  - we can leave it as warnings
  8 readability-const-return-type           - we can leave it as warnings
  7 cppcoreguidelines-pro-type-reinterpret-cast - we can leave it as warnings
  7 cppcoreguidelines-pro-type-const-cast   - we can leave it as warnings
  6 cppcoreguidelines-init-variables        - we can leave it as warnings (although it's a buggy clang-tidy - those variables are actually unconditionally initialized)
  5 readability-simplify-boolean-expr       - PLEASE ADD TO IGNORE
  5 readability-duplicate-include           - we can leave it as warnings
  5 bugprone-macro-parentheses              - we can leave it as warnings
  4 performance-no-int-to-ptr               - we can leave it as warnings
  4 bugprone-sizeof-expression              - PLEASE ADD TO IGNORE (how else should we take sizeof of a pointer??)
  3 misc-no-recursion                       - PLEASE ADD TO IGNORE (is any recursion forbidden??)
  3 cppcoreguidelines-pro-type-vararg       - PLEASE ADD TO IGNORE
  2 readability-suspicious-call-argument    - we can leave it as warnings
  2 readability-qualified-auto              - we can leave it as warnings
  2 readability-else-after-return           - I will fix it
  1 readability-convert-member-functions-to-static - I will fix it
  1 modernize-use-nullptr                   - I will fix it
  1 cppcoreguidelines-no-malloc             - PLEASE ADD TO IGNORE
  1 cppcoreguidelines-avoid-const-or-ref-data-members - we can leave it as warnings
  1 bugprone-switch-missing-default-case    - I will fix it

[IAmNotHanni]

I added some more checks, and this is the list now

    161 altera-struct-pack-align
    113 google-readability-casting
    112 altera-id-dependent-backward-branch
     70 hicpp-signed-bitwise
     41 cppcoreguidelines-pro-type-cstyle-cast
     18 hicpp-use-auto
     17 bugprone-multi-level-implicit-pointer-conversion
     15 llvm-header-guard
     12 misc-misplaced-const
     12 misc-include-cleaner
     11 misc-non-private-member-variables-in-classes
     11 hicpp-no-array-decay
      9 modernize-loop-convert
      8 readability-const-return-type
      6 cppcoreguidelines-init-variables
      5 readability-duplicate-include
      5 bugprone-macro-parentheses
      4 performance-no-int-to-ptr
      3 llvm-include-order
      3 cert-err33-c
      2 readability-suspicious-call-argument
      1 readability-convert-member-functions-to-static
      1 hicpp-no-malloc
      1 fuchsia-statically-constructed-objects
      1 cppcoreguidelines-avoid-const-or-ref-data-members
      1 cert-err58-cpp
      1 bugprone-switch-missing-default-case

[IAmNotHanni]

I think most of these are not helpful. I will just go back to using the list before and add your checks. I thouht we were missing some important checks by not having the * in the list, but those things seem not really important.

[adam-sawicki-a]

Out of those new ones, I think cert-err33-c makes sense.

[IAmNotHanni]

I added those CERT C Secure Coding Standard Checks to clang-tidy.

[adam-sawicki-a]

Thank you very much for this contribution!