Fix clang tidy workflow

[openroad-ci]

Although the base image for our clang-tidy dependency (openroad/ubuntu22.04-dev:latest) is built using dependencyInstaller.sh and should have libyaml-cpp-dev installed, for some reason it wasn't.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[oharboe]

@hzeller Do you have any experience with setting up a bazelisk run //:clang-tidy target that uses bazel LLVM toolchain instead of system tools? Good idea? bad idea? :-)

[hzeller]

This is what I do do get clang-tidy from the bazel llvm-toolchain:

export CLANG_TIDY=$(bazel run -c opt --run_under="echo" @llvm_toolchain//:clang-tidy 2>/dev/null)

This might already be sufficient to extract and use the same clang-tidy version in the CI as the bazel llvm-toolchain sees.

Note, the clang-tidy run in the CI is different from what I use to do cleanups. I think the tool in the CI figures out the changed files and then runs ${CLANG_TIDY} on it, then filters by the lines that have a diff. This is perfect for the code review, but not super-useful for clean-up work as for clean-up work we need a file with all issues, easily grep/awk-able and ideally would also have an effective cache of what does not need to be re-processed (clang-tidy is slow).

For clean-up efforts and this output I run the script in etc/run-clang-tidy-cached.cc (coming from my dev-tools).

To run clang-tidy, we also need a compilation db, which right now can be provided by cmake, but if that is not used (or once that is gone), bant can be used to create a compilation-db from the BUILD rules. Bant observes the workspace to create the compilation-db so it still is necessary to either run a full bazel build or at least the parts that tickle generating all files.

So to put everything together: create compilation db once (I prefer the compile_flags as it is much shorter than the json version, but as good for our purposes):

bant compile-flags -o compile_flags.txt   # before that, build at least once; call again when on MODULE.bazel change.

And the following runs clang-tidy on everything using all cores and keeps content-addressed caches of things it already did, so next time it runs it only re-processes the changed files and files that depend on changed header files; also the cache is stored out-of-tree, so a clean in that repository does not destroy the cache (which is good, as clang-tidy is veeeery slow and we don't want to accidentally loose a day of CPU time. Running this the first time is definitely a go-get-coffee moment). It also can keep track of different branches without effort (because of the content-addressed nature of the cache).

# This only uses the clang-tidy already configured for the llvm-toolchain in bazel
CLANG_TIDY=$(bazel run -c opt --run_under="echo" @llvm_toolchain//:clang-tidy 2>/dev/null) etc/run-clang-tidy-cached.cc

The output is a script-friendly OpenROAD_clang-tidy.out and a useful human-readable OpenROAD_clang-tidy.summary. For example, currently that looks roughly like

Cache dir "/home/hzeller/.cache/clang-tidy/OpenROAD_v20_7f7b5a2f"
1610 files of interest.
55 files to process (w/ 32 jobs)... 
Details: OpenROAD_clang-tidy.out
Summary: OpenROAD_clang-tidy.summary
---- Summary ----
 5325 [misc-include-cleaner]
  797 [readability-math-missing-parentheses]
  784 [misc-use-internal-linkage]
  336 [modernize-macro-to-enum]
  319 [clang-analyzer-security.insecureAPI.strcpy]
  262 [clang-diagnostic-error]
  241 [bugprone-suspicious-string-compare]
  229 [readability-redundant-casting]
  157 [bugprone-multi-level-implicit-pointer-conversion]
  153 [performance-avoid-endl]
  139 [google-runtime-int]
  137 [readability-use-std-min-max]
  103 [readability-non-const-parameter]
  102 [modernize-use-emplace]
   90 [performance-unnecessary-value-param]
   84 [modernize-use-default-member-init]
   76 [clang-analyzer-deadcode.DeadStores]
   73 [modernize-loop-convert]
   68 [modernize-type-traits]
   62 [readability-avoid-const-params-in-decls]
   62 [readability-redundant-declaration]
   59 [bugprone-assignment-in-if-condition]
   55 [bugprone-switch-missing-default-case]
   49 [modernize-use-equals-default]
   44 [google-build-using-namespace]
   38 [bugprone-implicit-widening-of-multiplication-result]
   33 [readability-redundant-control-flow]
   30 [modernize-deprecated-headers]
   27 [bugprone-reserved-identifier]
   27 [clang-diagnostic-unused-const-variable]
   21 [google-default-arguments]
   20 [modernize-min-max-use-initializer-list]
   19 [misc-static-assert]
   19 [readability-suspicious-call-argument]
   15 [misc-unused-using-decls]
   15 [performance-move-const-arg]
   13 [performance-unnecessary-copy-initialization]
   12 [performance-type-promotion-in-math-fn]
   11 [readability-container-size-empty]
    8 [clang-analyzer-core.UndefinedBinaryOperatorResult]
    7 [misc-header-include-cycle]
    6 [bugprone-empty-catch]
    6 [clang-analyzer-unix.Malloc]
    6 [readability-redundant-inline-specifier]
    5 [bugprone-pointer-arithmetic-on-polymorphic-object]
    5 [clang-analyzer-core.uninitialized.Assign]
    5 [readability-duplicate-include]
    4 [bugprone-unchecked-optional-access]
    3 [bugprone-argument-comment]
    3 [bugprone-copy-constructor-init]
    3 [bugprone-infinite-loop]
    3 [bugprone-unused-return-value]
    3 [clang-analyzer-core.NullDereference]
    3 [clang-analyzer-core.uninitialized.Branch]
    3 [misc-unused-alias-decls]
    3 [readability-static-accessed-through-instance]
    2 [bugprone-integer-division]
    2 [misc-confusable-identifiers]
    2 [readability-container-data-pointer]
    1 [bugprone-chained-comparison]
    1 [bugprone-return-const-ref-from-parameter]
    1 [bugprone-unused-local-non-trivial-variable]
    1 [clang-analyzer-optin.cplusplus.UninitializedObject]
    1 [clang-analyzer-unix.Stream]
    1 [modernize-use-using]
    1 [readability-avoid-return-with-void-value]
    1 [readability-avoid-unconditional-preprocessor-if]
    1 [readability-else-after-return]
    1 [readability-enum-initial-value]
    1 [readability-misplaced-array-index]

This is all I use to prepare all my clean-up pull requests (using the output in OpenROAD_clang-tidy.out and my various scripts and techniques.

There could probably ways to run clang-tidy directly from bazel, but never tried that and whatever it has to do would be an ugly knowing-the-internals-of-bazel-actions hack, and we're spcifically tying us into an undocumented part of a somewhat moving target of a tool. External scripts are much nicer to do that.

[oharboe]

@hzeller Sweet! :-)

Inspired by the above, I filed #8494 and #8495