From c1225c0dba8433f893ff8a92c58f8b684ccd7053 Mon Sep 17 00:00:00 2001 From: helly25 <6420169+helly25@users.noreply.github.com> Date: Sat, 8 Aug 2026 17:46:06 +0100 Subject: [PATCH] Clear the three most concentrated clang-tidy checks google-build-explicit-make-pair (32 findings, mbo/strings/numbers.h): a real defect. `std::make_pair(...)` with explicit template arguments pins the parameters to `uint32_t&&`/`unsigned&&`, which defeats the deduction make_pair exists for. Use std::pair directly, as the diagnostic says. performance-unnecessary-value-param (25 findings, mbo/container/any_scan.h): one source location, re-reported per template instantiation. Note the motivation is NOT copy cost -- MakeAnyScanData is 16 bytes holding one shared_ptr, so a copy is a single atomic increment, and by-value vs const& compile to the identical indirect parameter (both `ptr` in IR; the type is not register-transportable because shared_ptr is not trivially copyable). By-value simply buys nothing here: the const shared_ptr member means even a "move" copies. const& drops one atomic pair per construction. readability-implicit-bool-conversion (23 findings): not defects. * diff_myers.cc: `(d + lo) & 1` is an idiomatic parity test. Allow integer and pointer conditions via check options, which is standard C++ style and clears these legitimately. * hash_differential_test.cc: inside the XXH64 / XXH3_* reference macros the check misattributes a conversion to the gtest `<<` message chain -- it points at string literals like "len: " and proposes replacing them with `true`. Suppressed with a rationale, extending the NOLINT region the file already uses. All three checks now report zero. Tests pass across the touched packages (9/9). Signed-off-by: helly25 <6420169+helly25@users.noreply.github.com> --- .clang-tidy | 4 ++++ mbo/container/any_scan.h | 4 ++-- mbo/hash/hash_differential_test.cc | 8 ++++++-- mbo/strings/numbers.h | 4 ++-- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 005de779..aee435b6 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -84,6 +84,10 @@ CheckOptions: value: 'NULL' - key: performance-unnecessary-value-param.AllowedTypes value: 'absl::Status;absl::StatusOr;std::string_view' + - key: readability-implicit-bool-conversion.AllowIntegerConditions + value: '1' + - key: readability-implicit-bool-conversion.AllowPointerConditions + value: '1' - key: readability-function-cognitive-complexity.Threshold value: '25' - key: readability-braces-around-statements.ShortStatementLines diff --git a/mbo/container/any_scan.h b/mbo/container/any_scan.h index 036da653..8f2b12b9 100644 --- a/mbo/container/any_scan.h +++ b/mbo/container/any_scan.h @@ -353,7 +353,7 @@ class AnyScanImpl { // For MakAnyScan / MakeConstScan template requires(kAccessByRef) - explicit AnyScanImpl(MakeAnyScanData data) + explicit AnyScanImpl(const MakeAnyScanData& data) : funcs_{ .iter = [data = data] { @@ -374,7 +374,7 @@ class AnyScanImpl { !kAccessByRef // This is the ConvertingScan constructor && types::ConstructibleFrom> && types::ConstructibleFrom) - explicit AnyScanImpl(MakeAnyScanData data) + explicit AnyScanImpl(const MakeAnyScanData& data) : funcs_{ // NOTE: data must be copied here! .iter = diff --git a/mbo/hash/hash_differential_test.cc b/mbo/hash/hash_differential_test.cc index 76a8cf58..d371014c 100644 --- a/mbo/hash/hash_differential_test.cc +++ b/mbo/hash/hash_differential_test.cc @@ -31,7 +31,11 @@ namespace mbo::hash { namespace { -// NOLINTBEGIN(*-magic-numbers) +// readability-implicit-bool-conversion is suppressed rather than "fixed": inside +// the reference macros (XXH64 / XXH3_*) it misattributes a conversion to the gtest +// `<<` message chain, pointing at string literals like "len: " and proposing they +// be replaced with `true`. Nothing here converts anything to bool. +// NOLINTBEGIN(*-magic-numbers,readability-implicit-bool-conversion) TEST(DifferentialTest, Xxh64MatchesReference) { std::mt19937_64 rng(0xD1FF64U); // NOLINT(cert-msc51-cpp,cert-msc32-c,bugprone-random-generator-seed): reproducible @@ -89,7 +93,7 @@ TEST(DifferentialTest, Xxh3Hash128MatchesReference) { } } -// NOLINTEND(*-magic-numbers) +// NOLINTEND(*-magic-numbers,readability-implicit-bool-conversion) } // namespace } // namespace mbo::hash diff --git a/mbo/strings/numbers.h b/mbo/strings/numbers.h index f36588e9..346477bc 100644 --- a/mbo/strings/numbers.h +++ b/mbo/strings/numbers.h @@ -51,7 +51,7 @@ unsigned BigNumberLen(T v) { // We first check whether we can do even better by limiting us to 4 byte types. // We use a macro to let the compiler compute the actual length values. if constexpr (sizeof(v) <= 4) { -#define CHECK_CAP(cap) std::make_pair(cap, std::string_view(#cap).size() - 3) +#define CHECK_CAP(cap) std::pair(cap, std::string_view(#cap).size() - 3) constexpr auto kData = mbo::container::ToLimitedMap>({ CHECK_CAP(4'294'967'295ULL), CHECK_CAP(999'999'999ULL), @@ -68,7 +68,7 @@ unsigned BigNumberLen(T v) { #undef CHECK_CAP return kData.lower_bound(v)->second; } else { -#define CHECK_CAP(cap) std::make_pair(cap, std::string_view(#cap).size() - 3) +#define CHECK_CAP(cap) std::pair(cap, std::string_view(#cap).size() - 3) constexpr auto kData = mbo::container::ToLimitedMap>({ CHECK_CAP(18'446'744'073'709'551'615ULL), CHECK_CAP(9'999'999'999'999'999'999ULL),