From c953d6940ae3347e35ebe741e77d62d082ae3fdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pedro=20Bol=C3=ADvar=20Puente?= Date: Tue, 13 Jan 2026 12:26:29 +0100 Subject: [PATCH 1/3] Fix issues with collections of boxes failing to compile The fix involves preferring () to {} when calling placement-new generally, as the latter often leads to the initialization_list based constructor to be preferred, which causes the issue. This was introduced in: https://github.com/arximboldi/immer/commit/6d22844a30aed380d9fb38ea5d4bb59128943f53 --- immer/array_transient.hpp | 4 +-- immer/box.hpp | 2 +- immer/detail/arrays/no_capacity.hpp | 2 +- immer/detail/arrays/with_capacity.hpp | 6 ++--- immer/detail/hamts/node.hpp | 4 +-- immer/detail/rbts/node.hpp | 6 ++--- immer/detail/rbts/rbtree.hpp | 2 +- immer/detail/rbts/rrbtree.hpp | 2 +- immer/detail/util.hpp | 2 +- test/box/vector-of-boxes-transient.cpp | 37 ++++++++++++++++++++++++-- 10 files changed, 50 insertions(+), 17 deletions(-) diff --git a/immer/array_transient.hpp b/immer/array_transient.hpp index 0f1c96ff..6d62702f 100644 --- a/immer/array_transient.hpp +++ b/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: diff --git a/immer/box.hpp b/immer/box.hpp index 92e02909..6cb4de65 100644 --- a/immer/box.hpp +++ b/immer/box.hpp @@ -44,7 +44,7 @@ class box template holder(Args&&... args) - : value{std::forward(args)...} + : value(std::forward(args)...) { } }; diff --git a/immer/detail/arrays/no_capacity.hpp b/immer/detail/arrays/no_capacity.hpp index eaf3cf3d..77a402dc 100644 --- a/immer/detail/arrays/no_capacity.hpp +++ b/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 (...) { diff --git a/immer/detail/arrays/with_capacity.hpp b/immer/detail/arrays/with_capacity.hpp index 57546545..a8d56ff6 100644 --- a/immer/detail/arrays/with_capacity.hpp +++ b/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 (...) { diff --git a/immer/detail/hamts/node.hpp b/immer/detail/hamts/node.hpp index b04a9d17..32b8c92a 100644 --- a/immer/detail/hamts/node.hpp +++ b/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) diff --git a/immer/detail/rbts/node.hpp b/immer/detail/rbts/node.hpp index 4f1c3230..1d788765 100644 --- a/immer/detail/rbts/node.hpp +++ b/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(x)}; + new (p->leaf()) T(std::forward(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(x)}; + new (p->leaf()) T(std::forward(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(x)}; + new (dst->leaf() + n) T(std::forward(x)); } IMMER_CATCH (...) { detail::destroy_n(dst->leaf(), n); diff --git a/immer/detail/rbts/rbtree.hpp b/immer/detail/rbts/rbtree.hpp index c8fa4251..54d55638 100644 --- a/immer/detail/rbts/rbtree.hpp +++ b/immer/detail/rbts/rbtree.hpp @@ -274,7 +274,7 @@ struct rbtree auto ts = size - tail_off; if (ts < branches) { 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 { diff --git a/immer/detail/rbts/rrbtree.hpp b/immer/detail/rbts/rrbtree.hpp index f8ee3f84..e96c5c41 100644 --- a/immer/detail/rbts/rrbtree.hpp +++ b/immer/detail/rbts/rrbtree.hpp @@ -479,7 +479,7 @@ struct rrbtree auto ts = tail_size(); if (ts < branches) { 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)); diff --git a/immer/detail/util.hpp b/immer/detail/util.hpp index 21822bad..3c8c0070 100644 --- a/immer/detail/util.hpp +++ b/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)...}; + return new (ptr) T(std::forward(args)...); } IMMER_CATCH (...) { Heap::deallocate(sizeof(T), ptr); diff --git a/test/box/vector-of-boxes-transient.cpp b/test/box/vector-of-boxes-transient.cpp index 386be84c..04cb7215 100644 --- a/test/box/vector-of-boxes-transient.cpp +++ b/test/box/vector-of-boxes-transient.cpp @@ -7,6 +7,9 @@ // #include + +#include +#include #include #include @@ -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>; + node_t() = default; + node_t(immer::box) {} + node_t(immer::box) {} + 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>; +using summary_t = immer::vector; +} // 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); +} From 177458454e3d1e451de7b166f9ff207d24a87754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pedro=20Bol=C3=ADvar=20Puente?= Date: Tue, 13 Jan 2026 18:36:53 +0100 Subject: [PATCH 2/3] Restrict the value-constructor of immer::box This reintroduces the check that was removed in: https://github.com/arximboldi/immer/commit/6d22844a30aed380d9fb38ea5d4bb59128943f53 It does so in a way that is compatible with incomplete types. --- immer/box.hpp | 14 ++++++++++++-- test/box/generic.ipp | 13 +++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/immer/box.hpp b/immer/box.hpp index 6cb4de65..935729bb 100644 --- a/immer/box.hpp +++ b/immer/box.hpp @@ -77,7 +77,10 @@ class box */ template >::value>> + !std::is_same>::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()))> box(Arg&& arg) : impl_{detail::make(std::forward(arg))} { @@ -86,7 +89,14 @@ class box /*! * Constructs a box holding `T{arg1, arg2, args...}` */ - template + template (), + std::declval(), + std::declval()...))> box(Arg1&& arg1, Arg2&& arg2, Args&&... args) : impl_{detail::make(std::forward(arg1), std::forward(arg2), diff --git a/test/box/generic.ipp b/test/box/generic.ipp index d7b7e89e..184b7ef3 100644 --- a/test/box/generic.ipp +++ b/test/box/generic.ipp @@ -147,7 +147,16 @@ struct fwd_type; struct test_type { immer::box data; + + // Constructor that might trigger overload resolution for box + // while fwd_type is still incomplete + test_type() = default; + test_type(immer::box 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); } From 160f0d7025321e5dc1fc3f0b7742ca7621d43981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pedro=20Bol=C3=ADvar=20Puente?= Date: Tue, 13 Jan 2026 18:54:58 +0100 Subject: [PATCH 3/3] Add the IMMER_GC_TEST_GUARD to more tests They are pseudo-deterministically flaky? Depends on when GC gets triggered... Seems mostly deterministic for a single buidl, but the points change whenever you really change the code in any way. --- test/map/generic.ipp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/map/generic.ipp b/test/map/generic.ipp index 4ceb8fa5..d3335987 100644 --- a/test/map/generic.ipp +++ b/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>{};