Skip to content

Commit c3476b8

Browse files
committed
Fix FixedVector move semantics.
1 parent b89c47b commit c3476b8

File tree

2 files changed

+79
-12
lines changed

2 files changed

+79
-12
lines changed

core/templates/fixed_vector.h

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,15 @@ class FixedVector {
5151

5252
public:
5353
_FORCE_INLINE_ constexpr FixedVector() = default;
54+
5455
constexpr FixedVector(std::initializer_list<T> p_init) {
5556
ERR_FAIL_COND(p_init.size() > CAPACITY);
5657
for (const T &element : p_init) {
5758
memnew_placement(ptr() + _size++, T(element));
5859
}
5960
}
6061

61-
template <uint32_t p_capacity>
62-
constexpr FixedVector(const FixedVector<T, p_capacity> &p_from) {
63-
ERR_FAIL_COND(p_from.size() > CAPACITY);
62+
constexpr FixedVector(const FixedVector &p_from) {
6463
if constexpr (std::is_trivially_copyable_v<T>) {
6564
// Copy size and all provided elements at once.
6665
memcpy((void *)&_size, (void *)&p_from._size, sizeof(_size) + DATA_PADDING + p_from.size() * sizeof(T));
@@ -71,15 +70,48 @@ class FixedVector {
7170
}
7271
}
7372

74-
template <uint32_t p_capacity>
75-
constexpr FixedVector(FixedVector<T, p_capacity> &&p_from) {
76-
ERR_FAIL_COND(p_from.size() > CAPACITY);
73+
constexpr FixedVector(FixedVector &&p_from) {
7774
// Copy size and all provided elements at once.
7875
// Note: Assumes trivial relocatability.
7976
memcpy((void *)&_size, (void *)&p_from._size, sizeof(_size) + DATA_PADDING + p_from.size() * sizeof(T));
8077
p_from._size = 0;
8178
}
8279

80+
constexpr FixedVector &operator=(const FixedVector &p_from) {
81+
if constexpr (std::is_trivially_copyable_v<T>) {
82+
// Copy size and all provided elements at once.
83+
memcpy((void *)&_size, (void *)&p_from._size, sizeof(_size) + DATA_PADDING + p_from.size() * sizeof(T));
84+
} else {
85+
// Destruct extraneous elements.
86+
if constexpr (!std::is_trivially_destructible_v<T>) {
87+
for (uint32_t i = p_from.size(); i < _size; i++) {
88+
ptr()[i].~T();
89+
}
90+
}
91+
92+
_size = 0; // Loop-assign the rest.
93+
for (const T &element : p_from) {
94+
ptr()[_size++] = element;
95+
}
96+
}
97+
return *this;
98+
}
99+
100+
constexpr FixedVector &operator=(FixedVector &&p_from) {
101+
// Destruct extraneous elements.
102+
if constexpr (!std::is_trivially_destructible_v<T>) {
103+
for (uint32_t i = p_from.size(); i < _size; i++) {
104+
ptr()[i].~T();
105+
}
106+
}
107+
108+
// Relocate elements (and size) into our buffer.
109+
memcpy((void *)&_size, (void *)&p_from._size, sizeof(_size) + DATA_PADDING + p_from.size() * sizeof(T));
110+
p_from._size = 0;
111+
112+
return *this;
113+
}
114+
83115
~FixedVector() {
84116
if constexpr (!std::is_trivially_destructible_v<T>) {
85117
for (uint32_t i = 0; i < _size; i++) {
@@ -136,6 +168,12 @@ class FixedVector {
136168
_size++;
137169
}
138170

171+
constexpr void push_back(T &&p_val) {
172+
ERR_FAIL_COND(_size >= CAPACITY);
173+
memnew_placement(ptr() + _size, T(std::move(p_val)));
174+
_size++;
175+
}
176+
139177
constexpr void pop_back() {
140178
ERR_FAIL_COND(_size == 0);
141179
_size--;

tests/core/templates/test_fixed_vector.h

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,30 @@
3636

3737
namespace TestFixedVector {
3838

39+
struct MoveOnly {
40+
bool is_alive = true;
41+
42+
MoveOnly() = default;
43+
MoveOnly(const MoveOnly &p) = delete;
44+
MoveOnly &operator=(const MoveOnly &p) = delete;
45+
46+
MoveOnly(MoveOnly &&p) {
47+
is_alive = p.is_alive;
48+
p.is_alive = false;
49+
}
50+
MoveOnly &operator=(MoveOnly &&p) {
51+
if (&p == this) {
52+
return *this;
53+
}
54+
is_alive = p.is_alive;
55+
p.is_alive = false;
56+
return *this;
57+
}
58+
};
59+
60+
static_assert(!std::is_trivially_copyable_v<MoveOnly>);
61+
static_assert(!std::is_trivially_constructible_v<MoveOnly>);
62+
3963
TEST_CASE("[FixedVector] Basic Checks") {
4064
FixedVector<uint16_t, 1> vector;
4165
CHECK_EQ(vector.capacity(), 1);
@@ -62,12 +86,6 @@ TEST_CASE("[FixedVector] Basic Checks") {
6286
CHECK_EQ(vector1[0], 1);
6387
CHECK_EQ(vector1[1], 2);
6488

65-
FixedVector<uint16_t, 3> vector2(vector1);
66-
CHECK_EQ(vector2.capacity(), 3);
67-
CHECK_EQ(vector2.size(), 2);
68-
CHECK_EQ(vector2[0], 1);
69-
CHECK_EQ(vector2[1], 2);
70-
7189
FixedVector<Variant, 3> vector_variant;
7290
CHECK_EQ(vector_variant.size(), 0);
7391
CHECK_EQ(vector_variant.capacity(), 3);
@@ -79,6 +97,17 @@ TEST_CASE("[FixedVector] Basic Checks") {
7997
CHECK_EQ(vector_variant[0], "Test");
8098
CHECK_EQ(vector_variant[1], Variant(1));
8199
CHECK_EQ(vector_variant[2].get_type(), Variant::NIL);
100+
101+
// Test that move-only types are transferred.
102+
FixedVector<MoveOnly, 1> a;
103+
a.push_back(MoveOnly());
104+
CHECK_EQ(a.size(), 1);
105+
FixedVector<MoveOnly, 1> b(std::move(a));
106+
CHECK_EQ(a.size(), 0);
107+
CHECK_EQ(b.size(), 1);
108+
a = std::move(b);
109+
CHECK_EQ(a.size(), 1);
110+
CHECK_EQ(b.size(), 0);
82111
}
83112

84113
TEST_CASE("[FixedVector] Alignment Checks") {

0 commit comments

Comments
 (0)