Fix a batch of clang-tidy findings in src/#2252
Open
devtejasx wants to merge 5 commits into
Open
Conversation
Applies the misc-const-correctness fixes flagged in google#1925 across src/, covering the findings reachable in a Windows configuration. Refs google#1925
Fixes the readability-uppercase-literal-suffix findings from google#1925 in src/ (0u -> 0U). Refs google#1925
Fixes readability-simplify-boolean-expr, readability-isolate-declaration and readability-use-anyofallof findings from google#1925 in src/: apply De Morgan simplifications in benchmark_runner.cc and commandlineflags.cc, split a multi-variable declaration in sysinfo.cc, and replace a hand-written find loop in SameNames with std::all_of. Refs google#1925
Fixes the misc-use-anonymous-namespace findings from google#1925 in src/: IgnoreColorPrint and FormatTime in console_reporter.cc were file-static functions. Refs google#1925
Member
|
is there any kind of enforcement we can apply? we have a clang-tidy workflow that runs on PRs but it wasn't flagging any of these issues. |
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.
Works through a slice of the clang-tidy findings collated in #1925, scoped to
src/and split into one commit per check for easier review:misc-const-correctness: declare immutable localsconst(13 files, includes theMutexLockguards and theStateobjects constructed for Setup/Teardown callbacks)readability-uppercase-literal-suffix:0u->0Ureadability-simplify-boolean-expr/readability-isolate-declaration/readability-use-anyofallof: De Morgan simplifications inbenchmark_runner.ccandcommandlineflags.cc, split a multi-variable declaration insysinfo.cc, replace a hand-written find loop inSameNameswithstd::all_ofmisc-use-anonymous-namespace: move the file-staticIgnoreColorPrint/FormatTimehelpers inconsole_reporter.ccinto an anonymous namespaceNot included on purpose:
readability-convert-member-functions-to-staticonPerfCountersMeasurement::Read(the suggestion is only valid for the non-libpfm stub, making it static would break the Linux/libpfm build), and thecppcoreguidelines-avoid-non-const-global-variablesfindings on flag definitions and registration globals, which look intentional.The remaining findings were re-checked with clang-tidy 20 against a fresh
compile_commands.json; the checks addressed here now report clean forsrc/in a Windows configuration (Linux-only code paths, e.g. libpfm perf counters, were not analyzed and may retain findings).Testing: full CMake test suite passes locally in Debug and the library and tests build warning-free in Release (
-Wall -Wextra -Werror, MinGW-w64 GCC 15.2 on Windows 11).Refs #1925
AI disclosure (per AGENTS.md): this change was developed with AI assistance (Anthropic's Claude).