Fix issues with immer::box when used in collections

immer/array_transient.hpp

@@ -184,11 +184,11 @@ class array_transient : MemoryPolicy::transience_t::owner
     IMMER_NODISCARD persistent_type persistent() &
     {
         this->owner_t::operator=(owner_t{});
-        return persistent_type{impl_};
+        return persistent_type(impl_);
     }
     IMMER_NODISCARD persistent_type persistent() &&
     {
-        return persistent_type{std::move(impl_)};
+        return persistent_type(std::move(impl_));
     }
 
 private:

immer/box.hpp

@@ -44,7 +44,7 @@ class box
 
         template <typename... Args>
         holder(Args&&... args)
-            : value{std::forward<Args>(args)...}
+            : value(std::forward<Args>(args)...)
         {
         }
     };
@@ -77,7 +77,10 @@ class box
      */
     template <typename Arg,
               typename Enable = std::enable_if_t<
-                  !std::is_same<box, std::decay_t<Arg>>::value>>
+                  !std::is_same<box, std::decay_t<Arg>>::value>,
+              // this is similar to std::is_constructible but works around the
+              // fact that is_constructible is ill-formed for incomplete types
+              typename = decltype(T(std::declval<Arg>()))>
     box(Arg&& arg)
         : impl_{detail::make<heap, holder>(std::forward<Arg>(arg))}
     {
@@ -86,7 +89,14 @@ class box
     /*!
      * Constructs a box holding `T{arg1, arg2, args...}`
      */
-    template <typename Arg1, typename Arg2, typename... Args>
+    template <typename Arg1,
+              typename Arg2,
+              typename... Args,
+              // this is similar to std::is_constructible but works around the
+              // fact that is_constructible is ill-formed for incomplete types
+              typename = decltype(T(std::declval<Arg1>(),
+                                    std::declval<Arg2>(),
+                                    std::declval<Args>()...))>
     box(Arg1&& arg1, Arg2&& arg2, Args&&... args)
         : impl_{detail::make<heap, holder>(std::forward<Arg1>(arg1),
                                            std::forward<Arg2>(arg2),

immer/detail/arrays/no_capacity.hpp

@@ -163,7 +163,7 @@ struct no_capacity
     {
         auto p = node_t::copy_n(size + 1, ptr, size);
         IMMER_TRY {
-            new (p->data() + size) T{std::move(value)};
+            new (p->data() + size) T(std::move(value));
             return {p, size + 1};
         }
         IMMER_CATCH (...) {

immer/detail/arrays/with_capacity.hpp

@@ -198,7 +198,7 @@ struct with_capacity
         auto cap = recommend_up(size + 1, capacity);
         auto p   = node_t::copy_n(cap, ptr, size);
         IMMER_TRY {
-            new (p->data() + size) T{std::move(value)};
+            new (p->data() + size) T(std::move(value));
             return {p, size + 1, cap};
         }
         IMMER_CATCH (...) {
@@ -210,13 +210,13 @@ struct with_capacity
     void push_back_mut(edit_t e, T value)
     {
         if (ptr->can_mutate(e) && capacity > size) {
-            new (data() + size) T{std::move(value)};
+            new (data() + size) T(std::move(value));
             ++size;
         } else {
             auto cap = recommend_up(size + 1, capacity);
             auto p   = node_t::copy_e(e, cap, ptr, size);
             IMMER_TRY {
-                new (p->data() + size) T{std::move(value)};
+                new (p->data() + size) T(std::move(value));
                 *this = {p, size + 1, cap};
             }
             IMMER_CATCH (...) {

immer/detail/hamts/node.hpp

@@ -713,7 +713,7 @@ struct node
                 detail::uninitialized_copy(
                     src->values(), src->values() + voffset, dst->values());
             IMMER_TRY {
-                new (dst->values() + voffset) T{std::move(value)};
+                new (dst->values() + voffset) T(std::move(value));
                 IMMER_TRY {
                     if (nv)
                         detail::uninitialized_copy(src->values() + voffset,
@@ -768,7 +768,7 @@ struct node
                         src->values(), src->values() + voffset, dst->values());
             }
             IMMER_TRY {
-                new (dst->values() + voffset) T{std::move(value)};
+                new (dst->values() + voffset) T(std::move(value));
                 IMMER_TRY {
                     if (nv) {
                         if (mutate_values)

immer/detail/rbts/node.hpp

@@ -467,7 +467,7 @@ struct node
         assert(n >= 1);
         auto p = make_leaf_n(n);
         IMMER_TRY {
-            new (p->leaf()) T{std::forward<U>(x)};
+            new (p->leaf()) T(std::forward<U>(x));
         }
         IMMER_CATCH (...) {
             heap::deallocate(node_t::sizeof_leaf_n(n), p);
@@ -481,7 +481,7 @@ struct node
     {
         auto p = make_leaf_e(e);
         IMMER_TRY {
-            new (p->leaf()) T{std::forward<U>(x)};
+            new (p->leaf()) T(std::forward<U>(x));
         }
         IMMER_CATCH (...) {
             heap::deallocate(node_t::max_sizeof_leaf, p);
@@ -790,7 +790,7 @@ struct node
     {
         auto dst = copy_leaf_n(n + 1, src, n);
         IMMER_TRY {
-            new (dst->leaf() + n) T{std::forward<U>(x)};
+            new (dst->leaf() + n) T(std::forward<U>(x));
         }
         IMMER_CATCH (...) {
             detail::destroy_n(dst->leaf(), n);

immer/detail/rbts/rbtree.hpp

@@ -274,7 +274,7 @@ struct rbtree
         auto ts       = size - tail_off;
         if (ts < branches<BL>) {
             ensure_mutable_tail(e, ts);
-            new (&tail->leaf()[ts]) T{std::move(value)};
+            new (&tail->leaf()[ts]) T(std::move(value));
         } else {
             auto new_tail = node_t::make_leaf_e(e, std::move(value));
             IMMER_TRY {

immer/detail/rbts/rrbtree.hpp

@@ -479,7 +479,7 @@ struct rrbtree
         auto ts = tail_size();
         if (ts < branches<BL>) {
             ensure_mutable_tail(e, ts);
-            new (&tail->leaf()[ts]) T{std::move(value)};
+            new (&tail->leaf()[ts]) T(std::move(value));
         } else {
             using std::get;
             auto new_tail = node_t::make_leaf_e(e, std::move(value));

immer/detail/util.hpp

@@ -174,7 +174,7 @@ T* make(Args&&... args)
 {
     auto ptr = Heap::allocate(sizeof(T));
     IMMER_TRY {
-        return new (ptr) T{std::forward<Args>(args)...};
+        return new (ptr) T(std::forward<Args>(args)...);
     }
     IMMER_CATCH (...) {
         Heap::deallocate(sizeof(T), ptr);

test/box/generic.ipp

@@ -147,7 +147,16 @@ struct fwd_type;
 struct test_type
 {
     immer::box<fwd_type> data;
+
+    // Constructor that might trigger overload resolution for box<fwd_type>
+    // while fwd_type is still incomplete
+    test_type() = default;
+    test_type(immer::box<fwd_type> d) : data(std::move(d)) {}
 };
+
+// Function that takes test_type by value - requires complete copy/move operations
+inline test_type make_test_type(test_type t) { return t; }
+
 struct fwd_type
 {
     int data = 123;
@@ -158,4 +167,8 @@ TEST_CASE("Test box with a fwd declared type")
 {
     auto val = test_type{};
     REQUIRE(val.data.get().data == 123);
+
+    // Use the function to ensure copy/move are instantiated
+    auto val2 = make_test_type(val);
+    REQUIRE(val2.data.get().data == 123);
 }

test/box/vector-of-boxes-transient.cpp

@@ -7,6 +7,9 @@
 //
 
 #include <immer/box.hpp>
+
+#include <immer/array.hpp>
+#include <immer/array_transient.hpp>
 #include <immer/vector.hpp>
 #include <immer/vector_transient.hpp>
 
@@ -24,13 +27,43 @@ TEST_CASE("issue-33")
 
     // this one doesn't compile
     auto t = vect.transient();
-
     for (auto i = 0; i < 100; ++i) {
         t.push_back(Element("x"));
     }
-
     vect = t.persistent();
 
     CHECK(*vect[0] == "x");
     CHECK(*vect[99] == "x");
 }
+
+namespace {
+struct node_t
+{
+    using array_t = immer::array<immer::box<node_t>>;
+    node_t()      = default;
+    node_t(immer::box<int>) {}
+    node_t(immer::box<double>) {}
+    node_t(array_t) {}
+};
+} // namespace
+
+TEST_CASE("pr-316")
+{
+    auto x = node_t::array_t{}.transient().persistent();
+    CHECK(x.size() == 0);
+}
+
+namespace {
+struct step_t
+{};
+using history_t = immer::vector<immer::box<step_t>>;
+using summary_t = immer::vector<history_t>;
+} // namespace
+
+TEST_CASE("lager tree_debugger issue")
+{
+    auto step    = step_t{};
+    auto hist    = history_t{}.push_back(step);
+    auto summary = summary_t{}.push_back(hist);
+    CHECK(true);
+}

test/map/generic.ipp

@@ -265,6 +265,7 @@ TEST_CASE("accumulate")
 
 TEST_CASE("update a lot")
 {
+    IMMER_GC_TEST_GUARD;
     auto v = make_test_map(666u);
 
     SECTION("immutable")
@@ -293,6 +294,7 @@ TEST_CASE("update a lot")
 
 TEST_CASE("update_if_exists a lot")
 {
+    IMMER_GC_TEST_GUARD;
     auto v = make_test_map(666u);
 
     SECTION("immutable")
@@ -314,6 +316,7 @@ TEST_CASE("update_if_exists a lot")
 
 TEST_CASE("update boxed move string")
 {
+    IMMER_GC_TEST_GUARD;
     constexpr auto N = 666u;
     constexpr auto S = 7;
     auto s = MAP_T<std::string, immer::box<std::string, memory_policy_t>>{};