Enforce clang-tidy, at a version that cannot move under us

[martinus]

Closes #88.

The problem was curation, not a missing workflow

This repository has maintained a .clang-tidy and 13 NOLINT comments in the header for years without ever running clang-tidy in CI. The config said Checks: "*", which enables every check any future release adds, so wiring it up naively would have meant a red build on every toolchain bump for reasons unrelated to any change. So this does the curation first.

Families are now listed explicitly, and the ones that are wrong for this code are off with a line each saying why — the version macros cannot be enums because the preprocessor builds the inline namespace name out of them, storage keeps its elements in a trailing array on purpose, the one enum is a template tag that is never stored.

Pinning the compiler turned out not to be a pin

Families still grow, so the real guarantee is the clang-tidy version. But pinning only the compiler is not enough: clang-tidy parses the code with whatever standard library is installed, and I hit this immediately — clang-tidy-18 against Fedora's libstdc++ 16 dies inside <string>.

So the default is a container image, which pins both halves, with SVECTOR_CLANG_TIDY to override where the environment is known to be consistent. CI sets it to an apt-installed clang-tidy-18, which keeps the lint job quick and off Docker Hub — whose anonymous pull limit is shared across GitHub's runners and would make a required check flaky. With neither available the check skips loudly rather than failing, so lint-all.py stays usable.

A template only gets checked where it is instantiated

svector is a template, so a check only sees what something instantiated. The generated TU deliberately exercises a broad spread of the API: with only a push_back, three quarters of the container is never looked at, and the assign and insert findings below do not appear at all.

What it found

One thing worth fixing: enable_if_t was still spelled the C++11 way. Fixed.

Everything else is deliberate and is now a NOLINT that says so. Three are worth knowing about, because doing what the check asks would put back a bug that has already been fixed once:

• performance-unnecessary-copy-initialization on the copy in assign, resize and insert. That copy is what makes v.resize(1000, v[0]) work — issue v.push_back(v[0]) doesn't work, can lead to segmentation fault on resizing #50.
• cppcoreguidelines-missing-std-forward on resize_after_reserve. Its pack goes to a fill that copies it into every new element; forwarding would move from it on the first element and leave the rest with a moved-from value.
• misc-no-recursion on those same three functions, which re-dispatch exactly once after copying the aliasing value out of the way.

The rest are the tagged pointer being itself: sizeof(ptr) really does mean the pointer, and the scope guards really are meant not to be movable — their copies are = delete, which already suppresses the implicit moves, and a guard you can move out of its scope is not doing its job.

The issue asked me to look properly at missing-std-forward and storage_guard's special members as "plausibly substantive". Both are correct as written, and the first would be an outright bug if changed.

Verified

• The suite passes with the enable_if_t change.
• lint-all.py is green, and picks the new linter up automatically through its lint-* glob.
• The check actually fails on a planted violation, not just passes when clean — I confirmed with a braces-around-statements violation, since a lint job that cannot fail is worthless. My first attempt at that test was a bad one: a raw new triggered nothing, because cppcoreguidelines-owning-memory is deliberately off.

Note

The one thing CI cannot tell you is whether the container path works, since CI takes the override. I ran both locally: container clean, override reproducing the libstdc++ 16 incompatibility described above.

🤖 Generated with Claude Code