Clear misc-const-correctness, fix a build break from #275, quiet four checks - #285
Merged
Conversation
helly25
enabled auto-merge (squash)
August 9, 2026 20:06
Fab-Cat
approved these changes
Aug 9, 2026
helly25
force-pushed
the
clang_tidy_tail
branch
from
August 9, 2026 20:40
a289df3 to
a40cd6e
Compare
Two of the sites were real and are fixed:
* stringify.h - a loop `std::string_view` that is never mutated, which
STYLE_CPP.md now says should be const.
* optional_data_or_ref_test.cc - a `std::string` only ever read.
The rest are the check being wrong, so they get a NOLINT naming why:
* struct_names_clang.h (4) - `field_index` and `storage` are passed to
FieldCount()/Init() by non-const reference and via `&storage.Get()`.
#275 showed const there fails to compile.
* optional_ref_test.cc, optional_data_or_ref_test.cc (9) - these
declarations are the subject under test. const changes the deduced
type a neighbouring `static_assert(...decltype(ref)...)` checks.
misc-const-correctness now reports zero.
Signed-off-by: helly25 <6420169+helly25@users.noreply.github.com>
#275 made `test` and `item` const in limited_set_benchmark.h, but both are assigned: ./mbo/container/limited_set_benchmark.h:134:11: error: cannot assign to variable 'item' with const-qualified type 'const std::size_t' It reached main because `bazel test //...` skips `manual` targets, so nothing built the benchmark. Only #280 - which put those targets into the compile DB - made clang-tidy able to see it. Reverted, and CI now builds every clang-tidy-tagged manual target so the next one cannot slip through the same way. All five build. tools/clang_tidy.sh now skips mbo/hash/measurements/smhasher3/: bazel does not build it (it is an SMHasher3 plugin compiled by that project's cmake), so clang-tidy was linting it with flags guessed from unrelated files and reporting its SMHasher3 includes as missing. That removes 4 spurious categories at once, including all 10 c-style-cast findings. Disabled, each with its reason recorded: * misc-no-recursion - recursion is the shape of the code it flags, and it reports every member of a call chain. * bugprone-std-namespace-modification - the `namespace std` blocks are specialisations of std templates for our own types. * cppcoreguidelines-avoid-magic-numbers / readability-magic-numbers (aliases) - they fire on test expectations and on the numeric tables that ARE the subject. bazel test --config=clang //... - 109/109 pass. Signed-off-by: helly25 <6420169+helly25@users.noreply.github.com>
helly25
force-pushed
the
clang_tidy_tail
branch
from
August 9, 2026 20:41
a40cd6e to
422a9e1
Compare
helly25
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits. The second one is the important find.
1.
misc-const-correctness— clearedCloses out the check #275 could only half-finish. Two sites were real (a never-mutated loop
std::string_view, a read-onlystd::string); the other 13 getNOLINTs naming why the check is wrong: parameters passed by non-const reference, and declarations whoseconst-ness would change the deduced type a neighbouringstatic_assert(...decltype(ref)...)checks.2. A build break that #275 shipped to
main#275 made
testanditemconst, but both are assigned. It reachedmainbecausebazel test //...skipsmanualtargets, so nothing ever built the benchmark — my full-suite run reported 109/109 green while the benchmark was broken.It only became visible now because #280 put those
manualtargets into the compile DB, so clang-tidy could finally see them.Reverted, and CI now builds every
clang-tidy-tagged manual target, so the next one cannot slip through the same way. Verified all five build.Also: stop linting a file bazel does not build
tools/clang_tidy.shnow skipsmbo/hash/measurements/smhasher3/. It is an SMHasher3 plugin compiled by that project's cmake, so clang-tidy was linting it with flags guessed from unrelated files and reporting itsPlatform.h/Hashlib.hincludes as missing. Removes 4 spurious categories at once, including all 10 c-style-cast findings.Four checks disabled, each with its reason in
.clang-tidymisc-no-recursion— recursion is the shape of the code it flags (Stringifywalks nested structures,BigNumberLenrecurses once for negatives), and it reports every member of a call chain, so one algorithm yields a cluster.bugprone-std-namespace-modification— thenamespace stdblocks are specialisations of std templates for our own types, which is how those extension points are meant to be used.cppcoreguidelines-avoid-magic-numbers/readability-magic-numbers(aliases) — they fire on test expectations and on the numeric tables that are the subject.Test
misc-const-correctnessreports zero.clang-tidy-tagged manual targets build.bazel test --config=clang //...— 109/109 pass.pre-commit run -agreen.