[workspace] Patch googlebenchmark to avoid shadow warnings

This allows us to remove copy-pasta in every benchmark we write.
RobotLocomotion · Jul 16, 2023 · c01a517 · c01a517
1 parent 4aa538d
commit c01a517
Show file tree

Hide file tree

Showing 8 changed files with 25 additions and 16 deletions.
diff --git a/geometry/benchmarking/render_benchmark.cc b/geometry/benchmarking/render_benchmark.cc
@@ -84,8 +84,7 @@ class RenderBenchmark : public benchmark::Fixture {
     material_.AddProperty("label", "id", RenderLabel::kDontCare);
   }
 
-  using benchmark::Fixture::SetUp;
-  void SetUp(const ::benchmark::State&) { depth_cameras_.clear(); }
+  void SetUp(::benchmark::State&) { depth_cameras_.clear(); }
 
   template <EngineType engine_type>
   // NOLINTNEXTLINE(runtime/references)

diff --git a/multibody/benchmarking/acrobot.cc b/multibody/benchmarking/acrobot.cc
@@ -49,7 +49,6 @@ class FixtureBase : public benchmark::Fixture {
 template <typename T>
 class AcrobotFixture : public FixtureBase<T> {
  public:
-  using benchmark::Fixture::SetUp;
   void SetUp(benchmark::State&) override {
     plant_ = std::make_unique<AcrobotPlant<T>>();
     this->Populate(*plant_);
@@ -81,7 +80,6 @@ BENCHMARK_F(AcrobotFixtureAdx, AcrobotAdxMassMatrix)(benchmark::State& state) {
 template <typename T>
 class MultibodyFixture : public FixtureBase<T> {
  public:
-  using benchmark::Fixture::SetUp;
   void SetUp(benchmark::State&) override {
     auto double_plant = multibody::benchmarks::acrobot::MakeAcrobotPlant(
             multibody::benchmarks::acrobot::AcrobotParameters(), true);

diff --git a/multibody/benchmarking/cassie.cc b/multibody/benchmarking/cassie.cc
@@ -44,10 +44,6 @@ class Cassie : public benchmark::Fixture {
     tools::performance::AddMinMaxStatistics(this);
   }
 
-  // This apparently futile using statement works around "overloaded virtual"
-  // errors in g++. All of this is a consequence of the weird deprecation of
-  // const-ref State versions of SetUp() and TearDown() in benchmark.h.
-  using benchmark::Fixture::SetUp;
   void SetUp(BenchmarkStateRef state) override {
     SetUpNonZeroState();
     SetUpGradientsOrVariables(state);

diff --git a/multibody/benchmarking/position_constraint.cc b/multibody/benchmarking/position_constraint.cc
@@ -23,8 +23,7 @@ using systems::Context;
 
 class IiwaPositionConstraintFixture : public benchmark::Fixture {
  public:
-  using benchmark::Fixture::SetUp;
-  void SetUp(const ::benchmark::State&) override {
+  void SetUp(::benchmark::State&) override {
     tools::performance::AddMinMaxStatistics(this);
 
     const int kNumIiwas = 10;

diff --git a/systems/benchmarking/framework_benchmarks.cc b/systems/benchmarking/framework_benchmarks.cc
@@ -15,10 +15,6 @@ class BasicFixture : public benchmark::Fixture {
  public:
   BasicFixture() { tools::performance::AddMinMaxStatistics(this); }
 
-  // This apparently futile using statement works around "overloaded virtual"
-  // errors in g++. All of this is a consequence of the weird deprecation of
-  // const-ref State versions of SetUp() and TearDown() in benchmark.h.
-  using benchmark::Fixture::SetUp;
   void SetUp(benchmark::State&) override {
     builder_ = std::make_unique<DiagramBuilder<double>>();
   }

diff --git a/systems/benchmarking/multilayer_perceptron_benchmark.cc b/systems/benchmarking/multilayer_perceptron_benchmark.cc
@@ -24,8 +24,7 @@ class Mlp : public benchmark::Fixture {
     this->Unit(benchmark::kMicrosecond);
   }
 
-  using benchmark::Fixture::SetUp;
-  void SetUp(const benchmark::State& state) {
+  void SetUp(benchmark::State& state) {  // NOLINT(runtime/references)
     // Number of inputs.
     const int num_inputs = state.range(0);
     DRAKE_DEMAND(num_inputs >= 1);

diff --git a/tools/workspace/googlebenchmark/patches/remove_overloaded_fixture_set_up.patch b/tools/workspace/googlebenchmark/patches/remove_overloaded_fixture_set_up.patch
@@ -0,0 +1,21 @@
+Avoid GCC warnings about overloaded virtuals
+
+We only need the mutable spelling for Drake code.
+
+--- include/benchmark/benchmark.h
++++ include/benchmark/benchmark.h
+@@ -1425,12 +1425,8 @@
+     this->TearDown(st);
+   }
+
+-  // These will be deprecated ...
+-  virtual void SetUp(const State&) {}
+-  virtual void TearDown(const State&) {}
+-  // ... In favor of these.
+-  virtual void SetUp(State& st) { SetUp(const_cast<const State&>(st)); }
+-  virtual void TearDown(State& st) { TearDown(const_cast<const State&>(st)); }
++  virtual void SetUp(State&) {}
++  virtual void TearDown(State&) {}
+
+  protected:
+   virtual void BenchmarkCase(State&) = 0;
diff --git a/tools/workspace/googlebenchmark/repository.bzl b/tools/workspace/googlebenchmark/repository.bzl
@@ -11,6 +11,7 @@ def googlebenchmark_repository(
         mirrors = mirrors,
         patches = [
             ":patches/console_allocs.patch",
+            ":patches/remove_overloaded_fixture_set_up.patch",
             ":patches/string_precision.patch",
         ],
     )