Merge pull request godotengine#106997 from Ivorforce/fixed-move-semantics

Fix `FixedVector` move and copy semantics.

Author: Repiteo

core/templates/fixed_vector.h

@@ -53,16 +53,15 @@ class FixedVector {
 
 public:
 	_FORCE_INLINE_ constexpr FixedVector() = default;
+
 	constexpr FixedVector(std::initializer_list<T> p_init) {
 		ERR_FAIL_COND(p_init.size() > CAPACITY);
 		for (const T &element : p_init) {
 			memnew_placement(ptr() + _size++, T(element));
 		}
 	}
 
-	template <uint32_t p_capacity>
-	constexpr FixedVector(const FixedVector<T, p_capacity> &p_from) {
-		ERR_FAIL_COND(p_from.size() > CAPACITY);
+	constexpr FixedVector(const FixedVector &p_from) {
 		if constexpr (std::is_trivially_copyable_v<T>) {
 			// Copy size and all provided elements at once.
 			memcpy((void *)&_size, (void *)&p_from._size, sizeof(_size) + DATA_PADDING + p_from.size() * sizeof(T));
@@ -73,15 +72,48 @@ class FixedVector {
 		}
 	}
 
-	template <uint32_t p_capacity>
-	constexpr FixedVector(FixedVector<T, p_capacity> &&p_from) {
-		ERR_FAIL_COND(p_from.size() > CAPACITY);
+	constexpr FixedVector(FixedVector &&p_from) {
 		// Copy size and all provided elements at once.
 		// Note: Assumes trivial relocatability.
 		memcpy((void *)&_size, (void *)&p_from._size, sizeof(_size) + DATA_PADDING + p_from.size() * sizeof(T));
 		p_from._size = 0;
 	}
 
+	constexpr FixedVector &operator=(const FixedVector &p_from) {
+		if constexpr (std::is_trivially_copyable_v<T>) {
+			// Copy size and all provided elements at once.
+			memcpy((void *)&_size, (void *)&p_from._size, sizeof(_size) + DATA_PADDING + p_from.size() * sizeof(T));
+		} else {
+			// Destruct extraneous elements.
+			if constexpr (!std::is_trivially_destructible_v<T>) {
+				for (uint32_t i = p_from.size(); i < _size; i++) {
+					ptr()[i].~T();
+				}
+			}
+
+			_size = 0; // Loop-assign the rest.
+			for (const T &element : p_from) {
+				ptr()[_size++] = element;
+			}
+		}
+		return *this;
+	}
+
+	constexpr FixedVector &operator=(FixedVector &&p_from) {
+		// Destruct extraneous elements.
+		if constexpr (!std::is_trivially_destructible_v<T>) {
+			for (uint32_t i = p_from.size(); i < _size; i++) {
+				ptr()[i].~T();
+			}
+		}
+
+		// Relocate elements (and size) into our buffer.
+		memcpy((void *)&_size, (void *)&p_from._size, sizeof(_size) + DATA_PADDING + p_from.size() * sizeof(T));
+		p_from._size = 0;
+
+		return *this;
+	}
+
 	~FixedVector() {
 		if constexpr (!std::is_trivially_destructible_v<T>) {
 			for (uint32_t i = 0; i < _size; i++) {
@@ -138,6 +170,12 @@ class FixedVector {
 		_size++;
 	}
 
+	constexpr void push_back(T &&p_val) {
+		ERR_FAIL_COND(_size >= CAPACITY);
+		memnew_placement(ptr() + _size, T(std::move(p_val)));
+		_size++;
+	}
+
 	constexpr void pop_back() {
 		ERR_FAIL_COND(_size == 0);
 		_size--;

tests/core/templates/test_fixed_vector.h

@@ -36,6 +36,30 @@
 
 namespace TestFixedVector {
 
+struct MoveOnly {
+	bool is_alive = true;
+
+	MoveOnly() = default;
+	MoveOnly(const MoveOnly &p) = delete;
+	MoveOnly &operator=(const MoveOnly &p) = delete;
+
+	MoveOnly(MoveOnly &&p) {
+		is_alive = p.is_alive;
+		p.is_alive = false;
+	}
+	MoveOnly &operator=(MoveOnly &&p) {
+		if (&p == this) {
+			return *this;
+		}
+		is_alive = p.is_alive;
+		p.is_alive = false;
+		return *this;
+	}
+};
+
+static_assert(!std::is_trivially_copyable_v<MoveOnly>);
+static_assert(!std::is_trivially_constructible_v<MoveOnly>);
+
 TEST_CASE("[FixedVector] Basic Checks") {
 	FixedVector<uint16_t, 1> vector;
 	CHECK_EQ(vector.capacity(), 1);
@@ -62,12 +86,6 @@ TEST_CASE("[FixedVector] Basic Checks") {
 	CHECK_EQ(vector1[0], 1);
 	CHECK_EQ(vector1[1], 2);
 
-	FixedVector<uint16_t, 3> vector2(vector1);
-	CHECK_EQ(vector2.capacity(), 3);
-	CHECK_EQ(vector2.size(), 2);
-	CHECK_EQ(vector2[0], 1);
-	CHECK_EQ(vector2[1], 2);
-
 	FixedVector<Variant, 3> vector_variant;
 	CHECK_EQ(vector_variant.size(), 0);
 	CHECK_EQ(vector_variant.capacity(), 3);
@@ -79,6 +97,17 @@ TEST_CASE("[FixedVector] Basic Checks") {
 	CHECK_EQ(vector_variant[0], "Test");
 	CHECK_EQ(vector_variant[1], Variant(1));
 	CHECK_EQ(vector_variant[2].get_type(), Variant::NIL);
+
+	// Test that move-only types are transferred.
+	FixedVector<MoveOnly, 1> a;
+	a.push_back(MoveOnly());
+	CHECK_EQ(a.size(), 1);
+	FixedVector<MoveOnly, 1> b(std::move(a));
+	CHECK_EQ(a.size(), 0);
+	CHECK_EQ(b.size(), 1);
+	a = std::move(b);
+	CHECK_EQ(a.size(), 1);
+	CHECK_EQ(b.size(), 0);
 }
 
 TEST_CASE("[FixedVector] Alignment Checks") {