Apply the safe misc-const-correctness fixes - #275
Merged
Conversation
31 const additions across 17 files, all of them locals that are genuinely
never modified.
The check's automatic fixes are NOT safe on this codebase and were not
taken wholesale. Applying all of them broke the build in three distinct
ways:
* mbo/types/stringify.h:916 - "cannot assign to variable 'idx' with
const-qualified type". The variable is assigned; the check was simply
wrong.
* mbo/types/internal/struct_names_clang.h:127 - a non-const reference
can no longer bind to the now-const value.
* mbo/types/optional_ref_test.cc, optional_data_or_ref_test.cc - static
assertions fail, because const changes the deduced type the tests
assert on.
mbo/hash/hash_benchmark.cc is the clearest case: it proposed const for
`total_bytes`, which is `+=`-accumulated, and `counter`, which is
`counter++`-incremented. These are template-heavy sites where the
mutating instantiation is not visible to the translation unit that
exported the fix.
Those five files are reverted and keep 18 findings, to be handled
individually rather than by --fix. Everything here is a bare `const`
addition; no logic changed.
Fixes were exported per TU and merged with clang-apply-replacements
rather than applying --fix in parallel, so that headers shared by many
TUs could not be written concurrently.
bazel test --config=clang //... - 109/109 pass.
Signed-off-by: helly25 <6420169+helly25@users.noreply.github.com>
helly25
enabled auto-merge (squash)
August 8, 2026 20:23
Fab-Cat
approved these changes
Aug 8, 2026
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.
Fifth clang-tidy triage PR. 31
constadditions across 17 files — all bareconst, no logic changed.The headline: this check's
--fixis not safe hereI did not take the automatic fixes wholesale, and that turned out to matter. Applying all of them broke the build in three distinct ways:
mbo/types/stringify.h:916cannot assign to variable 'idx' with const-qualified type— the variable is assigned. The check was simply wrong.mbo/types/internal/struct_names_clang.h:127optional_ref_test.cc,optional_data_or_ref_test.ccstatic assertion failed—constchanges the deduced type the tests assert onmbo/hash/hash_benchmark.ccis the clearest case. It proposedconstfor:These are template-heavy sites where the mutating instantiation is not visible to the translation unit that exported the fix. Anyone running
clang-tidy --fixfor this check on this repo will silently break it.Those five files are reverted here and keep 18 findings, to be handled individually rather than mechanically.
Method
Fixes were exported per TU with
--export-fixesand merged once viaclang-apply-replacements, rather than running--fixin parallel. With 81 TUs and headers included by many of them, parallel--fixwould have had several processes rewriting the same header concurrently.Test
bazel test --config=clang //...— 109/109 pass.constaddition (git diffcontains no other kind of line).pre-commit run -agreen.Follow-up
The remaining 18 findings live in
stringify.cc(1),hash_benchmark.cc(6),struct_names_test.cc(4),optional_ref_test.cc(1),optional_data_or_ref_test.cc(6). Since several are demonstrably wrong rather than merely awkward,NOLINTwith a per-site reason looks right — but that is a judgement call per site, so it is left out of this PR.