Making use of static analysis to find bugs and UB

[xokdvium]

Is your feature request related to a problem? Please describe.
C++ is an inherently complex language and has countless footguns. Currently there are no safeguards against this and bug lifetime is very long. This makes even tiny refactoring hard because even a tiny change can lead to run-time errors or memory corruption. A lot of errors are left undiscovered for a long time or are missed during review.

Some examples:

• fix(libutil/posix-source-accessor.cc): get rid of use-after-move bug #11837 - This is trivially caught by the cplusplus.Move.html check.
• fix(libstore-tests): remove use-after-free bug for StringSource #11813
• fix(nix/eval.cc): move call to fs::create_directory out of assert #11718 - bugprone/assert-side-effect.html

C++ compilers aren't very helpful most of the time, and even for diagnosable errors current compiler flags don't include enough warnings.

Describe the solution you'd like
Include clang-tidy runs and more compiler warnings in the build process and CI. Ideally clang-tidy would be run for every change.

Meson has native clang-tidy integration since 0.52 and has been extended in 1.3 to enable automatic fixes via the clang-tidy-fix target.
We can start by bumping the warning_level in Meson and enabling the bare minimum checks for easily diagnosable issues like clang-analyzer-* and some bugprone-* checks.

There are some potential downsides like:

• Increased load on the CI builders.
• More stringent review process.
• Opinionated lints and checks being a source of debate for developers and maintainers. We need to make sure to only enable the checks that have the potential to uncover bugs.
• False positives that need to be explicitly disabled.

Describe alternatives you've considered

• Running clang-tidy out-of-tree.
• Praying to the C++ gods that UB doesn't spawn nasal demons.

Additional context

• Enable C++ Sanitizers for Nix #10969 (comment)

Priorities

Add 👍 to issues you find important.

[Mic92]

I would appreciate having clang-tidy run on nix. If it the meson integration would work for us, we should use it. That target should likely go in it's own derivation.

[xokdvium]

That target should likely go in it's own derivation.

Should this be a one-per-library derivation or can we lint the whole codebase at once? @Ericson2314
Not sure if the lint results can be effectively cached.

[Ericson2314]

@xokdvium I am fine either way re granularity; assume this is pretty fast right?

[xokdvium]

Regarding the runtime. On my machine with -j24 ninjaBuildPhase for native-clangStdenvPackages :

real    2m0.979s
user    36m41.883s
sys     2m17.883s

I guess the runtime would be very sensitive to the enabled checks. For the basic barebones config, which gives too many false-positives, but is an ok baseline:

Checks:
  - -*
  - performance-*
  - bugprone-*
  - clang-analyzer-*

Running the clang-tidy via meson on all threads -j24 gives around:

real    11m20.384s
user    194m51.148s
sys     4m20.908s

I wouldn't call that pretty fast.

But this shouldn't be run very often, since for CI we are primarily interested in diffs. It should be pretty easy to reuse the clang-tidy-diff.py for checking only the affected files. That would require going around the Meson integration and wrapping clang-tidy-diff.

[Mic92]

Should this be a one-per-library derivation or can we lint the whole codebase at once? @Ericson2314 Not sure if the lint results can be effectively cached.

It's probably better to start having this once for the whole code base. The per library derivation would probably introduce quite a bit of complexity that we maybe want to avoid for the first iteration. But also your clang-tidy-diff approach sound good to me, given we have github actions anyway and this would make the whole thing a lot faster.