From 603f7ffb1d9b3ac42c13e4bd25ec33ac271d9269 Mon Sep 17 00:00:00 2001 From: helly25 <6420169+helly25@users.noreply.github.com> Date: Sat, 8 Aug 2026 20:52:48 +0100 Subject: [PATCH] Apply the safe misc-const-correctness fixes 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> --- mbo/container/internal/limited_ordered.h | 14 +++++++------- mbo/container/limited_set_benchmark.h | 4 ++-- mbo/container/limited_vector.h | 10 +++++----- mbo/diff/internal/context.h | 2 +- mbo/diff/internal/data.cc | 2 +- mbo/diff/internal/update_absl_log_flags.cc | 4 ++-- mbo/file/glob.cc | 2 +- mbo/file/glob.h | 2 +- mbo/file/glob_test.cc | 2 +- mbo/json/json.h | 2 +- mbo/log/scoped_stream.h | 2 +- mbo/strings/split.h | 2 +- mbo/testing/matchers.h | 2 +- mbo/testing/runfiles_dir.cc | 2 +- mbo/testing/status.h | 2 +- mbo/testing/status_test.cc | 2 +- mbo/types/stringify_ostream.cc | 6 +++--- 17 files changed, 31 insertions(+), 31 deletions(-) diff --git a/mbo/container/internal/limited_ordered.h b/mbo/container/internal/limited_ordered.h index 9460ad61..a06be013 100644 --- a/mbo/container/internal/limited_ordered.h +++ b/mbo/container/internal/limited_ordered.h @@ -627,7 +627,7 @@ class [[nodiscard]] LimitedOrdered { MBO_ALWAYS_INLINE constexpr std::size_t index_of(const Key& key) const requires(!kOptimizeIndexOf || (kOptimizeIndexOf && !kCustomIndexOfBeyondUnroll && Capacity > kUnrollMaxCapacity)) { - const_iterator it = lower_bound(key); + const const_iterator it = lower_bound(key); return it == end() || key_comp_(key, GetKey(*it)) ? npos : it - begin(); } @@ -643,17 +643,17 @@ class [[nodiscard]] LimitedOrdered { MBO_FORCE_INLINE constexpr iterator find(const Key& key) { if constexpr (kOptimizeIndexOf) { - std::size_t pos = index_of(key); + const std::size_t pos = index_of(key); return pos == npos ? end() : iterator(&values_[pos]); } else { // Not kOptimizeIndexOf - iterator it = lower_bound(key); + const iterator it = lower_bound(key); return it == end() || key_comp_(key, GetKey(*it)) ? end() : it; } } MBO_FORCE_INLINE constexpr const_iterator find(const Key& key) const { if constexpr (kOptimizeIndexOf) { - std::size_t pos = index_of(key); + const std::size_t pos = index_of(key); return pos == npos ? end() : const_iterator(&values_[pos]); } else { // Not kOptimizeIndexOf const_iterator it = lower_bound(key); @@ -755,8 +755,8 @@ class [[nodiscard]] LimitedOrdered { std::swap(values_[pos].data.second, other.values_[pos].data.second); } } - std::size_t other_size = other.size_; - std::size_t this_size = size_; + const std::size_t other_size = other.size_; + const std::size_t this_size = size_; for (; pos < size_; ++pos) { other.emplace(std::move(values_[pos].data)); } @@ -769,7 +769,7 @@ class [[nodiscard]] LimitedOrdered { template constexpr std::pair emplace(Args&&... args) noexcept(!kRequireThrows) { - RawValue new_val(std::forward(args)...); + const RawValue new_val(std::forward(args)...); const iterator dst = lower_bound(GetKey(new_val)); if (dst != end() && !key_comp_(GetKey(*dst), GetKey(new_val)) && !key_comp_(GetKey(new_val), GetKey(*dst))) { return std::make_pair(dst, false); diff --git a/mbo/container/limited_set_benchmark.h b/mbo/container/limited_set_benchmark.h index f6f49d36..f01ee696 100644 --- a/mbo/container/limited_set_benchmark.h +++ b/mbo/container/limited_set_benchmark.h @@ -103,8 +103,8 @@ class Benchmarks { } return test_data; }(); - std::size_t test = 0; - std::size_t item = 0; + const std::size_t test = 0; + const std::size_t item = 0; int64_t item_count = 0; const auto* data = &test_data[0].data; const auto* input = &test_data[0].input; diff --git a/mbo/container/limited_vector.h b/mbo/container/limited_vector.h index e759dab9..ad4629d0 100644 --- a/mbo/container/limited_vector.h +++ b/mbo/container/limited_vector.h @@ -267,8 +267,8 @@ class LimitedVector final { for (; pos < size_ && pos < other.size(); ++pos) { std::swap(values_[pos].data, other.at(pos)); } - std::size_t other_size = other.size_; - std::size_t this_size = size_; + const std::size_t other_size = other.size_; + const std::size_t this_size = size_; for (; pos < size_; ++pos) { other.emplace_back(std::move(values_[pos].data)); } @@ -519,7 +519,7 @@ LimitedVector(T&&... v) -> LimitedVector, sizeof...(T)> template requires std::three_way_comparable_with constexpr inline auto operator<=>(const LimitedVector& lhs, const LimitedVector& rhs) noexcept { - std::size_t minsize = std::min(LN, RN); + const std::size_t minsize = std::min(LN, RN); for (std::size_t index = 0; index < minsize; ++index) { const auto comp = lhs[index] <=> rhs[index]; if (comp != 0) { @@ -535,7 +535,7 @@ constexpr inline bool operator==(const LimitedVector& lhs, const Limite if (lhs.size() != rhs.size()) { return false; } - std::size_t minsize = std::min(LN, RN); + const std::size_t minsize = std::min(LN, RN); for (std::size_t index = 0; index < minsize; ++index) { const auto comp = lhs[index] <=> rhs[index]; if (comp != 0) { @@ -548,7 +548,7 @@ constexpr inline bool operator==(const LimitedVector& lhs, const Limite template requires std::three_way_comparable_with constexpr inline bool operator<(const LimitedVector& lhs, const LimitedVector& rhs) noexcept { - std::size_t minsize = std::min(LN, RN); + const std::size_t minsize = std::min(LN, RN); for (std::size_t index = 0; index < minsize; ++index) { const auto comp = lhs[index] <=> rhs[index]; if (comp != 0) { diff --git a/mbo/diff/internal/context.h b/mbo/diff/internal/context.h index 9ef6a0f4..08d66d68 100644 --- a/mbo/diff/internal/context.h +++ b/mbo/diff/internal/context.h @@ -53,7 +53,7 @@ class Context final { } std::string_view PopFront() noexcept { - std::string_view result = data_.front(); + const std::string_view result = data_.front(); data_.pop_front(); return result; } diff --git a/mbo/diff/internal/data.cc b/mbo/diff/internal/data.cc index 1f85bfe4..c3286c59 100644 --- a/mbo/diff/internal/data.cc +++ b/mbo/diff/internal/data.cc @@ -82,7 +82,7 @@ std::vector Data::SplitAndAdaptLastLine( const std::size_t count = std::count_if(text.begin(), text.end(), [](char chr) { return chr == '\n'; }); std::vector result; result.reserve(count + 1); // N newlines split into N + 1 lines. - for (std::string_view line : absl::StrSplit(text, '\n')) { + for (const std::string_view line : absl::StrSplit(text, '\n')) { result.push_back(Process(options, regex_replace, line, owned)); } // A missing final newline normally makes the last line carry the `\ No newline at end of file` diff --git a/mbo/diff/internal/update_absl_log_flags.cc b/mbo/diff/internal/update_absl_log_flags.cc index 6c58bcd3..1a94ea94 100644 --- a/mbo/diff/internal/update_absl_log_flags.cc +++ b/mbo/diff/internal/update_absl_log_flags.cc @@ -34,13 +34,13 @@ void UpdateAbslLogFlags() { // cannot be called to prevent duplicate initalization which triggers // `absl::log_internal::SetTimeZone() has already been called`. { - absl::CommandLineFlag* flag = absl::FindCommandLineFlag("minloglevel"); + const absl::CommandLineFlag* flag = absl::FindCommandLineFlag("minloglevel"); if (flag->CurrentValue() == flag->DefaultValue()) { absl::SetFlag(&FLAGS_minloglevel, 1); } } { - absl::CommandLineFlag* flag = absl::FindCommandLineFlag("stderrthreshold"); + const absl::CommandLineFlag* flag = absl::FindCommandLineFlag("stderrthreshold"); if (flag->CurrentValue() == flag->DefaultValue()) { absl::SetFlag(&FLAGS_stderrthreshold, 1); } diff --git a/mbo/file/glob.cc b/mbo/file/glob.cc index 56e72f56..572ce4e7 100644 --- a/mbo/file/glob.cc +++ b/mbo/file/glob.cc @@ -149,7 +149,7 @@ MBO_ALWAYS_INLINE absl::StatusOr GlobFindRange(std::string_view& result.has_slash = true; continue; case '-': { - char last = re2_pattern.back(); + const char last = re2_pattern.back(); re2_pattern += chr; pattern.remove_prefix(1); if (pattern.front() == ']') { diff --git a/mbo/file/glob.h b/mbo/file/glob.h index 2dec5439..4eccb7ed 100644 --- a/mbo/file/glob.h +++ b/mbo/file/glob.h @@ -142,7 +142,7 @@ struct GlobEntry : mbo::types::Extend { return 0; } std::error_code error; - std::size_t result = entry.file_size(error); + const std::size_t result = entry.file_size(error); return error ? 0 : result; } }; diff --git a/mbo/file/glob_test.cc b/mbo/file/glob_test.cc index 76b3b3fa..3af32ccf 100644 --- a/mbo/file/glob_test.cc +++ b/mbo/file/glob_test.cc @@ -300,7 +300,7 @@ absl::StatusOr CreateFileSystemEntries( return absl::AbortedError(absl::StrCat("Cannot create dir: ", path)); } if (!file.empty()) { - std::ofstream output(*root / path / file, std::ios::binary); + const std::ofstream output(*root / path / file, std::ios::binary); } } return *root; diff --git a/mbo/json/json.h b/mbo/json/json.h index 481b12b8..1ea7e58c 100644 --- a/mbo/json/json.h +++ b/mbo/json/json.h @@ -388,7 +388,7 @@ class Json { std::ostream& os, SerializeMode mode = SerializeMode::kCompact, const types::StringifyRootOptions& root_options = types::StringifyRootOptions{}) const { - ::mbo::types::Stringify stringify{static_cast(mode), root_options}; + const ::mbo::types::Stringify stringify{static_cast(mode), root_options}; if (IsNull()) { struct Null {}; diff --git a/mbo/log/scoped_stream.h b/mbo/log/scoped_stream.h index 1ca43624..652f54d8 100644 --- a/mbo/log/scoped_stream.h +++ b/mbo/log/scoped_stream.h @@ -255,7 +255,7 @@ void ScopedStreamErr(const Disallowed&&) { // NOLINT(*-named-parameter) template MBO_FORCE_INLINE auto ScopedStreamVoid(const std::source_location& loc = std::source_location::current()) { - static VoidStream void_stream; + static const VoidStream void_stream; return ScopedStream(loc, void_stream); } diff --git a/mbo/strings/split.h b/mbo/strings/split.h index a4ff1123..5f94a516 100644 --- a/mbo/strings/split.h +++ b/mbo/strings/split.h @@ -27,7 +27,7 @@ class AtLast { explicit AtLast(char sep) : sep_(sep) {} std::string_view Find(std::string_view text, std::size_t pos) const { - std::size_t next_pos = text.substr(pos).rfind(sep_); + const std::size_t next_pos = text.substr(pos).rfind(sep_); if (next_pos == std::string_view::npos) { return std::string_view{text.data() + text.size(), 0}; } diff --git a/mbo/testing/matchers.h b/mbo/testing/matchers.h index ead7fe76..4333adae 100644 --- a/mbo/testing/matchers.h +++ b/mbo/testing/matchers.h @@ -247,7 +247,7 @@ class CapacityIsMatcher { } bool MatchAndExplain(Container container, ::testing::MatchResultListener* listener) const override { - CapacityType capacity = container.capacity(); + const CapacityType capacity = container.capacity(); ::testing::StringMatchResultListener capacity_listener; const bool result = capacity_matcher_.MatchAndExplain(capacity, &capacity_listener); *listener << "whose capacity " << capacity << (result ? " matches" : " doesn't match"); diff --git a/mbo/testing/runfiles_dir.cc b/mbo/testing/runfiles_dir.cc index 162b7957..d71e2b42 100644 --- a/mbo/testing/runfiles_dir.cc +++ b/mbo/testing/runfiles_dir.cc @@ -87,7 +87,7 @@ absl::StatusOr RunfilesDir(std::string_view workspace, std::string_ } const std::string mapping_file = absl::StrCat(test_bin, "/_repo_mapping"); MBO_ASSIGN_OR_RETURN(const std::string mapping, mbo::file::GetContents(mapping_file)); - for (std::string_view line : absl::StrSplit(mapping, '\n')) { + for (const std::string_view line : absl::StrSplit(mapping, '\n')) { const std::vector parts = absl::StrSplit(line, ',', absl::AllowEmpty()); if (parts.size() == 3 && parts[1] == workspace) { return runfiles->Rlocation(mbo::file::JoinPaths(parts[2], source_rel)); diff --git a/mbo/testing/status.h b/mbo/testing/status.h index 3975608e..defe36ee 100644 --- a/mbo/testing/status.h +++ b/mbo/testing/status.h @@ -334,7 +334,7 @@ class StatusPayloads { payload_map.emplace(type_url, payload); }); ::testing::StringMatchResultListener inner; - bool match = payload_matcher_.MatchAndExplain(payload_map, &inner); + const bool match = payload_matcher_.MatchAndExplain(payload_map, &inner); if (inner.str().empty()) { if (actual_status.ok()) { *listener << "which has OK status (and no payload)"; diff --git a/mbo/testing/status_test.cc b/mbo/testing/status_test.cc index b5f0f9cb..64050715 100644 --- a/mbo/testing/status_test.cc +++ b/mbo/testing/status_test.cc @@ -176,7 +176,7 @@ TEST_F(StatusMatcherTest, StatusIsWithCodeMatcher) { EXPECT_THAT(absl::OkStatus(), Not(StatusIs(Ne(absl::StatusCode::kOk)))); EXPECT_THAT(absl::AbortedError("boom"), StatusIs(Ne(absl::StatusCode::kOk), HasSubstr("boom"))); - absl::StatusOr status_or = absl::UnavailableError("down"); + const absl::StatusOr status_or = absl::UnavailableError("down"); EXPECT_THAT(status_or, StatusIs(AnyOf(absl::StatusCode::kUnavailable, absl::StatusCode::kDeadlineExceeded))); EXPECT_THAT(status_or, Not(StatusIs(absl::StatusCode::kNotFound))); diff --git a/mbo/types/stringify_ostream.cc b/mbo/types/stringify_ostream.cc index e5a722eb..43ad107e 100644 --- a/mbo/types/stringify_ostream.cc +++ b/mbo/types/stringify_ostream.cc @@ -31,7 +31,7 @@ std::shared_ptr g_stringify ABSL_GUARDED_BY(g_mx) = nullptr; namespace types_internal { std::shared_ptr GetStringifyForOstream() { - absl::MutexLock lock(g_mx); + const absl::MutexLock lock(g_mx); if (g_stringify == nullptr) { g_stringify = std::make_shared(); } @@ -41,12 +41,12 @@ std::shared_ptr GetStringifyForOstream() { } // namespace types_internal void SetStringifyOstreamOutputMode(Stringify::OutputMode output_mode) { - absl::MutexLock lock(g_mx); + const absl::MutexLock lock(g_mx); g_stringify.reset(new Stringify(output_mode)); // NOLINT } void SetStringifyOstreamOptions(const StringifyOptions& options) { - absl::MutexLock lock(g_mx); + const absl::MutexLock lock(g_mx); g_stringify.reset(new Stringify(options)); // NOLINT }