From 0fbf4f3e152e8fe9a41fa1a960ea3dac3cb76e59 Mon Sep 17 00:00:00 2001
From: Joe Burzinski <tridacnid@gmail.com>
Date: Mon, 18 Nov 2019 22:22:38 -0600
Subject: [PATCH] Fix wrong namespacing of benchmarking constructor helpers

---
 docs/benchmarks.md                            | 12 +--
 src/catch2/benchmark/catch_constructor.hpp    | 92 ++++++++++---------
 tests/SelfTest/UsageTests/Benchmark.tests.cpp | 15 +++
 3 files changed, 67 insertions(+), 52 deletions(-)

diff --git a/docs/benchmarks.md b/docs/benchmarks.md
index 6513cc2d..ccc4175b 100644
--- a/docs/benchmarks.md
+++ b/docs/benchmarks.md
@@ -164,7 +164,7 @@ Note that it is not possible to simply use the same instance for different runs
 and resetting it between each run since that would pollute the measurements with
 the resetting code.
 
-It is also possible to just provide an argument name to the simple `BENCHMARK` macro to get 
+It is also possible to just provide an argument name to the simple `BENCHMARK` macro to get
 the same semantics as providing a callable to `meter.measure` with `int` argument:
 
 ```c++
@@ -185,19 +185,17 @@ construct and destroy objects without dynamic allocation and in a way that lets
 you measure construction and destruction separately.
 
 ```c++
-BENCHMARK_ADVANCED("construct")(Catch::Benchmark::Chronometer meter)
-{
+BENCHMARK_ADVANCED("construct")(Catch::Benchmark::Chronometer meter) {
     std::vector<Catch::Benchmark::storage_for<std::string>> storage(meter.runs());
     meter.measure([&](int i) { storage[i].construct("thing"); });
-})
+};
 
