Skip to content

Commit 6adf650

Browse files
arximboldiarximboldi
authored
Merge pull request #318 from arximboldi/box-issues
Fix issues with immer::box when used in collections
2 parents 38ad8ee + 160f0d7 commit 6adf650

12 files changed

Lines changed: 78 additions & 19 deletions

File tree

‎immer/array_transient.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,11 @@ class array_transient : MemoryPolicy::transience_t::owner
184184
IMMER_NODISCARD persistent_type persistent() &
185185
{
186186
this->owner_t::operator=(owner_t{});
187-
return persistent_type{impl_};
187+
return persistent_type(impl_);
188188
}
189189
IMMER_NODISCARD persistent_type persistent() &&
190190
{
191-
return persistent_type{std::move(impl_)};
191+
return persistent_type(std::move(impl_));
192192
}
193193

194194
private:

‎immer/box.hpp‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class box
4444

4545
template <typename... Args>
4646
holder(Args&&... args)
47-
: value{std::forward<Args>(args)...}
47+
: value(std::forward<Args>(args)...)
4848
{
4949
}
5050
};
@@ -77,7 +77,10 @@ class box
7777
*/
7878
template <typename Arg,
7979
typename Enable = std::enable_if_t<
80-
!std::is_same<box, std::decay_t<Arg>>::value>>
80+
!std::is_same<box, std::decay_t<Arg>>::value>,
81+
// this is similar to std::is_constructible but works around the
82+
// fact that is_constructible is ill-formed for incomplete types
83+
typename = decltype(T(std::declval<Arg>()))>
8184
box(Arg&& arg)
8285
: impl_{detail::make<heap, holder>(std::forward<Arg>(arg))}
8386
{
@@ -86,7 +89,14 @@ class box
8689
/*!
8790
* Constructs a box holding `T{arg1, arg2, args...}`
8891
*/
89-
template <typename Arg1, typename Arg2, typename... Args>
92+
template <typename Arg1,
93+
typename Arg2,
94+
typename... Args,
95+
// this is similar to std::is_constructible but works around the
96+
// fact that is_constructible is ill-formed for incomplete types
97+
typename = decltype(T(std::declval<Arg1>(),
98+
std::declval<Arg2>(),
99+
std::declval<Args>()...))>
90100
box(Arg1&& arg1, Arg2&& arg2, Args&&... args)
91101
: impl_{detail::make<heap, holder>(std::forward<Arg1>(arg1),
92102
std::forward<Arg2>(arg2),

‎immer/detail/arrays/no_capacity.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ struct no_capacity
163163
{
164164
auto p = node_t::copy_n(size + 1, ptr, size);
165165
IMMER_TRY {
166-
new (p->data() + size) T{std::move(value)};
166+
new (p->data() + size) T(std::move(value));
167167
return {p, size + 1};
168168
}
169169
IMMER_CATCH (...) {

‎immer/detail/arrays/with_capacity.hpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ struct with_capacity
198198
auto cap = recommend_up(size + 1, capacity);
199199
auto p = node_t::copy_n(cap, ptr, size);
200200
IMMER_TRY {
201-
new (p->data() + size) T{std::move(value)};
201+
new (p->data() + size) T(std::move(value));
202202
return {p, size + 1, cap};
203203
}
204204
IMMER_CATCH (...) {
@@ -210,13 +210,13 @@ struct with_capacity
210210
void push_back_mut(edit_t e, T value)
211211
{
212212
if (ptr->can_mutate(e) && capacity > size) {
213-
new (data() + size) T{std::move(value)};
213+
new (data() + size) T(std::move(value));
214214
++size;
215215
} else {
216216
auto cap = recommend_up(size + 1, capacity);
217217
auto p = node_t::copy_e(e, cap, ptr, size);
218218
IMMER_TRY {
219-
new (p->data() + size) T{std::move(value)};
219+
new (p->data() + size) T(std::move(value));
220220
*this = {p, size + 1, cap};
221221
}
222222
IMMER_CATCH (...) {

‎immer/detail/hamts/node.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ struct node
713713
detail::uninitialized_copy(
714714
src->values(), src->values() + voffset, dst->values());
715715
IMMER_TRY {
716-
new (dst->values() + voffset) T{std::move(value)};
716+
new (dst->values() + voffset) T(std::move(value));
717717
IMMER_TRY {
718718
if (nv)
719719
detail::uninitialized_copy(src->values() + voffset,
@@ -768,7 +768,7 @@ struct node
768768
src->values(), src->values() + voffset, dst->values());
769769
}
770770
IMMER_TRY {
771-
new (dst->values() + voffset) T{std::move(value)};
771+
new (dst->values() + voffset) T(std::move(value));
772772
IMMER_TRY {
773773
if (nv) {
774774
if (mutate_values)

‎immer/detail/rbts/node.hpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ struct node
467467
assert(n >= 1);
468468
auto p = make_leaf_n(n);
469469
IMMER_TRY {
470-
new (p->leaf()) T{std::forward<U>(x)};
470+
new (p->leaf()) T(std::forward<U>(x));
471471
}
472472
IMMER_CATCH (...) {
473473
heap::deallocate(node_t::sizeof_leaf_n(n), p);
@@ -481,7 +481,7 @@ struct node
481481
{
482482
auto p = make_leaf_e(e);
483483
IMMER_TRY {
484-
new (p->leaf()) T{std::forward<U>(x)};
484+
new (p->leaf()) T(std::forward<U>(x));
485485
}
486486
IMMER_CATCH (...) {
487487
heap::deallocate(node_t::max_sizeof_leaf, p);
@@ -790,7 +790,7 @@ struct node
790790
{
791791
auto dst = copy_leaf_n(n + 1, src, n);
792792
IMMER_TRY {
793-
new (dst->leaf() + n) T{std::forward<U>(x)};
793+
new (dst->leaf() + n) T(std::forward<U>(x));
794794
}
795795
IMMER_CATCH (...) {
796796
detail::destroy_n(dst->leaf(), n);

‎immer/detail/rbts/rbtree.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ struct rbtree
274274
auto ts = size - tail_off;
275275
if (ts < branches<BL>) {
276276
ensure_mutable_tail(e, ts);
277-
new (&tail->leaf()[ts]) T{std::move(value)};
277+
new (&tail->leaf()[ts]) T(std::move(value));
278278
} else {
279279
auto new_tail = node_t::make_leaf_e(e, std::move(value));
280280
IMMER_TRY {

‎immer/detail/rbts/rrbtree.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ struct rrbtree
479479
auto ts = tail_size();
480480
if (ts < branches<BL>) {
481481
ensure_mutable_tail(e, ts);
482-
new (&tail->leaf()[ts]) T{std::move(value)};
482+
new (&tail->leaf()[ts]) T(std::move(value));
483483
} else {
484484
using std::get;
485485
auto new_tail = node_t::make_leaf_e(e, std::move(value));

‎immer/detail/util.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ T* make(Args&&... args)
174174
{
175175
auto ptr = Heap::allocate(sizeof(T));
176176
IMMER_TRY {
177-
return new (ptr) T{std::forward<Args>(args)...};
177+
return new (ptr) T(std::forward<Args>(args)...);
178178
}
179179
IMMER_CATCH (...) {
180180
Heap::deallocate(sizeof(T), ptr);

‎test/box/generic.ipp‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,16 @@ struct fwd_type;
147147
struct test_type
148148
{
149149
immer::box<fwd_type> data;
150+
151+
// Constructor that might trigger overload resolution for box<fwd_type>
152+
// while fwd_type is still incomplete
153+
test_type() = default;
154+
test_type(immer::box<fwd_type> d) : data(std::move(d)) {}
150155
};
156+
157+
// Function that takes test_type by value - requires complete copy/move operations
158+
inline test_type make_test_type(test_type t) { return t; }
159+
151160
struct fwd_type
152161
{
153162
int data = 123;
@@ -158,4 +167,8 @@ TEST_CASE("Test box with a fwd declared type")
158167
{
159168
auto val = test_type{};
160169
REQUIRE(val.data.get().data == 123);
170+
171+
// Use the function to ensure copy/move are instantiated
172+
auto val2 = make_test_type(val);
173+
REQUIRE(val2.data.get().data == 123);
161174
}

0 commit comments

Comments
 (0)