[Bf-blender-cvs] [a45284b855d] master: BLI: remove deduplicated memory utility functions

Jacques Lucke noreply at git.blender.org
Fri Dec 9 14:15:54 CET 2022


Commit: a45284b855d4fef54d91412cdf169dd721110db1
Author: Jacques Lucke
Date:   Fri Dec 9 14:10:15 2022 +0100
Branches: master
https://developer.blender.org/rBa45284b855d4fef54d91412cdf169dd721110db1

BLI: remove deduplicated memory utility functions

These functions were originally implemented because:
- Not all of them existed pre C++17, but now we are using C++17.
- The call stack depth is quite a bit deeper with the std functions, making
  debugging slower and more annoying. I didn't find this to be a problem
  anymore recently.

No functional changes are expected.

===================================================================

M	source/blender/blenlib/BLI_memory_utils.hh
M	source/blender/blenlib/tests/BLI_memory_utils_test.cc

===================================================================

diff --git a/source/blender/blenlib/BLI_memory_utils.hh b/source/blender/blenlib/BLI_memory_utils.hh
index b6ffa6d8b4a..6f119a49f01 100644
--- a/source/blender/blenlib/BLI_memory_utils.hh
+++ b/source/blender/blenlib/BLI_memory_utils.hh
@@ -4,11 +4,9 @@
 
 /** \file
  * \ingroup bli
- * Some of the functions below have very similar alternatives in the standard library. However, it
- * is rather annoying to use those when debugging. Therefore, some more specialized and easier to
- * debug functions are provided here.
  */
 
+#include <algorithm>
 #include <memory>
 #include <new>
 #include <type_traits>
@@ -33,280 +31,66 @@ template<typename T>
 inline constexpr bool is_trivially_move_constructible_extended_v =
     is_trivial_extended_v<T> || std::is_trivially_move_constructible_v<T>;
 
-/**
- * Call the destructor on n consecutive values. For trivially destructible types, this does
- * nothing.
- *
- * Exception Safety: Destructors shouldn't throw exceptions.
- *
- * Before:
- *  ptr: initialized
- * After:
- *  ptr: uninitialized
- */
 template<typename T> void destruct_n(T *ptr, int64_t n)
 {
-  BLI_assert(n >= 0);
-
-  static_assert(std::is_nothrow_destructible_v<T>,
-                "This should be true for all types. Destructors are noexcept by default.");
-
-  /* This is not strictly necessary, because the loop below will be optimized away anyway. It is
-   * nice to make behavior this explicitly, though. */
   if (is_trivially_destructible_extended_v<T>) {
     return;
   }
 
-  for (int64_t i = 0; i < n; i++) {
-    ptr[i].~T();
-  }
+  std::destroy_n(ptr, n);
 }
 
-/**
- * Call the default constructor on n consecutive elements. For trivially constructible types, this
- * does nothing.
- *
- * Exception Safety: Strong.
- *
- * Before:
- *  ptr: uninitialized
- * After:
- *  ptr: initialized
- */
 template<typename T> void default_construct_n(T *ptr, int64_t n)
 {
-  BLI_assert(n >= 0);
-
-  /* This is not strictly necessary, because the loop below will be optimized away anyway. It is
-   * nice to make behavior this explicitly, though. */
-  if (std::is_trivially_constructible_v<T>) {
-    return;
-  }
-
-  int64_t current = 0;
-  try {
-    for (; current < n; current++) {
-      new (static_cast<void *>(ptr + current)) T;
-    }
-  }
-  catch (...) {
-    destruct_n(ptr, current);
-    throw;
-  }
+  std::uninitialized_default_construct_n(ptr, n);
 }
 
-/**
- * Copy n values from src to dst.
- *
- * Exception Safety: Basic.
- *
- * Before:
- *  src: initialized
- *  dst: initialized
- * After:
- *  src: initialized
- *  dst: initialized
- */
 template<typename T> void initialized_copy_n(const T *src, int64_t n, T *dst)
 {
-  BLI_assert(n >= 0);
-
-  for (int64_t i = 0; i < n; i++) {
-    dst[i] = src[i];
-  }
+  std::copy_n(src, n, dst);
 }
 
-/**
- * Copy n values from src to dst.
- *
- * Exception Safety: Strong.
- *
- * Before:
- *  src: initialized
- *  dst: uninitialized
- * After:
- *  src: initialized
- *  dst: initialized
- */
 template<typename T> void uninitialized_copy_n(const T *src, int64_t n, T *dst)
 {
-  BLI_assert(n >= 0);
-
-  int64_t current = 0;
-  try {
-    for (; current < n; current++) {
-      new (static_cast<void *>(dst + current)) T(src[current]);
-    }
-  }
-  catch (...) {
-    destruct_n(dst, current);
-    throw;
-  }
+  std::uninitialized_copy_n(src, n, dst);
 }
 
