Make State constructor private. (#650)

The State constructor should not be part of the public API. Adding a
utility method to BenchmarkInstance allows us to avoid leaking the
RunInThread method into the public API.

include/benchmark/benchmark.h

@@ -430,6 +430,7 @@ struct Statistics {
 };
 
 namespace internal {
+struct BenchmarkInstance;
 class ThreadTimer;
 class ThreadManager;
 
@@ -664,20 +665,20 @@ class State {
   // Number of threads concurrently executing the benchmark.
   const int threads;
 
-  // TODO(EricWF) make me private
+ private:
   State(size_t max_iters, const std::vector<int64_t>& ranges, int thread_i,
         int n_threads, internal::ThreadTimer* timer,
         internal::ThreadManager* manager);
 
- private:
   void StartKeepRunning();
   // Implementation of KeepRunning() and KeepRunningBatch().
   // is_batch must be true unless n is 1.
   bool KeepRunningInternal(size_t n, bool is_batch);
   void FinishKeepRunning();
   internal::ThreadTimer* timer_;
   internal::ThreadManager* manager_;
-  BENCHMARK_DISALLOW_COPY_AND_ASSIGN(State);
+
+  friend struct internal::BenchmarkInstance;
 };
 
 inline BENCHMARK_ALWAYS_INLINE bool State::KeepRunning() {
@@ -931,9 +932,6 @@ class Benchmark {
 
   virtual void Run(State& state) = 0;
 
-  // Used inside the benchmark implementation
-  struct Instance;
-
  protected:
   explicit Benchmark(const char* name);
   Benchmark(Benchmark const&);

src/benchmark.cc

@@ -126,7 +126,7 @@ void UseCharPointer(char const volatile*) {}
 namespace {
 
 BenchmarkReporter::Run CreateRunReport(
-    const benchmark::internal::Benchmark::Instance& b,
+    const benchmark::internal::BenchmarkInstance& b,
     const internal::ThreadManager::Result& results, size_t memory_iterations,
     const MemoryManager::Result& memory_result, double seconds) {
   // Create report about this benchmark run.
@@ -169,12 +169,10 @@ BenchmarkReporter::Run CreateRunReport(
 
 // Execute one thread of benchmark b for the specified number of iterations.
 // Adds the stats collected for the thread into *total.
-void RunInThread(const benchmark::internal::Benchmark::Instance* b,
-                 size_t iters, int thread_id,
-                 internal::ThreadManager* manager) {
+void RunInThread(const BenchmarkInstance* b, size_t iters, int thread_id,
+                 ThreadManager* manager) {
   internal::ThreadTimer timer;
-  State st(iters, b->arg, thread_id, b->threads, &timer, manager);
-  b->benchmark->Run(st);
+  State st = b->Run(iters, thread_id, &timer, manager);
   CHECK(st.iterations() >= st.max_iterations)
       << "Benchmark returned before State::KeepRunning() returned false!";
   {
@@ -199,7 +197,7 @@ struct RunResults {
 };
 
 RunResults RunBenchmark(
-    const benchmark::internal::Benchmark::Instance& b,
+    const benchmark::internal::BenchmarkInstance& b,
     std::vector<BenchmarkReporter::Run>* complexity_reports) {
   RunResults run_results;
 
@@ -437,7 +435,7 @@ void State::FinishKeepRunning() {
 namespace internal {
 namespace {
 
-void RunBenchmarks(const std::vector<Benchmark::Instance>& benchmarks,
+void RunBenchmarks(const std::vector<BenchmarkInstance>& benchmarks,
                    BenchmarkReporter* display_reporter,
                    BenchmarkReporter* file_reporter) {
   // Note the file_reporter can be null.
@@ -447,7 +445,7 @@ void RunBenchmarks(const std::vector<Benchmark::Instance>& benchmarks,
   bool has_repetitions = FLAGS_benchmark_repetitions > 1;
   size_t name_field_width = 10;
   size_t stat_field_width = 0;
-  for (const Benchmark::Instance& benchmark : benchmarks) {
+  for (const BenchmarkInstance& benchmark : benchmarks) {
     name_field_width =
         std::max<size_t>(name_field_width, benchmark.name.size());
     has_repetitions |= benchmark.repetitions > 1;
@@ -595,7 +593,7 @@ size_t RunSpecifiedBenchmarks(BenchmarkReporter* display_reporter,
     file_reporter->SetErrorStream(&output_file);
   }
 
-  std::vector<internal::Benchmark::Instance> benchmarks;
+  std::vector<internal::BenchmarkInstance> benchmarks;
   if (!FindBenchmarksInternal(spec, &benchmarks, &Err)) return 0;
 
   if (benchmarks.empty()) {

src/benchmark_api_internal.cc

@@ -0,0 +1,15 @@
+#include "benchmark_api_internal.h"
+
+namespace benchmark {
+namespace internal {
+
+State BenchmarkInstance::Run(
+    size_t iters, int thread_id, internal::ThreadTimer* timer,
+    internal::ThreadManager* manager) const {
+  State st(iters, arg, thread_id, threads, timer, manager);
+  benchmark->Run(st);
+  return st;
+}
+
+}  // internal
+}  // benchmark

src/benchmark_api_internal.h

@@ -6,14 +6,15 @@
 #include <cmath>
 #include <iosfwd>
 #include <limits>
+#include <memory>
 #include <string>
 #include <vector>
 
 namespace benchmark {
 namespace internal {
 
 // Information kept per benchmark we may want to run
-struct Benchmark::Instance {
+struct BenchmarkInstance {
   std::string name;
   Benchmark* benchmark;
   AggregationReportMode aggregation_report_mode;
@@ -31,10 +32,13 @@ struct Benchmark::Instance {
   double min_time;
   size_t iterations;
   int threads;  // Number of concurrent threads to us
+
+  State Run(size_t iters, int thread_id, internal::ThreadTimer* timer,
+            internal::ThreadManager* manager) const;
 };
 
 bool FindBenchmarksInternal(const std::string& re,
-                            std::vector<Benchmark::Instance>* benchmarks,
+                            std::vector<BenchmarkInstance>* benchmarks,
                             std::ostream* Err);
 
 bool IsZero(double n);

src/benchmark_register.cc

@@ -78,7 +78,7 @@ class BenchmarkFamilies {
   // Extract the list of benchmark instances that match the specified
   // regular expression.
   bool FindBenchmarks(std::string re,
-                      std::vector<Benchmark::Instance>* benchmarks,
+                      std::vector<BenchmarkInstance>* benchmarks,
                       std::ostream* Err);
 
  private:
@@ -107,7 +107,7 @@ void BenchmarkFamilies::ClearBenchmarks() {
 }
 
 bool BenchmarkFamilies::FindBenchmarks(
-    std::string spec, std::vector<Benchmark::Instance>* benchmarks,
+    std::string spec, std::vector<BenchmarkInstance>* benchmarks,
     std::ostream* ErrStream) {
   CHECK(ErrStream);
   auto& Err = *ErrStream;
@@ -152,7 +152,7 @@ bool BenchmarkFamilies::FindBenchmarks(
 
     for (auto const& args : family->args_) {
       for (int num_threads : *thread_counts) {
-        Benchmark::Instance instance;
+        BenchmarkInstance instance;
         instance.name = family->name_;
         instance.benchmark = family.get();
         instance.aggregation_report_mode = family->aggregation_report_mode_;
@@ -225,7 +225,7 @@ Benchmark* RegisterBenchmarkInternal(Benchmark* bench) {
 // FIXME: This function is a hack so that benchmark.cc can access
 // `BenchmarkFamilies`
 bool FindBenchmarksInternal(const std::string& re,
-                            std::vector<Benchmark::Instance>* benchmarks,
+                            std::vector<BenchmarkInstance>* benchmarks,
                             std::ostream* Err) {
   return BenchmarkFamilies::GetInstance()->FindBenchmarks(re, benchmarks, Err);
 }