Convert uses of `const char*` to `std::string` (#1567) * Convert uses of const char* to std::string * fix sanitizer builds * reformat user guide * include python bindings * clang-format
diff --git a/bindings/python/google_benchmark/benchmark.cc b/bindings/python/google_benchmark/benchmark.cc index cf2da2e..f444769 100644 --- a/bindings/python/google_benchmark/benchmark.cc +++ b/bindings/python/google_benchmark/benchmark.cc
@@ -34,7 +34,7 @@ return remaining_argv; } -benchmark::internal::Benchmark* RegisterBenchmark(const char* name, +benchmark::internal::Benchmark* RegisterBenchmark(const std::string& name, nb::callable f) { return benchmark::RegisterBenchmark( name, [f](benchmark::State& state) { f(&state); }); @@ -165,8 +165,8 @@ .def_prop_rw("complexity_n", &State::complexity_length_n, &State::SetComplexityN) .def_prop_rw("items_processed", &State::items_processed, - &State::SetItemsProcessed) - .def("set_label", (void (State::*)(const char*)) & State::SetLabel) + &State::SetItemsProcessed) + .def("set_label", &State::SetLabel) .def("range", &State::range, nb::arg("pos") = 0) .def_prop_ro("iterations", &State::iterations) .def_prop_ro("name", &State::name)
diff --git a/docs/user_guide.md b/docs/user_guide.md index 7fffb60..133fca5 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md
@@ -56,7 +56,7 @@ [Exiting with an Error](#exiting-with-an-error) -[A Faster KeepRunning Loop](#a-faster-keep-running-loop) +[A Faster `KeepRunning` Loop](#a-faster-keep-running-loop) ## Benchmarking Tips @@ -271,10 +271,12 @@ Global setup/teardown specific to each benchmark can be done by passing a callback to Setup/Teardown: -The setup/teardown callbacks will be invoked once for each benchmark. -If the benchmark is multi-threaded (will run in k threads), they will be invoked exactly once before -each run with k threads. -If the benchmark uses different size groups of threads, the above will be true for each size group. +The setup/teardown callbacks will be invoked once for each benchmark. If the +benchmark is multi-threaded (will run in k threads), they will be invoked +exactly once before each run with k threads. + +If the benchmark uses different size groups of threads, the above will be true +for each size group. Eg., @@ -1142,7 +1144,7 @@ When errors caused by external influences, such as file I/O and network communication, occur within a benchmark the -`State::SkipWithError(const char* msg)` function can be used to skip that run +`State::SkipWithError(const std::string& msg)` function can be used to skip that run of benchmark and report the error. Note that only future iterations of the `KeepRunning()` are skipped. For the ranged-for version of the benchmark loop Users must explicitly exit the loop, otherwise all iterations will be performed. @@ -1253,7 +1255,8 @@ If you see this error: ``` -***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead. +***WARNING*** CPU scaling is enabled, the benchmark real time measurements may +be noisy and will incur extra overhead. ``` you might want to disable the CPU frequency scaling while running the
diff --git a/include/benchmark/benchmark.h b/include/benchmark/benchmark.h index 153cbba..e44d534 100644 --- a/include/benchmark/benchmark.h +++ b/include/benchmark/benchmark.h
@@ -865,11 +865,7 @@ // BM_Compress 50 50 14115038 compress:27.3% // // REQUIRES: a benchmark has exited its benchmarking loop. - void SetLabel(const char* label); - - void BENCHMARK_ALWAYS_INLINE SetLabel(const std::string& str) { - this->SetLabel(str.c_str()); - } + void SetLabel(const std::string& label); // Range arguments for this run. CHECKs if the argument has been set. BENCHMARK_ALWAYS_INLINE @@ -1249,8 +1245,8 @@ TimeUnit GetTimeUnit() const; protected: - explicit Benchmark(const char* name); - void SetName(const char* name); + explicit Benchmark(const std::string& name); + void SetName(const std::string& name); public: const char* GetName() const; @@ -1305,12 +1301,12 @@ // the specified functor 'fn'. // // RETURNS: A pointer to the registered benchmark. -internal::Benchmark* RegisterBenchmark(const char* name, +internal::Benchmark* RegisterBenchmark(const std::string& name, internal::Function* fn); #if defined(BENCHMARK_HAS_CXX11) template <class Lambda> -internal::Benchmark* RegisterBenchmark(const char* name, Lambda&& fn); +internal::Benchmark* RegisterBenchmark(const std::string& name, Lambda&& fn); #endif // Remove all registered benchmarks. All pointers to previously registered @@ -1322,7 +1318,7 @@ // (ie those created using the BENCHMARK(...) macros. class BENCHMARK_EXPORT FunctionBenchmark : public Benchmark { public: - FunctionBenchmark(const char* name, Function* func) + FunctionBenchmark(const std::string& name, Function* func) : Benchmark(name), func_(func) {} void Run(State& st) BENCHMARK_OVERRIDE; @@ -1339,20 +1335,20 @@ private: template <class OLambda> - LambdaBenchmark(const char* name, OLambda&& lam) + LambdaBenchmark(const std::string& name, OLambda&& lam) : Benchmark(name), lambda_(std::forward<OLambda>(lam)) {} LambdaBenchmark(LambdaBenchmark const&) = delete; template <class Lam> // NOLINTNEXTLINE(readability-redundant-declaration) - friend Benchmark* ::benchmark::RegisterBenchmark(const char*, Lam&&); + friend Benchmark* ::benchmark::RegisterBenchmark(const std::string&, Lam&&); Lambda lambda_; }; #endif } // namespace internal -inline internal::Benchmark* RegisterBenchmark(const char* name, +inline internal::Benchmark* RegisterBenchmark(const std::string& name, internal::Function* fn) { return internal::RegisterBenchmarkInternal( ::new internal::FunctionBenchmark(name, fn)); @@ -1360,7 +1356,7 @@ #ifdef BENCHMARK_HAS_CXX11 template <class Lambda> -internal::Benchmark* RegisterBenchmark(const char* name, Lambda&& fn) { +internal::Benchmark* RegisterBenchmark(const std::string& name, Lambda&& fn) { using BenchType = internal::LambdaBenchmark<typename std::decay<Lambda>::type>; return internal::RegisterBenchmarkInternal( @@ -1371,7 +1367,7 @@ #if defined(BENCHMARK_HAS_CXX11) && \ (!defined(BENCHMARK_GCC_VERSION) || BENCHMARK_GCC_VERSION >= 409) template <class Lambda, class... Args> -internal::Benchmark* RegisterBenchmark(const char* name, Lambda&& fn, +internal::Benchmark* RegisterBenchmark(const std::string& name, Lambda&& fn, Args&&... args) { return benchmark::RegisterBenchmark( name, [=](benchmark::State& st) { fn(st, args...); });
diff --git a/src/benchmark.cc b/src/benchmark.cc index 1408840..f1633b7 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc
@@ -273,7 +273,7 @@ timer_->SetIterationTime(seconds); } -void State::SetLabel(const char* label) { +void State::SetLabel(const std::string& label) { MutexLock l(manager_->GetBenchmarkMutex()); manager_->results.report_label_ = label; }
diff --git a/src/benchmark_register.cc b/src/benchmark_register.cc index 4503dd1..e447c9a 100644 --- a/src/benchmark_register.cc +++ b/src/benchmark_register.cc
@@ -204,7 +204,7 @@ // Benchmark //=============================================================================// -Benchmark::Benchmark(const char* name) +Benchmark::Benchmark(const std::string& name) : name_(name), aggregation_report_mode_(ARM_Unspecified), time_unit_(GetDefaultTimeUnit()), @@ -230,7 +230,7 @@ Benchmark::~Benchmark() {} Benchmark* Benchmark::Name(const std::string& name) { - SetName(name.c_str()); + SetName(name); return this; } @@ -468,7 +468,7 @@ return this; } -void Benchmark::SetName(const char* name) { name_ = name; } +void Benchmark::SetName(const std::string& name) { name_ = name; } const char* Benchmark::GetName() const { return name_.c_str(); }
diff --git a/src/benchmark_runner.cc b/src/benchmark_runner.cc index 62383ea..f7ae424 100644 --- a/src/benchmark_runner.cc +++ b/src/benchmark_runner.cc
@@ -177,11 +177,10 @@ } if (value.back() == 'x') { - const char* iters_str = value.c_str(); char* p_end; // Reset errno before it's changed by strtol. errno = 0; - IterationCount num_iters = std::strtol(iters_str, &p_end, 10); + IterationCount num_iters = std::strtol(value.c_str(), &p_end, 10); // After a valid parse, p_end should have been set to // point to the 'x' suffix. @@ -194,7 +193,6 @@ return ret; } - const char* time_str = value.c_str(); bool has_suffix = value.back() == 's'; if (!has_suffix) { BM_VLOG(0) << "Value passed to --benchmark_min_time should have a suffix. " @@ -204,7 +202,7 @@ char* p_end; // Reset errno before it's changed by strtod. errno = 0; - double min_time = std::strtod(time_str, &p_end); + double min_time = std::strtod(value.c_str(), &p_end); // After a successful parse, p_end should point to the suffix 's', // or the end of the string if the suffix was omitted.
diff --git a/src/json_reporter.cc b/src/json_reporter.cc index 36efbf0..6559dfd 100644 --- a/src/json_reporter.cc +++ b/src/json_reporter.cc
@@ -297,7 +297,8 @@ out << ",\n" << indent << FormatKV("max_bytes_used", memory_result.max_bytes_used); - auto report_if_present = [&out, &indent](const char* label, int64_t val) { + auto report_if_present = [&out, &indent](const std::string& label, + int64_t val) { if (val != MemoryManager::TombstoneValue) out << ",\n" << indent << FormatKV(label, val); };
diff --git a/test/output_test.h b/test/output_test.h index c6ff8ef..c08fe1d 100644 --- a/test/output_test.h +++ b/test/output_test.h
@@ -85,7 +85,7 @@ struct Results; typedef std::function<void(Results const&)> ResultsCheckFn; -size_t AddChecker(const char* bm_name_pattern, const ResultsCheckFn& fn); +size_t AddChecker(const std::string& bm_name_pattern, const ResultsCheckFn& fn); // Class holding the results of a benchmark. // It is passed in calls to checker functions. @@ -117,7 +117,7 @@ // get the string for a result by name, or nullptr if the name // is not found - const std::string* Get(const char* entry_name) const { + const std::string* Get(const std::string& entry_name) const { auto it = values.find(entry_name); if (it == values.end()) return nullptr; return &it->second; @@ -126,12 +126,12 @@ // get a result by name, parsed as a specific type. // NOTE: for counters, use GetCounterAs instead. template <class T> - T GetAs(const char* entry_name) const; + T GetAs(const std::string& entry_name) const; // counters are written as doubles, so they have to be read first // as a double, and only then converted to the asked type. template <class T> - T GetCounterAs(const char* entry_name) const { + T GetCounterAs(const std::string& entry_name) const { double dval = GetAs<double>(entry_name); T tval = static_cast<T>(dval); return tval; @@ -139,7 +139,7 @@ }; template <class T> -T Results::GetAs(const char* entry_name) const { +T Results::GetAs(const std::string& entry_name) const { auto* sv = Get(entry_name); BM_CHECK(sv != nullptr && !sv->empty()); std::stringstream ss;
diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 986c4ad..241af5c 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc
@@ -299,7 +299,7 @@ } // end namespace internal -size_t AddChecker(const char* bm_name, const ResultsCheckFn& fn) { +size_t AddChecker(const std::string& bm_name, const ResultsCheckFn& fn) { auto& rc = internal::GetResultsChecker(); rc.Add(bm_name, fn); return rc.results.size(); @@ -394,14 +394,14 @@ benchmark::JSONReporter JR; benchmark::CSVReporter CSVR; struct ReporterTest { - const char* name; + std::string name; std::vector<TestCase>& output_cases; std::vector<TestCase>& error_cases; benchmark::BenchmarkReporter& reporter; std::stringstream out_stream; std::stringstream err_stream; - ReporterTest(const char* n, std::vector<TestCase>& out_tc, + ReporterTest(const std::string& n, std::vector<TestCase>& out_tc, std::vector<TestCase>& err_tc, benchmark::BenchmarkReporter& br) : name(n), output_cases(out_tc), error_cases(err_tc), reporter(br) { @@ -409,12 +409,12 @@ reporter.SetErrorStream(&err_stream); } } TestCases[] = { - {"ConsoleReporter", GetTestCaseList(TC_ConsoleOut), + {std::string("ConsoleReporter"), GetTestCaseList(TC_ConsoleOut), GetTestCaseList(TC_ConsoleErr), CR}, - {"JSONReporter", GetTestCaseList(TC_JSONOut), GetTestCaseList(TC_JSONErr), - JR}, - {"CSVReporter", GetTestCaseList(TC_CSVOut), GetTestCaseList(TC_CSVErr), - CSVR}, + {std::string("JSONReporter"), GetTestCaseList(TC_JSONOut), + GetTestCaseList(TC_JSONErr), JR}, + {std::string("CSVReporter"), GetTestCaseList(TC_CSVOut), + GetTestCaseList(TC_CSVErr), CSVR}, }; // Create the test reporter and run the benchmarks. @@ -423,7 +423,8 @@ benchmark::RunSpecifiedBenchmarks(&test_rep); for (auto& rep_test : TestCases) { - std::string msg = std::string("\nTesting ") + rep_test.name + " Output\n"; + std::string msg = + std::string("\nTesting ") + rep_test.name + std::string(" Output\n"); std::string banner(msg.size() - 1, '-'); std::cout << banner << msg << banner << "\n"; @@ -440,7 +441,7 @@ // the checks to subscribees. auto& csv = TestCases[2]; // would use == but gcc spits a warning - BM_CHECK(std::strcmp(csv.name, "CSVReporter") == 0); + BM_CHECK(csv.name == std::string("CSVReporter")); internal::GetResultsChecker().CheckResults(csv.out_stream); }
diff --git a/test/register_benchmark_test.cc b/test/register_benchmark_test.cc index 240c8c2..d69d144 100644 --- a/test/register_benchmark_test.cc +++ b/test/register_benchmark_test.cc
@@ -19,11 +19,11 @@ }; struct TestCase { - std::string name; - const char* label; + const std::string name; + const std::string label; // Note: not explicit as we rely on it being converted through ADD_CASES. - TestCase(const char* xname) : TestCase(xname, nullptr) {} - TestCase(const char* xname, const char* xlabel) + TestCase(const std::string& xname) : TestCase(xname, "") {} + TestCase(const std::string& xname, const std::string& xlabel) : name(xname), label(xlabel) {} typedef benchmark::BenchmarkReporter::Run Run; @@ -32,7 +32,7 @@ // clang-format off BM_CHECK(name == run.benchmark_name()) << "expected " << name << " got " << run.benchmark_name(); - if (label) { + if (!label.empty()) { BM_CHECK(run.report_label == label) << "expected " << label << " got " << run.report_label; } else { @@ -123,7 +123,7 @@ { CustomFixture fx; benchmark::RegisterBenchmark("custom_fixture", fx); - AddCases({"custom_fixture"}); + AddCases({std::string("custom_fixture")}); } #endif #ifndef BENCHMARK_HAS_NO_VARIADIC_REGISTER_BENCHMARK
diff --git a/test/skip_with_error_test.cc b/test/skip_with_error_test.cc index 2dd222a..b4c5e15 100644 --- a/test/skip_with_error_test.cc +++ b/test/skip_with_error_test.cc
@@ -48,7 +48,8 @@ std::vector<TestCase> ExpectedResults; -int AddCases(const char* base_name, std::initializer_list<TestCase> const& v) { +int AddCases(const std::string& base_name, + std::initializer_list<TestCase> const& v) { for (auto TC : v) { TC.name = base_name + TC.name; ExpectedResults.push_back(std::move(TC));