-/**
- * Convert n values from type `From` to type `To`.
- *
- * Exception Safety: Strong.
- *
- * Before:
- *  src: initialized
- *  dst: uninitialized
- * After:
- *  src: initialized
- *  dst: initialized
- */
 template<typename From, typename To>
 void uninitialized_convert_n(const From *src, int64_t n, To *dst)
 {
-  BLI_assert(n >= 0);
-
-  int64_t current = 0;
-  try {
-    for (; current < n; current++) {
-      new (static_cast<void *>(dst + current)) To(static_cast<To>(src[current]));
-    }
-  }
-  catch (...) {
-    destruct_n(dst, current);
-    throw;
-  }
+  std::uninitialized_copy_n(src, n, dst);
 }
 
-/**
- * Move n values from src to dst.
- *
- * Exception Safety: Basic.
- *
- * Before:
- *  src: initialized
- *  dst: initialized
- * After:
- *  src: initialized, moved-from
- *  dst: initialized
- */
 template<typename T> void initialized_move_n(T *src, int64_t n, T *dst)
 {
-  BLI_assert(n >= 0);
-
-  for (int64_t i = 0; i < n; i++) {
-    dst[i] = std::move(src[i]);
-  }
+  std::copy_n(std::make_move_iterator(src), n, dst);
 }
 
-/**
- * Move n values from src to dst.
- *
- * Exception Safety: Basic.
- *
- * Before:
- *  src: initialized
- *  dst: uninitialized
- * After:
- *  src: initialized, moved-from
- *  dst: initialized
- */
 template<typename T> void uninitialized_move_n(T *src, int64_t n, T *dst)
 {
-  BLI_assert(n >= 0);
-
-  int64_t current = 0;
-  try {
-    for (; current < n; current++) {
-      new (static_cast<void *>(dst + current)) T(std::move(src[current]));
-    }
-  }
-  catch (...) {
-    destruct_n(dst, current);
-    throw;
-  }
+  std::uninitialized_copy_n(std::make_move_iterator(src), n, dst);
 }
 
-/**
- * Relocate n values from src to dst. Relocation is a move followed by destruction of the src
- * value.
- *
- * Exception Safety: Basic.
- *
- * Before:
- *  src: initialized
- *  dst: initialized
- * After:
- *  src: uninitialized
- *  dst: initialized
- */
 template<typename T> void initialized_relocate_n(T *src, int64_t n, T *dst)
 {
-  BLI_assert(n >= 0);
-
   initialized_move_n(src, n, dst);
   destruct_n(src, n);
 }
 
-/**
- * Relocate n values from src to dst. Relocation is a move followed by destruction of the src
- * value.
- *
- * Exception Safety: Basic.
- *
- * Before:
- *  src: initialized
- *  dst: uninitialized
- * After:
- *  src: uninitialized
- *  dst: initialized
- */
 template<typename T> void uninitialized_relocate_n(T *src, int64_t n, T *dst)
 {
-  BLI_assert(n >= 0);
-
   uninitialized_move_n(src, n, dst);
   destruct_n(src, n);
 }
 
-/**
- * Copy the value to n consecutive elements.
- *
- * Exception Safety: Basic.
- *
- * Before:
- *  dst: initialized
- * After:
- *  dst: initialized
- */
 template<typename T> void initialized_fill_n(T *dst, int64_t n, const T &value)
 {
-  BLI_assert(n >= 0);
-
-  for (int64_t i = 0; i < n; i++) {
-    dst[i] = value;
-  }
+  std::fill_n(dst, n, value);
 }
 
