Skip to content

Commit 8fe2b70

Browse files
committed
Fix assigning to a vector that has been move-assigned from.
Support minimum sizes greater than 0 in `dynamic_array`. Make move construction of `uninitialized_dynamic_array` faster at the expense of some copy assignment operators for types that use it. Support non-default-constructible types in `test_sequence_container`.
1 parent 75fb909 commit 8fe2b70

12 files changed

+87
-30
lines changed

source/containers/bounded_vector.cpp

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,11 @@ struct [[clang::trivial_abi]] bounded_vector : private lexicographical_compariso
122122
if (this == std::addressof(other)) {
123123
return *this;
124124
}
125-
if constexpr (min_capacity > 0_bi) {
126-
if (!m_storage.data()) {
127-
replace_empty_allocation(other.size());
128-
::containers::uninitialized_copy_no_overlap(other, data());
129-
m_size = other.m_size;
130-
return *this;
131-
}
125+
if (!m_storage.data()) {
126+
replace_empty_allocation(other.size());
127+
::containers::uninitialized_copy_no_overlap(other, data());
128+
m_size = other.m_size;
129+
return *this;
132130
}
133131
containers::assign(*this, other);
134132
return *this;

source/containers/dynamic_array.cpp

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,12 @@ import numeric_traits;
3333
import std_module;
3434

3535
namespace containers {
36+
37+
using namespace bounded::literal;
3638

3739
export template<typename T, typename Size = array_size_type<T>>
3840
struct [[clang::trivial_abi]] dynamic_array : private lexicographical_comparison::base {
41+
static_assert(numeric_traits::min_value<Size> >= 0_bi);
3942
using size_type = Size;
4043

4144
constexpr dynamic_array() = default;
@@ -62,12 +65,14 @@ struct [[clang::trivial_abi]] dynamic_array : private lexicographical_comparison
6265
}
6366

6467
constexpr dynamic_array(dynamic_array && other) noexcept:
65-
m_data(std::exchange(other.m_data, {}))
68+
m_data(std::move(other.m_data))
6669
{
6770
}
6871

6972
constexpr ~dynamic_array() {
70-
::containers::destroy_range(*this);
73+
if (data()) {
74+
::containers::destroy_range(*this);
75+
}
7176
}
7277

7378
constexpr auto operator=(dynamic_array const & other) & -> dynamic_array & {
@@ -77,9 +82,10 @@ struct [[clang::trivial_abi]] dynamic_array : private lexicographical_comparison
7782
return *this;
7883
}
7984
constexpr auto operator=(dynamic_array && other) & noexcept -> dynamic_array & {
80-
::containers::destroy_range(*this);
85+
if (data()) {
86+
::containers::destroy_range(*this);
87+
}
8188
m_data = std::move(other.m_data);
82-
other.m_data = {};
8389
return *this;
8490
}
8591

@@ -100,13 +106,20 @@ struct [[clang::trivial_abi]] dynamic_array : private lexicographical_comparison
100106
*this = {};
101107
}
102108

109+
// Requires that `range` does not alias any elements of `*this`
103110
template<size_then_use_range Range> requires(!std::is_array_v<Range> or !std::is_reference_v<Range>)
104111
constexpr auto assign(Range && range) & -> void {
105-
auto const difference = ::containers::linear_size(range);
106-
if (difference == size()) {
112+
auto const new_size = ::containers::linear_size(range);
113+
if (!data()) {
114+
*this = dynamic_array<T, Size>(OPERATORS_FORWARD(range));
115+
return;
116+
}
117+
if (new_size == size()) {
107118
::containers::copy(OPERATORS_FORWARD(range), ::containers::begin(*this));
108119
} else {
109-
clear();
120+
if constexpr (numeric_traits::min_value<Size> == 0_bi) {
121+
clear();
122+
}
110123
*this = dynamic_array<T, Size>(OPERATORS_FORWARD(range));
111124
}
112125
}

source/containers/test/bidirectional_linked_list.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ using namespace bounded::literal;
2929

3030
static_assert(bounded::convertible_to<containers::bidirectional_linked_list<int>::iterator, containers::bidirectional_linked_list<int>::const_iterator>);
3131

32+
static_assert(bounded::default_constructible<containers::bidirectional_linked_list<int>>);
33+
static_assert(bounded::default_constructible<containers::bidirectional_linked_list<bounded_test::integer>>);
3234
static_assert(containers_test::test_sequence_container<containers::bidirectional_linked_list<int>>());
3335
static_assert(containers_test::test_sequence_container<containers::bidirectional_linked_list<bounded_test::integer>>());
3436

source/containers/test/dynamic_array.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,15 @@ import bounded;
1313
import bounded.test_int;
1414
import std_module;
1515

16+
static_assert(bounded::default_constructible<containers::dynamic_array<int>>);
17+
static_assert(bounded::default_constructible<containers::dynamic_array<bounded_test::integer>>);
18+
1619
static_assert(containers_test::test_sequence_container<containers::dynamic_array<int>>());
1720
static_assert(containers_test::test_sequence_container<containers::dynamic_array<bounded_test::integer>>());
1821

22+
static_assert(containers_test::test_sequence_container<containers::dynamic_array<int, bounded::integer<1, 40>>>());
23+
static_assert(containers_test::test_sequence_container<containers::dynamic_array<bounded_test::integer, bounded::integer<1, 40>>>());
24+
1925
static_assert(bounded::convertible_to<containers::dynamic_array<bounded_test::integer> const &, std::span<bounded_test::integer const>>);
2026
static_assert(bounded::convertible_to<containers::dynamic_array<bounded_test::integer> &, std::span<bounded_test::integer>>);
2127

source/containers/test/forward_linked_list.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ using namespace bounded::literal;
2929

3030
static_assert(bounded::convertible_to<containers::forward_linked_list<int>::iterator, containers::forward_linked_list<int>::const_iterator>);
3131

32+
static_assert(bounded::default_constructible<containers::forward_linked_list<int>>);
33+
static_assert(bounded::default_constructible<containers::forward_linked_list<bounded_test::integer>>);
3234
static_assert(containers_test::test_sequence_container<containers::forward_linked_list<int>>());
3335
static_assert(containers_test::test_sequence_container<containers::forward_linked_list<bounded_test::non_copyable_integer>>());
3436

source/containers/test/small_buffer_optimized_vector.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,14 @@ namespace {
2020

2121
using namespace bounded::literal;
2222

23+
static_assert(bounded::default_constructible<containers::small_buffer_optimized_vector<int, 40>>);
2324
static_assert(containers_test::test_sequence_container<containers::small_buffer_optimized_vector<int, 40>>());
2425
static_assert(containers_test::test_set_size<containers::small_buffer_optimized_vector<int, 1>>());
2526

2627
template<typename T>
2728
constexpr auto test_sbo_vector_impl(auto const capacity, auto const size) -> void {
2829
using container = containers::small_buffer_optimized_vector<T, capacity.value()>;
30+
static_assert(bounded::default_constructible<container>);
2931
static_assert(containers_test::test_sequence_container<container>());
3032
static_assert(containers_test::test_reserve_and_capacity<container>());
3133
constexpr auto generator = containers::repeat_default_n<T>(size);
@@ -49,6 +51,7 @@ TEST_CASE("small_buffer_optimized_vector", "[small_buffer_optimized_vector]") {
4951
test_sbo_vector<char>(254_bi);
5052
test_sbo_vector<char>(255_bi);
5153
test_sbo_vector<char>(256_bi);
54+
static_assert(bounded::default_constructible<containers::small_buffer_optimized_vector<bounded_test::integer, 3>>);
5255
containers_test::test_sequence_container<containers::small_buffer_optimized_vector<bounded_test::integer, 3>>();
5356
containers_test::test_reserve_and_capacity<containers::small_buffer_optimized_vector<bounded_test::integer, 3>>();
5457
containers_test::test_set_size<containers::small_buffer_optimized_vector<bounded_test::integer, 1>>();

source/containers/test/stable_vector.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ using test_stable_vector = containers::stable_vector<T, 1000_bi>;
2222

2323
static_assert(test_stable_vector<int>::capacity() == 1000_bi);
2424

25+
static_assert(bounded::default_constructible<test_stable_vector<int>>);
26+
static_assert(bounded::default_constructible<test_stable_vector<bounded_test::integer>>);
2527
static_assert(containers_test::test_sequence_container<test_stable_vector<int>>());
2628
static_assert(containers_test::test_sequence_container<test_stable_vector<bounded_test::integer>>());
2729

source/containers/test/static_vector.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ using test_static_vector = containers::static_vector<T, 40_bi>;
5757

5858
static_assert(test_static_vector<int>::capacity() == 40_bi);
5959

60+
static_assert(bounded::default_constructible<test_static_vector<int>>);
6061
static_assert(containers_test::test_sequence_container<test_static_vector<int>>());
6162
static_assert(containers_test::test_set_size<test_static_vector<int>>());
6263

@@ -73,6 +74,7 @@ static_assert(homogeneous_equals(
7374
static_assert(!bounded::constructible_from<containers::static_vector<int, 1_bi>, containers::c_array<int, 2> &&>);
7475

7576
TEST_CASE("static_vector", "[static_vector]") {
77+
static_assert(bounded::default_constructible<test_static_vector<bounded_test::integer>>);
7678
containers_test::test_sequence_container<test_static_vector<bounded_test::integer>>();
7779
containers_test::test_set_size<test_static_vector<bounded_test::integer>>();
7880
}

source/containers/test/string.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ auto check_equal(std::string_view const input) {
2323
}
2424

2525
TEST_CASE("string", "[string]") {
26+
static_assert(bounded::default_constructible<containers::string>);
2627
containers_test::test_sequence_container<containers::string>();
2728
containers_test::test_reserve_and_capacity<containers::string>();
2829
containers_test::test_set_size<containers::string>();

source/containers/test/test_sequence_container.cpp

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,23 @@ constexpr auto test_default_copy_assignment_from_non_empty(auto const make) -> v
129129
BOUNDED_ASSERT(containers::equal(target, Container()));
130130
}
131131
template<typename Container>
132-
constexpr auto test_copy_assignment_from_moved_from(auto const make) -> void {
132+
constexpr auto test_copy_assignment_from_move_constructed_from(auto const make) -> void {
133133
auto target = Container(make());
134134
auto const source = std::move(target);
135135
target = source;
136136
BOUNDED_ASSERT(containers::equal(target, make()));
137137
BOUNDED_ASSERT(containers::equal(target, source));
138138
}
139139
template<typename Container>
140+
constexpr auto test_copy_assignment_from_move_assigned_from(auto const make) -> void {
141+
auto target = Container(make());
142+
auto source = Container(make());
143+
source = std::move(target);
144+
target = source;
145+
BOUNDED_ASSERT(containers::equal(target, make()));
146+
BOUNDED_ASSERT(containers::equal(target, source));
147+
}
148+
template<typename Container>
140149
constexpr auto test_self_copy_assignment(auto const make) -> void {
141150
auto copy = Container(make());
142151
// Turn off compiler warning for self assignment
@@ -146,11 +155,14 @@ constexpr auto test_self_copy_assignment(auto const make) -> void {
146155
}
147156
template<typename Container>
148157
constexpr auto test_copy_assignment(auto const make) -> void {
149-
test_copy_assignment_from_empty<Container>(make);
158+
if constexpr (bounded::default_constructible<Container>) {
159+
test_copy_assignment_from_empty<Container>(make);
160+
test_default_copy_assignment_from_empty<Container>();
161+
test_default_copy_assignment_from_non_empty<Container>(make);
162+
}
150163
test_copy_assignment_from_non_empty<Container>(make);
151-
test_default_copy_assignment_from_empty<Container>();
152-
test_default_copy_assignment_from_non_empty<Container>(make);
153-
test_copy_assignment_from_moved_from<Container>(make);
164+
test_copy_assignment_from_move_constructed_from<Container>(make);
165+
test_copy_assignment_from_move_assigned_from<Container>(make);
154166
test_self_copy_assignment<Container>(make);
155167
}
156168

@@ -169,13 +181,21 @@ constexpr auto test_move_assignment_from_non_empty(auto const make) -> void {
169181
BOUNDED_ASSERT(containers::equal(target, make()));
170182
}
171183
template<typename Container>
172-
constexpr auto test_move_assignment_from_moved_from(auto const make) -> void {
184+
constexpr auto test_move_assignment_from_move_constructed_from(auto const make) -> void {
173185
auto target = Container(make());
174186
auto temp = std::move(target);
175187
target = std::move(temp);
176188
BOUNDED_ASSERT(containers::equal(target, make()));
177189
}
178190
template<typename Container>
191+
constexpr auto test_move_assignment_from_move_assigned_from(auto const make) -> void {
192+
auto target = Container(make());
193+
auto temp = Container(make());
194+
temp = std::move(target);
195+
target = std::move(temp);
196+
BOUNDED_ASSERT(containers::equal(target, make()));
197+
}
198+
template<typename Container>
179199
constexpr auto test_recover_from_self_move(auto const make, auto const & validate) -> void {
180200
auto container = Container(make());
181201
container = std::move(container);
@@ -194,9 +214,12 @@ constexpr auto test_self_move_assignment(auto const make, auto const & validate)
194214
}
195215
template<typename Container>
196216
constexpr auto test_move_assignment(auto const make) -> void {
197-
test_move_assignment_from_empty<Container>(make);
217+
if constexpr (bounded::default_constructible<Container>) {
218+
test_move_assignment_from_empty<Container>(make);
219+
}
198220
test_move_assignment_from_non_empty<Container>(make);
199-
test_move_assignment_from_moved_from<Container>(make);
221+
test_move_assignment_from_move_constructed_from<Container>(make);
222+
test_move_assignment_from_move_assigned_from<Container>(make);
200223
test_self_move_assignment<Container>(make, [&](Container const & container) { return containers::equal(container, make()); });
201224
}
202225

@@ -240,7 +263,9 @@ constexpr auto test_special_members(auto const make) -> bool {
240263
test_copy_assignment<Container>(make);
241264
}
242265
}
243-
test_assignment_from_empty_braces<Container>(make);
266+
if constexpr (bounded::default_constructible<Container>) {
267+
test_assignment_from_empty_braces<Container>(make);
268+
}
244269
test_swap(make);
245270
return true;
246271
}
@@ -256,14 +281,16 @@ constexpr auto test_sequence_container() -> bool {
256281
static_assert(containers::is_container<Container>);
257282
test_forward_range_concepts<Container>();
258283

259-
test_range_based_for_loop<Container>();
284+
if constexpr (bounded::default_constructible<Container>) {
285+
test_range_based_for_loop<Container>();
260286

261-
test_sequence_container_default_constructed_empty<Container>();
262-
test_sequence_container_implicit_from_two_empty_braces<Container>();
287+
test_sequence_container_default_constructed_empty<Container>();
288+
test_sequence_container_implicit_from_two_empty_braces<Container>();
263289

264-
test_sequence_container_from<Container>([] {
265-
return containers::to_array<containers::range_value_t<Container>>({});
266-
});
290+
test_sequence_container_from<Container>([] {
291+
return containers::to_array<containers::range_value_t<Container>>({});
292+
});
293+
}
267294
test_sequence_container_from<Container>([] {
268295
return containers::to_array<containers::range_value_t<Container>>({5});
269296
});

0 commit comments

Comments
 (0)