[clang-tidy] autofix readability issues (google#1931)

* [clang-tidy] autofix readability issues

* more modern clang format

Author: dmah42

.github/workflows/clang-format-lint.yml

@@ -10,9 +10,7 @@ jobs:
 
     steps:
     - uses: actions/checkout@v4
-    - uses: DoozyX/clang-format-lint-action@v0.15
+    - uses: DoozyX/clang-format-lint-action@v0.18.2
       with:
         source: './include/benchmark ./src ./test'
-        extensions: 'h,cc'
-        clangFormatVersion: 12
-        style: Google
+        clangFormatVersion: 18

include/benchmark/benchmark.h

@@ -313,7 +313,7 @@ BENCHMARK_EXPORT std::string GetBenchmarkVersion();
 BENCHMARK_EXPORT void PrintDefaultHelp();
 
 BENCHMARK_EXPORT void Initialize(int* argc, char** argv,
-                                 void (*HelperPrinterf)() = PrintDefaultHelp);
+                                 void (*HelperPrintf)() = PrintDefaultHelp);
 BENCHMARK_EXPORT void Shutdown();
 
 // Report to stdout all arguments in 'argv' as unrecognized except the first.
@@ -631,7 +631,7 @@ class Counter {
   Counter(double v = 0., Flags f = kDefaults, OneK k = kIs1000)
       : value(v), flags(f), oneK(k) {}
 
-  BENCHMARK_ALWAYS_INLINE operator double const &() const { return value; }
+  BENCHMARK_ALWAYS_INLINE operator double const&() const { return value; }
   BENCHMARK_ALWAYS_INLINE operator double&() { return value; }
 };
 