-/**
- * Copy the value to n consecutive elements.
- *
- *  Exception Safety: Strong.
- *
- * Before:
- *  dst: uninitialized
- * After:
- *  dst: initialized
- */
 template<typename T> void uninitialized_fill_n(T *dst, int64_t n, const T &value)
 {
-  BLI_assert(n >= 0);
-
-  int64_t current = 0;
-  try {
-    for (; current < n; current++) {
-      new (static_cast<void *>(dst + current)) T(value);
-    }
-  }
-  catch (...) {
-    destruct_n(dst, current);
-    throw;
-  }
+  std::uninitialized_fill_n(dst, n, value);
 }
 
 template<typename T> struct DestructValueAtAddress {
diff --git a/source/blender/blenlib/tests/BLI_memory_utils_test.cc b/source/blender/blenlib/tests/BLI_memory_utils_test.cc
index 939ac6151b0..3596de5c179 100644
--- a/source/blender/blenlib/tests/BLI_memory_utils_test.cc
+++ b/source/blender/blenlib/tests/BLI_memory_utils_test.cc
@@ -7,121 +7,6 @@
 
 namespace blender::tests {
 
-namespace {
-struct MyValue {
-  static inline int alive = 0;
-
-  MyValue()
-  {
-    if (alive == 15) {
-      throw std::exception();
-    }
-
-    alive++;
-  }
-
-  MyValue(const MyValue & /*other*/)
-  {
-    if (alive == 15) {
-      throw std::exception();
-    }
-
-    alive++;
-  }
-
-  ~MyValue()
-  {
-    alive--;
-  }
-};
-}  // namespace
-
-TEST(memory_utils, DefaultConstructN_ActuallyCallsConstructor)
-{
-  constexpr int amount = 10;
-  TypedBuffer<MyValue, amount> buffer;
-
-  EXPECT_EQ(MyValue::alive, 0);
-  default_construct_n(buffer.ptr(), amount);
-  EXPECT_EQ(MyValue::alive, amount);
-  destruct_n(buffer.ptr(), amount);
-  EXPECT_EQ(MyValue::alive, 0);
-}
-
-TEST(memory_utils, DefaultConstructN_StrongExceptionSafety)
-{
-  constexpr int amount = 20;
-  TypedBuffer<MyValue, amount> buffer;
-
-  EXPECT_EQ(MyValue::alive, 0);
-  EXPECT_THROW(default_construct_n(buffer.ptr(), amount), std::exception);
-  EXPECT_EQ(MyValue::alive, 0);
-}
-
-TEST(memory_utils, UninitializedCopyN_ActuallyCopies)
-{
-  constexpr int amount = 5;
-  TypedBuffer<MyValue, amount> buffer1;
-  TypedBuffer<MyValue, amount> buffer2;
-
-  EXPECT_EQ(MyValue::alive, 0);
-  default_construct_n(buffer1.ptr(), amount);
-  EXPECT_EQ(MyValue::alive, amount);
-  uninitialized_copy_n(buffer1.ptr(), amount, buffer2.ptr());
-  EXPECT_EQ(MyValue::alive, 2 * amount);
-  destruct_n(buffer1.ptr(), amount);
-  EXPECT_EQ(MyValue::alive, amount);
-  destruct_n(buffer2.ptr(), amount);
-  EXPECT_EQ(MyValue::alive, 0);
-}
-
-TEST(memory_utils, UninitializedCopyN_StrongExceptionSafety)
-{
-  constexpr int amount = 10;
-  TypedBuffer<MyValue, amount> buffer1;
-  TypedBuffer<MyValue, amount> buffer2;
-
-  EXPECT_EQ(MyValue::alive, 0);
-  default_construct_n(buffer1.ptr(), amount);
-  EXPECT_EQ(MyValue::alive, amount);
-  EXPECT_THROW(uninitialized_copy_n(buffer1.ptr(), amount, buffer2.ptr()), std::exception);
-  EXPECT_EQ(MyValue::alive, amount);
-  destruct_n(buffer1.ptr(), amount);
-  EXPECT_EQ(MyValue::alive, 0);
-}
-
-TEST(memory_utils, UninitializedFillN_ActuallyCopies)
-{
-  constexpr int amount = 10;
-  TypedBuffer<MyValue, amount> buffer;
-
-  EXPECT_EQ(MyValue::alive, 0);
-  {
-    MyValue value;
-    EXPECT_EQ(MyValue::alive, 1);
-    uninitialized_fill_n(buffer.ptr(), amount, value);
-    EXPECT_EQ(MyValue::alive, 1 + amount);
-    destruct_n(buffer.ptr(), amount);
-    EXPECT_EQ(MyValue::alive, 1);
-  }
-  EXPECT_EQ(MyValue::alive, 0);
-}
-
-TEST(memory_utils, UninitializedFillN_StrongExceptionSafety)
-{
-  constexpr int amount = 20;
-  TypedBuffer<MyValue, amount> buffer;
-
-  EXPECT_EQ(MyValue::alive, 0);
-  {
-    MyValue value;
-    EXPECT_EQ(MyValue::alive, 1);
-    EXPECT_THROW(uninitialized_fill_n(buffer.ptr(), amount, value), std::exception);
-    EXPECT_EQ(MyValue::alive, 1);
-  }
-  EXPECT_EQ(MyValue::alive, 0);
-}
-
 class TestBaseClass {
   virtual void mymethod(){};
 };



More information about the Bf-blender-cvs mailing list