-BENCHMARK_ADVANCED("destroy", [](Catch::Benchmark::Chronometer meter)
-{
+BENCHMARK_ADVANCED("destroy")(Catch::Benchmark::Chronometer meter) {
     std::vector<Catch::Benchmark::destructable_object<std::string>> storage(meter.runs());
     for(auto&& o : storage)
         o.construct("thing");
     meter.measure([&](int i) { storage[i].destruct(); });
-})
+};
 ```
 
 `Catch::Benchmark::storage_for<T>` objects are just pieces of raw storage suitable for `T`
diff --git a/src/catch2/benchmark/catch_constructor.hpp b/src/catch2/benchmark/catch_constructor.hpp
index bf6dfec9..f12958f3 100644
--- a/src/catch2/benchmark/catch_constructor.hpp
+++ b/src/catch2/benchmark/catch_constructor.hpp
@@ -14,60 +14,62 @@
 #include <type_traits>
 
 namespace Catch {
-    namespace Detail {
-        template <typename T, bool Destruct>
-        struct ObjectStorage
-        {
-            using TStorage = typename std::aligned_storage<sizeof(T), std::alignment_of<T>::value>::type;
-
-            ObjectStorage() : data() {}
-
-            ObjectStorage(const ObjectStorage& other)
+    namespace Benchmark {
+        namespace Detail {
+            template <typename T, bool Destruct>
+            struct ObjectStorage
             {
-                new(&data) T(other.stored_object());
-            }
+                using TStorage = typename std::aligned_storage<sizeof(T), std::alignment_of<T>::value>::type;
 
-            ObjectStorage(ObjectStorage&& other)
-            {
-                new(&data) T(std::move(other.stored_object()));
-            }
+                ObjectStorage() : data() {}
 
-            ~ObjectStorage() { destruct_on_exit<T>(); }
+                ObjectStorage(const ObjectStorage& other)
+                {
+                    new(&data) T(other.stored_object());
+                }
 
-            template <typename... Args>
-            void construct(Args&&... args)
-            {
-                new (&data) T(std::forward<Args>(args)...);
-            }
+                ObjectStorage(ObjectStorage&& other)
+                {
+                    new(&data) T(std::move(other.stored_object()));
+                }
 
-            template <bool AllowManualDestruction = !Destruct>
-            typename std::enable_if<AllowManualDestruction>::type destruct()
-            {
-                stored_object().~T();
-            }
+                ~ObjectStorage() { destruct_on_exit<T>(); }
 
-        private:
-            // If this is a constructor benchmark, destruct the underlying object
-            template <typename U>
-            void destruct_on_exit(typename std::enable_if<Destruct, U>::type* = 0) { destruct<true>(); }
-            // Otherwise, don't
-            template <typename U>
-            void destruct_on_exit(typename std::enable_if<!Destruct, U>::type* = 0) { }
+                template <typename... Args>
+                void construct(Args&&... args)
+                {
+                    new (&data) T(std::forward<Args>(args)...);
+                }
 
-            T& stored_object()
-            {
-                return *static_cast<T*>(static_cast<void*>(&data));
-            }
+                template <bool AllowManualDestruction = !Destruct>
+                typename std::enable_if<AllowManualDestruction>::type destruct()
+                {
+                    stored_object().~T();
+                }
 
-            TStorage data;
-        };
-    }
+            private:
+                // If this is a constructor benchmark, destruct the underlying object
+                template <typename U>
+                void destruct_on_exit(typename std::enable_if<Destruct, U>::type* = 0) { destruct<true>(); }
+                // Otherwise, don't
+                template <typename U>
+                void destruct_on_exit(typename std::enable_if<!Destruct, U>::type* = 0) { }
 
-    template <typename T>
-    using storage_for = Detail::ObjectStorage<T, true>;
+                T& stored_object()
+                {
+                    return *static_cast<T*>(static_cast<void*>(&data));
+                }
 
-    template <typename T>
-    using destructable_object = Detail::ObjectStorage<T, false>;
-}
+                TStorage data;
+            };
+        } // namespace Detail
+
+        template <typename T>
+        using storage_for = Detail::ObjectStorage<T, true>;
+
+        template <typename T>
+        using destructable_object = Detail::ObjectStorage<T, false>;
+    } // namespace Benchmark
+} // namespace Catch
 
 #endif // TWOBLUECUBES_CATCH_CONSTRUCTOR_HPP_INCLUDED
diff --git a/tests/SelfTest/UsageTests/Benchmark.tests.cpp b/tests/SelfTest/UsageTests/Benchmark.tests.cpp
index fe2d736c..38ad6a9f 100644
--- a/tests/SelfTest/UsageTests/Benchmark.tests.cpp
+++ b/tests/SelfTest/UsageTests/Benchmark.tests.cpp
@@ -1,5 +1,6 @@
 #include <catch2/catch_test_macros.hpp>
 #include <catch2/benchmark/catch_benchmark.hpp>
+#include <catch2/benchmark/catch_constructor.hpp>
 #include <catch2/catch_generators_specific.hpp>
 
 #include <map>
@@ -127,4 +128,18 @@ TEST_CASE("Benchmark containers", "[!benchmark]") {
             REQUIRE(v[i] == generated);
         }
     }
+
+    SECTION("construct and destroy example") {
+        BENCHMARK_ADVANCED("construct")(Catch::Benchmark::Chronometer meter) {
+            std::vector<Catch::Benchmark::storage_for<std::string>> storage(meter.runs());
+            meter.measure([&](int i) { storage[i].construct("thing"); });
+        };
+
+        BENCHMARK_ADVANCED("destroy")(Catch::Benchmark::Chronometer meter) {
+            std::vector<Catch::Benchmark::destructable_object<std::string>> storage(meter.runs());
+            for(auto&& o : storage)
+                o.construct("thing");
+            meter.measure([&](int i) { storage[i].destruct(); });
+        };
+    }
 }