From 63658e8915e6e0311a85eeb096c97579a20b194a Mon Sep 17 00:00:00 2001
From: Niels Dekker <N.Dekker@lumc.nl>
Date: Tue, 18 Jun 2024 16:12:06 +0200
Subject: [PATCH] PERF: Add fast `noexcept` "move semantics" to `Array2D`

Added a defaulted `noexcept` move-constructor and move-assignment operator to
`Array2D`, which both _move_ the data of the `Array2D`.

Previously, an attempt to "move" an `Array2D` would have done an expensive copy.
---
 Modules/Core/Common/include/itkArray2D.h     | 11 ++++
 Modules/Core/Common/test/itkArray2DGTest.cxx | 56 ++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/Modules/Core/Common/include/itkArray2D.h b/Modules/Core/Common/include/itkArray2D.h
index 44f0089bb6e..eadafb3f3cf 100644
--- a/Modules/Core/Common/include/itkArray2D.h
+++ b/Modules/Core/Common/include/itkArray2D.h
@@ -60,6 +60,11 @@ class ITK_TEMPLATE_EXPORT Array2D : public vnl_matrix<TValue>
   /** Copy-constructor. */
   Array2D(const Self & array);
 
+  /** Move-constructor.
+   * \note This move-constructor is `noexcept`, even while the move-constructor of its base class (`vnl_matrix`) is not
+   * `noexcept`, because unlike `vnl_matrix`, `Array2D` always manages its own memory. */
+  Array2D(Self &&) noexcept = default;
+
   /** Converting constructor. Implicitly converts the specified matrix to an Array2D. */
   Array2D(const VnlMatrixType & matrix);
 
@@ -67,6 +72,12 @@ class ITK_TEMPLATE_EXPORT Array2D : public vnl_matrix<TValue>
   Self &
   operator=(const Self & array);
 
+  /** Move-assignment operator.
+   * \note This move-assignment operator is `noexcept`, even while the move-assignment operator of its base class
+   * (`vnl_matrix`) is not `noexcept`, because unlike `vnl_matrix`, `Array2D` always manages its own memory. */
+  Self &
+  operator=(Self &&) noexcept = default;
+
   /** Assigns the specified matrix to an Array2D. */
   Self &
   operator=(const VnlMatrixType & matrix);
diff --git a/Modules/Core/Common/test/itkArray2DGTest.cxx b/Modules/Core/Common/test/itkArray2DGTest.cxx
index 54f562bf4e8..3c60d4309b9 100644
--- a/Modules/Core/Common/test/itkArray2DGTest.cxx
+++ b/Modules/Core/Common/test/itkArray2DGTest.cxx
@@ -22,6 +22,13 @@
 #include <limits>
 
 
+static_assert(std::is_nothrow_move_constructible_v<itk::Array2D<int>> &&
+                std::is_nothrow_move_constructible_v<itk::Array2D<double>>,
+              "Array2D should have a `noexcept` move-constructor!");
+static_assert(std::is_nothrow_move_assignable_v<itk::Array2D<int>> &&
+                std::is_nothrow_move_assignable_v<itk::Array2D<double>>,
+              "Array2D should have a `noexcept` move-assignment operator!");
+
 // Tests that Array2D may be constructed with an initial value for each element.
 TEST(Array2D, ConstructorSupportsInitialValue)
 {
@@ -51,3 +58,52 @@ TEST(Array2D, ConstructorSupportsInitialValue)
     checkConstructor(1, 2, initialValue);
   }
 }
+
+
+// Tests that when move-constructing an Array2D, the data is "taken" from the original, and "moved" to the newly
+// constructed object.
+TEST(Array2D, MoveConstruct)
+{
+  const auto checkMoveConstruct = [](auto && original) {
+    const auto * const * const originalDataArray{ original.data_array() };
+    const unsigned int         originalSize{ original.size() };
+
+    const auto moveConstructed = std::move(original);
+
+    // After the "move", the move-constructed object has retrieved the original data.
+    EXPECT_EQ(moveConstructed.data_array(), originalDataArray);
+    EXPECT_EQ(moveConstructed.size(), originalSize);
+
+    // After the "move", the original is left empty.
+    EXPECT_EQ(original.data_array(), nullptr);
+    EXPECT_EQ(original.size(), 0U);
+  };
+
+  checkMoveConstruct(itk::Array2D<int>(1U, 1U));
+  checkMoveConstruct(itk::Array2D<double>(1U, 2U));
+}
+
+
+// Tests that when move-assigning an Array2D, the data is "taken" from the original, and "moved" to the target of the
+// move-assignment.
+TEST(Array2D, MoveAssign)
+{
+  const auto checkMoveAssign = [](auto original) {
+    const auto * const * const originalDataArray{ original.data_array() };
+    const unsigned int         originalSize{ original.size() };
+
+    decltype(original) moveAssigmentTarget;
+    moveAssigmentTarget = std::move(original);
+
+    // After the "move", the target of the move-assignment has retrieved the original data.
+    EXPECT_EQ(moveAssigmentTarget.data_array(), originalDataArray);
+    EXPECT_EQ(moveAssigmentTarget.size(), originalSize);
+
+    // After the "move", the original is left empty.
+    EXPECT_EQ(original.data_array(), nullptr);
+    EXPECT_EQ(original.size(), 0U);
+  };
+
+  checkMoveAssign(itk::Array2D<int>(1U, 1U));
+  checkMoveAssign(itk::Array2D<double>(1U, 2U));
+}