@@ -1165,7 +1165,7 @@ class BENCHMARK_EXPORT Benchmark {
   // Pass this benchmark object to *func, which can customize
   // the benchmark by calling various methods like Arg, Args,
   // Threads, etc.
-  Benchmark* Apply(void (*func)(Benchmark* benchmark));
+  Benchmark* Apply(void (*custom_arguments)(Benchmark* benchmark));
 
   // Set the range multiplier for non-dense range. If not called, the range
   // multiplier kRangeMultiplier will be used.
@@ -1869,8 +1869,8 @@ class BENCHMARK_EXPORT ConsoleReporter : public BenchmarkReporter {
   void ReportRuns(const std::vector<Run>& reports) override;
 
  protected:
-  virtual void PrintRunData(const Run& report);
-  virtual void PrintHeader(const Run& report);
+  virtual void PrintRunData(const Run& result);
+  virtual void PrintHeader(const Run& run);
 
   OutputOptions output_options_;
   size_t name_field_width_;
@@ -1886,7 +1886,7 @@ class BENCHMARK_EXPORT JSONReporter : public BenchmarkReporter {
   void Finalize() override;
 
  private:
-  void PrintRunData(const Run& report);
+  void PrintRunData(const Run& run);
 
   bool first_report_;
 };
@@ -1900,7 +1900,7 @@ class BENCHMARK_EXPORT BENCHMARK_DEPRECATED_MSG(
   void ReportRuns(const std::vector<Run>& reports) override;
 
  private:
-  void PrintRunData(const Run& report);
+  void PrintRunData(const Run& run);
 
   bool printed_header_;
   std::set<std::string> user_counter_names_;

src/benchmark.cc

@@ -46,7 +46,6 @@
 #include "commandlineflags.h"
 #include "complexity.h"
 #include "counter.h"
-#include "internal_macros.h"
 #include "log.h"
 #include "mutex.h"
 #include "perf_counters.h"
@@ -198,7 +197,7 @@ State::State(std::string name, IterationCount max_iters,
   // `PauseTiming`, a new `Counter` will be inserted the first time, which
   // won't have the flag.  Inserting them now also reduces the allocations
   // during the benchmark.
-  if (perf_counters_measurement_) {
+  if (perf_counters_measurement_ != nullptr) {
     for (const std::string& counter_name :
          perf_counters_measurement_->names()) {
       counters[counter_name] = Counter(0.0, Counter::kAvgIterations);
@@ -247,7 +246,7 @@ void State::PauseTiming() {
   // Add in time accumulated so far
   BM_CHECK(started_ && !finished_ && !skipped());
   timer_->StopTimer();
-  if (perf_counters_measurement_) {
+  if (perf_counters_measurement_ != nullptr) {
     std::vector<std::pair<std::string, double>> measurements;
     if (!perf_counters_measurement_->Stop(measurements)) {
       BM_CHECK(false) << "Perf counters read the value failed.";
@@ -265,7 +264,7 @@ void State::PauseTiming() {
 void State::ResumeTiming() {
   BM_CHECK(started_ && !finished_ && !skipped());
   timer_->StartTimer();
-  if (perf_counters_measurement_) {
+  if (perf_counters_measurement_ != nullptr) {
     perf_counters_measurement_->Start();
   }
 }
@@ -342,7 +341,7 @@ namespace {
 // Flushes streams after invoking reporter methods that write to them. This
 // ensures users get timely updates even when streams are not line-buffered.
 void FlushStreams(BenchmarkReporter* reporter) {
-  if (!reporter) {
+  if (reporter == nullptr) {
     return;
   }
   std::flush(reporter->GetOutputStream());
@@ -367,7 +366,7 @@ void Report(BenchmarkReporter* display_reporter,
 
   report_one(display_reporter, run_results.display_report_aggregates_only,
              run_results);
-  if (file_reporter) {
+  if (file_reporter != nullptr) {
     report_one(file_reporter, run_results.file_report_aggregates_only,
                run_results);
   }
@@ -408,7 +407,7 @@ void RunBenchmarks(const std::vector<BenchmarkInstance>& benchmarks,
       per_family_reports;
 
   if (display_reporter->ReportContext(context) &&
-      (!file_reporter || file_reporter->ReportContext(context))) {
+      ((file_reporter == nullptr) || file_reporter->ReportContext(context))) {
     FlushStreams(display_reporter);
     FlushStreams(file_reporter);
 
@@ -433,12 +432,12 @@ void RunBenchmarks(const std::vector<BenchmarkInstance>& benchmarks,
       if (benchmark.complexity() != oNone) {
         reports_for_family = &per_family_reports[benchmark.family_index()];
       }
-      benchmarks_with_threads += (benchmark.threads() > 1);
+      benchmarks_with_threads += static_cast<int>(benchmark.threads() > 1);
       runners.emplace_back(benchmark, &perfcounters, reports_for_family);
       int num_repeats_of_this_instance = runners.back().GetNumRepeats();
       num_repetitions_total +=
           static_cast<size_t>(num_repeats_of_this_instance);
-      if (reports_for_family) {
+      if (reports_for_family != nullptr) {
         reports_for_family->num_runs_total += num_repeats_of_this_instance;
       }
     }
@@ -482,7 +481,7 @@ void RunBenchmarks(const std::vector<BenchmarkInstance>& benchmarks,
 
       display_reporter->ReportRunsConfig(
           runner.GetMinTime(), runner.HasExplicitIters(), runner.GetIters());
-      if (file_reporter) {
+      if (file_reporter != nullptr) {
         file_reporter->ReportRunsConfig(
             runner.GetMinTime(), runner.HasExplicitIters(), runner.GetIters());
       }
@@ -506,7 +505,7 @@ void RunBenchmarks(const std::vector<BenchmarkInstance>& benchmarks,
     }
   }
   display_reporter->Finalize();
-  if (file_reporter) {
+  if (file_reporter != nullptr) {
     file_reporter->Finalize();
   }
   FlushStreams(display_reporter);
@@ -569,7 +568,7 @@ ConsoleReporter::OutputOptions GetOutputOptions(bool force_no_color) {
 }  // end namespace internal
 
 BenchmarkReporter* CreateDefaultDisplayReporter() {
-  static auto default_display_reporter =
+  static auto* default_display_reporter =
       internal::CreateReporter(FLAGS_benchmark_format,
                                internal::GetOutputOptions())
           .release();
@@ -611,15 +610,15 @@ size_t RunSpecifiedBenchmarks(BenchmarkReporter* display_reporter,
   std::ofstream output_file;
   std::unique_ptr<BenchmarkReporter> default_display_reporter;
   std::unique_ptr<BenchmarkReporter> default_file_reporter;
-  if (!display_reporter) {
+  if (display_reporter == nullptr) {
     default_display_reporter.reset(CreateDefaultDisplayReporter());
     display_reporter = default_display_reporter.get();
   }
   auto& Out = display_reporter->GetOutputStream();
   auto& Err = display_reporter->GetErrorStream();
 
   std::string const& fname = FLAGS_benchmark_out;
-  if (fname.empty() && file_reporter) {
+  if (fname.empty() && (file_reporter != nullptr)) {
     Err << "A custom file reporter was provided but "
            "--benchmark_out=<file> was not specified.\n";
     Out.flush();
@@ -634,7 +633,7 @@ size_t RunSpecifiedBenchmarks(BenchmarkReporter* display_reporter,
       Err.flush();
       std::exit(1);
     }
-    if (!file_reporter) {
+    if (file_reporter == nullptr) {
       default_file_reporter = internal::CreateReporter(
           FLAGS_benchmark_out_format, FLAGS_benchmark_counters_tabular
                                           ? ConsoleReporter::OO_Tabular
@@ -743,8 +742,8 @@ void SetDefaultTimeUnitFromFlag(const std::string& time_unit_flag) {
 void ParseCommandLineFlags(int* argc, char** argv) {
   using namespace benchmark;
   BenchmarkReporter::Context::executable_name =
-      (argc && *argc > 0) ? argv[0] : "unknown";
-  for (int i = 1; argc && i < *argc; ++i) {
+      ((argc != nullptr) && *argc > 0) ? argv[0] : "unknown";
+  for (int i = 1; (argc != nullptr) && i < *argc; ++i) {
     if (ParseBoolFlag(argv[i], "benchmark_list_tests",
                       &FLAGS_benchmark_list_tests) ||
         ParseStringFlag(argv[i], "benchmark_filter", &FLAGS_benchmark_filter) ||

src/benchmark_api_internal.cc

@@ -101,15 +101,15 @@ State BenchmarkInstance::Run(
 }
 
 void BenchmarkInstance::Setup() const {
-  if (setup_) {
+  if (setup_ != nullptr) {
     State st(name_.function_name, /*iters*/ 1, args_, /*thread_id*/ 0, threads_,
              nullptr, nullptr, nullptr, nullptr);
     setup_(st);
   }
 }
 
 void BenchmarkInstance::Teardown() const {
-  if (teardown_) {
+  if (teardown_ != nullptr) {
     State st(name_.function_name, /*iters*/ 1, args_, /*thread_id*/ 0, threads_,
              nullptr, nullptr, nullptr, nullptr);
     teardown_(st);

src/benchmark_api_internal.h

@@ -17,9 +17,9 @@ namespace internal {
 // Information kept per benchmark we may want to run
 class BenchmarkInstance {
  public:
-  BenchmarkInstance(Benchmark* benchmark, int family_index,
-                    int per_family_instance_index,
-                    const std::vector<int64_t>& args, int threads);
+  BenchmarkInstance(Benchmark* benchmark, int family_idx,
+                    int per_family_instance_idx,
+                    const std::vector<int64_t>& args, int thread_count);
 
   const BenchmarkName& name() const { return name_; }
   int family_index() const { return family_index_; }

src/benchmark_main.cc

@@ -14,5 +14,5 @@
 
 #include "benchmark/benchmark.h"
 
-BENCHMARK_EXPORT int main(int, char**);
+BENCHMARK_EXPORT int main(int /*argc*/, char** /*argv*/);
 BENCHMARK_MAIN();

src/benchmark_name.cc

@@ -27,8 +27,8 @@ size_t size_impl(const Head& head, const Tail&... tail) {
 }
 
 // Join a pack of std::strings using a delimiter
-// TODO: use absl::StrJoin
-void join_impl(std::string&, char) {}
+// TODO(dominic): use absl::StrJoin
+void join_impl(std::string& /*unused*/, char /*unused*/) {}
 
 template <typename Head, typename... Tail>
 void join_impl(std::string& s, const char delimiter, const Head& head,

src/benchmark_register.cc

@@ -53,13 +53,13 @@ namespace benchmark {
 
 namespace {
 // For non-dense Range, intermediate values are powers of kRangeMultiplier.
-static constexpr int kRangeMultiplier = 8;
+constexpr int kRangeMultiplier = 8;
 
 // The size of a benchmark family determines is the number of inputs to repeat
 // the benchmark on. If this is "large" then warn the user during configuration.
-static constexpr size_t kMaxFamilySize = 100;
+constexpr size_t kMaxFamilySize = 100;
 
-static constexpr char kDisabledPrefix[] = "DISABLED_";
+constexpr char kDisabledPrefix[] = "DISABLED_";
 }  // end namespace
 
 namespace internal {
@@ -82,7 +82,7 @@ class BenchmarkFamilies {
 
   // Extract the list of benchmark instances that match the specified
   // regular expression.
-  bool FindBenchmarks(std::string re,
+  bool FindBenchmarks(std::string spec,
                       std::vector<BenchmarkInstance>* benchmarks,
                       std::ostream* Err);
 

src/benchmark_runner.cc

@@ -46,7 +46,6 @@
 #include "commandlineflags.h"
 #include "complexity.h"
 #include "counter.h"
-#include "internal_macros.h"
 #include "log.h"
 #include "mutex.h"
 #include "perf_counters.h"
@@ -74,7 +73,7 @@ ProfilerManager* profiler_manager = nullptr;
 
 namespace {
 
-static constexpr IterationCount kMaxIterations = 1000000000000;
+constexpr IterationCount kMaxIterations = 1000000000000;
 const double kDefaultMinTime =
     std::strtod(::benchmark::kDefaultMinTimeStr, /*p_end*/ nullptr);
 
@@ -100,7 +99,7 @@ BenchmarkReporter::Run CreateRunReport(
   report.repetition_index = repetition_index;
   report.repetitions = repeats;
 
-  if (!report.skipped) {
+  if (report.skipped == 0u) {
     if (b.use_manual_time()) {
       report.real_accumulated_time = results.manual_time_used;
     } else {
@@ -118,9 +117,10 @@ BenchmarkReporter::Run CreateRunReport(
       assert(memory_result != nullptr);
       report.memory_result = memory_result;
       report.allocs_per_iter =
-          memory_iterations ? static_cast<double>(memory_result->num_allocs) /
-                                  static_cast<double>(memory_iterations)
-                            : 0;
+          memory_iterations != 0
+              ? static_cast<double>(memory_result->num_allocs) /
+                    static_cast<double>(memory_iterations)
+              : 0;
     }
 
     internal::Finish(&report.counters, results.iterations, seconds,
@@ -273,10 +273,11 @@ BenchmarkRunner::BenchmarkRunner(
       FLAGS_benchmark_report_aggregates_only;
   if (b.aggregation_report_mode() != internal::ARM_Unspecified) {
     run_results.display_report_aggregates_only =
-        (b.aggregation_report_mode() &
-         internal::ARM_DisplayReportAggregatesOnly);
+        ((b.aggregation_report_mode() &
+          internal::ARM_DisplayReportAggregatesOnly) != 0u);
     run_results.file_report_aggregates_only =
-        (b.aggregation_report_mode() & internal::ARM_FileReportAggregatesOnly);
+        ((b.aggregation_report_mode() &
+          internal::ARM_FileReportAggregatesOnly) != 0u);
     BM_CHECK(FLAGS_benchmark_perf_counters.empty() ||
              (perf_counters_measurement_ptr->num_counters() == 0))
         << "Perf counters were requested but could not be set up.";
@@ -364,7 +365,7 @@ bool BenchmarkRunner::ShouldReportIterationResults(
   // Determine if this run should be reported;
   // Either it has run for a sufficient amount of time
   // or because an error was reported.
-  return i.results.skipped_ || FLAGS_benchmark_dry_run ||
+  return (i.results.skipped_ != 0u) || FLAGS_benchmark_dry_run ||
          i.iters >= kMaxIterations ||  // Too many iterations already.
          i.seconds >=
              GetMinTimeToApply() ||  // The elapsed time is large enough.
@@ -528,9 +529,9 @@ void BenchmarkRunner::DoOneRepetition() {
       CreateRunReport(b, i.results, memory_iterations, memory_result, i.seconds,
                       num_repetitions_done, repeats);
 
-  if (reports_for_family) {
+  if (reports_for_family != nullptr) {
     ++reports_for_family->num_runs_done;
-    if (!report.skipped) {
+    if (report.skipped == 0u) {
       reports_for_family->Runs.push_back(report);
     }
   }

src/benchmark_runner.h

@@ -51,7 +51,7 @@ BenchTimeType ParseBenchMinTime(const std::string& value);
 class BenchmarkRunner {
  public:
   BenchmarkRunner(const benchmark::internal::BenchmarkInstance& b_,
-                  benchmark::internal::PerfCountersMeasurement* pmc_,
+                  benchmark::internal::PerfCountersMeasurement* pcm_,
                   BenchmarkReporter::PerFamilyRunReports* reports_for_family);
 
   int GetNumRepeats() const { return repeats; }