recursive variant move construction/assignment complexity

include/mapbox/recursive_wrapper.hpp

@@ -32,18 +32,6 @@ class recursive_wrapper
 public:
     using type = T;
 
-    /**
-     * Default constructor default initializes the internally stored value.
-     * For POD types this means nothing is done and the storage is
-     * uninitialized.
-     *
-     * @throws std::bad_alloc if there is insufficient memory for an object
-     *         of type T.
-     * @throws any exception thrown by the default constructur of T.
-     */
-    recursive_wrapper()
-        : p_(new T){}
-
     ~recursive_wrapper() noexcept { delete p_; }
 
     recursive_wrapper(recursive_wrapper const& operand)
@@ -52,8 +40,11 @@ class recursive_wrapper
     recursive_wrapper(T const& operand)
         : p_(new T(operand)) {}
 
-    recursive_wrapper(recursive_wrapper&& operand)
-        : p_(new T(std::move(operand.get()))) {}
+    recursive_wrapper(recursive_wrapper&& operand) noexcept
+        : p_(operand.p_)
+    {
+        operand.p_ = nullptr;
+    }
 
     recursive_wrapper(T&& operand)
         : p_(new T(std::move(operand))) {}

include/mapbox/variant.hpp

@@ -235,6 +235,7 @@ struct variant_helper<T, Types...>
         if (old_type_index == sizeof...(Types))
         {
             new (new_value) T(std::move(*reinterpret_cast<T*>(old_value)));
+            reinterpret_cast<T*>(old_value)->~T();
         }
         else
         {
@@ -613,6 +614,7 @@ class variant
         : type_index(old.type_index)
     {
         helper_type::move(old.type_index, &old.data, &data);
+        old.type_index = detail::invalid_value;
     }
 
 private:
@@ -630,6 +632,7 @@ class variant
         type_index = detail::invalid_value;
         helper_type::move(rhs.type_index, &rhs.data, &data);
         type_index = rhs.type_index;
+        rhs.type_index = detail::invalid_value;
     }
 
 public:

test/recursive_wrapper_test.cpp

@@ -92,6 +92,30 @@ struct to_string
     }
 };
 
+void bench_large_expression(std::size_t num)
+{
+    std::cerr << "----- sum of " << num << " ones -----" << std::endl;
+    expression sum = 0;
+    {
+        std::cerr << "construction ";
+        auto_cpu_timer t;
+        for (std::size_t i = 0; i < num; ++i)
+        {
+            sum = binary_op<add>(std::move(sum), 1);
+        }
+    }
+    int total = 0;
+    {
+        std::cerr << "calculation ";
+        auto_cpu_timer t;
+        for (std::size_t i = 0; i < num; ++i)
+        {
+            total += util::apply_visitor(calculator(), sum);
+        }
+    }
+    std::cerr << "total=" << total << std::endl;
+}
+
 } // namespace test
 
 int main(int argc, char** argv)
@@ -121,5 +145,8 @@ int main(int argc, char** argv)
 
     std::cerr << util::apply_visitor(test::to_string(), result) << "=" << util::apply_visitor(test::calculator(), result) << std::endl;
 
+    test::bench_large_expression(1000);
+    test::bench_large_expression(5000);
+
     return EXIT_SUCCESS;
 }

test/t/recursive_wrapper.cpp

@@ -27,7 +27,7 @@ TEST_CASE("recursive wrapper of int")
         rwi b{a};
         REQUIRE(b.get() == 8);
 
-        rwi c;
+        rwi c{-1};
         c = b;
         REQUIRE(b.get() == 8);
         REQUIRE(c.get() == 8);
@@ -114,13 +114,6 @@ TEST_CASE("swap")
 TEST_CASE("recursive wrapper of pair<int, int>")
 {
 
-    SECTION("default constructed")
-    {
-        rwp a;
-        REQUIRE(a.get().first == 0);
-        REQUIRE(a.get().second == 0);
-    }
-
     SECTION("construct with value")
     {
         rwp a{std::make_pair(1, 2)};
@@ -139,7 +132,7 @@ TEST_CASE("recursive wrapper of pair<int, int>")
         REQUIRE(b.get().first == 3);
         REQUIRE(b.get().second == 4);
 
-        rwp c;
+        rwp c{{-1, -2}};
         c = b;
         REQUIRE(b.get().first == 3);
         REQUIRE(b.get().second == 4);

test/t/variant.cpp

@@ -29,7 +29,6 @@ TEST_CASE("variant can be moved into vector", "[variant]")
     variant_type v(std::string("test"));
     std::vector<variant_type> vec;
     vec.emplace_back(std::move(v));
-    REQUIRE(v.get<std::string>() != std::string("test"));
     REQUIRE(vec.at(0).get<std::string>() == std::string("test"));
 }
 

test/unique_ptr_test.cpp

@@ -93,6 +93,36 @@ struct to_string
     }
 };
 
+template <typename Op, typename Node = binary_op<Op>>
+std::unique_ptr<Node> make_binary(expression&& lhs, expression&& rhs)
+{
+    return std::unique_ptr<Node>(new Node(std::move(lhs), std::move(rhs)));
+}
+
+void bench_large_expression(std::size_t num)
+{
+    std::cerr << "----- sum of " << num << " ones -----" << std::endl;
+    expression sum = 0;
+    {
+        std::cerr << "construction ";
+        auto_cpu_timer t;
+        for (std::size_t i = 0; i < num; ++i)
+        {
+            sum = make_binary<add>(std::move(sum), 1);
+        }
+    }
+    int total = 0;
+    {
+        std::cerr << "calculation ";
+        auto_cpu_timer t;
+        for (std::size_t i = 0; i < num; ++i)
+        {
+            total += util::apply_visitor(calculator(), sum);
+        }
+    }
+    std::cerr << "total=" << total << std::endl;
+}
+
 } // namespace test
 
 int main(int argc, char** argv)
@@ -105,8 +135,8 @@ int main(int argc, char** argv)
 
     const std::size_t NUM_ITER = static_cast<std::size_t>(std::stol(argv[1]));
 
-    test::expression sum(std::unique_ptr<test::binary_op<test::add>>(new test::binary_op<test::add>(2, 3)));
-    test::expression result(std::unique_ptr<test::binary_op<test::sub>>(new test::binary_op<test::sub>(std::move(sum), 4)));
+    test::expression sum = test::make_binary<test::add>(2, 3);
+    test::expression result = test::make_binary<test::sub>(std::move(sum), 4);
 
     std::cerr << "TYPE OF RESULT-> " << util::apply_visitor(test::test(), result) << std::endl;
 
@@ -122,5 +152,8 @@ int main(int argc, char** argv)
 
     std::cerr << util::apply_visitor(test::to_string(), result) << "=" << util::apply_visitor(test::calculator(), result) << std::endl;
 
+    test::bench_large_expression(1000);
+    test::bench_large_expression(5000);
+
     return EXIT_SUCCESS;
 }