From 31f12337e6ea96ce339ba1d05c18aa248a577ba6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Del=20Pino?= <stephane.delpino44@gmail.com>
Date: Tue, 6 Apr 2021 08:42:45 +0200
Subject: [PATCH] Change SubArray implementation

It does not store the underlying Array anymore.
- this is quite dangerous but it increases performances by a factor 2+
- all copy constructors have been removed as well to reduce the risk
of bad manipulation (ie: using a SubArray on a destroyed Array)
---
 src/utils/SubArray.hpp  | 42 ++++++++--------------------------------
 tests/test_SubArray.cpp | 43 +----------------------------------------
 2 files changed, 9 insertions(+), 76 deletions(-)

diff --git a/src/utils/SubArray.hpp b/src/utils/SubArray.hpp
index 5b79bf44c..d96bb008d 100644
--- a/src/utils/SubArray.hpp
+++ b/src/utils/SubArray.hpp
@@ -16,11 +16,8 @@ class [[nodiscard]] SubArray
   using index_type = size_t;
 
  private:
-  // underlying array
-  Array<DataType> m_array;
-
-  DataType* m_sub_values;
-  size_t m_size;
+  PUGS_RESTRICT DataType* const m_sub_values;
+  const size_t m_size;
 
   // Allows const version to access our data
   friend SubArray<std::add_const_t<DataType>>;
@@ -47,32 +44,15 @@ class [[nodiscard]] SubArray
       this->size(), PUGS_LAMBDA(index_type i) { m_sub_values[i] = data; });
   }
 
-  template <typename DataType2>
-  PUGS_INLINE SubArray& operator=(const SubArray<DataType2>& sub_array) noexcept
-  {
-    // ensures that DataType is the same as source DataType2
-    static_assert(std::is_same<std::remove_const_t<DataType>, std::remove_const_t<DataType2>>(),
-                  "Cannot assign SubArray of different type");
-    // ensures that const is not lost through copy
-    static_assert(((std::is_const<DataType2>() and std::is_const<DataType>()) or not std::is_const<DataType2>()),
-                  "Cannot assign SubArray of const to SubArray of non-const");
-
-    m_array      = sub_array.m_array;
-    m_size       = sub_array.m_size;
-    m_sub_values = sub_array.m_sub_values;
-
-    return *this;
-  }
-
   PUGS_INLINE
-  SubArray& operator=(const SubArray&) = default;
+  SubArray& operator=(const SubArray&) = delete;
 
   PUGS_INLINE
-  SubArray& operator=(SubArray&&) = default;
+  SubArray& operator=(SubArray&&) = delete;
 
   PUGS_INLINE
-  explicit SubArray(Array<DataType> array, size_t begin, size_t size)
-    : m_array{array}, m_sub_values{&array[0] + begin}, m_size{size}
+  explicit SubArray(const Array<DataType>& array, size_t begin, size_t size)
+    : m_sub_values{&array[0] + begin}, m_size{size}
   {
     Assert(begin + size <= array.size(), "SubView is not contained in the source Array");
   }
@@ -81,16 +61,10 @@ class [[nodiscard]] SubArray
   SubArray() = delete;
 
   PUGS_INLINE
-  SubArray(const SubArray&) = default;
-
-  template <typename DataType2>
-  PUGS_INLINE SubArray(const SubArray<DataType2>& sub_array) noexcept
-  {
-    this->operator=(sub_array);
-  }
+  SubArray(const SubArray&) = delete;
 
   PUGS_INLINE
-  SubArray(SubArray &&) = default;
+  SubArray(SubArray &&) = delete;
 
   PUGS_INLINE
   ~SubArray() = default;
diff --git a/tests/test_SubArray.cpp b/tests/test_SubArray.cpp
index 647dc259e..b0fdec788 100644
--- a/tests/test_SubArray.cpp
+++ b/tests/test_SubArray.cpp
@@ -51,52 +51,11 @@ TEST_CASE("SubArray", "[utils]")
     REQUIRE(((sub_a[0] == 66) and (sub_a[1] == 91) and (sub_a[2] == 120) and (sub_a[3] == 153) and (sub_a[4] == 190)));
   }
 
-  SECTION("sub array copy")
-  {
-    a.fill(0);
-    SubArray<int> sub_a{a, 2, 1};
-    sub_a = SubArray{a, 5, 5};
-    for (size_t i = 0; i < sub_a.size(); ++i) {
-      sub_a[i] = i + 1;
-    }
-
-    REQUIRE(((a[0] == 0) and (a[1] == 0) and (a[2] == 0) and (a[3] == 0) and (a[4] == 0) and (a[5] == 1) and
-             (a[6] == 2) and (a[7] == 3) and (a[8] == 4) and (a[9] == 5)));
-
-    for (size_t i = 0; i < a.size(); ++i) {
-      a[i] = (i + 1) * (2 * i + 1);
-    }
-
-    REQUIRE(((sub_a[0] == 66) and (sub_a[1] == 91) and (sub_a[2] == 120) and (sub_a[3] == 153) and (sub_a[4] == 190)));
-
-    SubArray sub_b = sub_a;
-
-    REQUIRE(((sub_b[0] == 66) and (sub_b[1] == 91) and (sub_b[2] == 120) and (sub_b[3] == 153) and (sub_b[4] == 190)));
-
-    sub_b = sub_a;
-
-    REQUIRE(((sub_b[0] == 66) and (sub_b[1] == 91) and (sub_b[2] == 120) and (sub_b[3] == 153) and (sub_b[4] == 190)));
-
-    SubArray<const int> const_sub_a{sub_a};
-
-    REQUIRE(((const_sub_a[0] == 66) and (const_sub_a[1] == 91) and (const_sub_a[2] == 120) and
-             (const_sub_a[3] == 153) and (const_sub_a[4] == 190)));
-
-    sub_a.fill(2);
-    REQUIRE(((const_sub_a[0] == 2) and (const_sub_a[1] == 2) and (const_sub_a[2] == 2) and (const_sub_a[3] == 2) and
-             (const_sub_a[4] == 2)));
-
-    SubArray sub_c{SubArray{a, 3, 6}};
-    REQUIRE(((sub_c[0] == a[3]) and (sub_c[1] == a[4]) and (sub_c[2] == a[5]) and (sub_c[3] == a[6]) and
-             (sub_c[4] == a[7]) and (sub_c[5] == a[8])));
-  }
-
 #ifndef NDEBUG
   SECTION("errors")
   {
     a.fill(0);
-    SubArray<int> sub_a{a, 0, 0};
-    sub_a = SubArray{a, 5, 5};
+    SubArray<int> sub_a{a, 5, 5};
     for (size_t i = 0; i < sub_a.size(); ++i) {
       sub_a[i] = i + 1;
     }
-- 
GitLab