BLI: improve exception safety of Set and Map

For more information see rB2aff45146f1464ba8899368ad004522cb6a1a98c.

4 files changed, 168 insertions, 7 deletions

diff --git a/source/blender/blenlib/tests/BLI_array_test.cc b/source/blender/blenlib/tests/BLI_array_test.cc
index 3d45a9f5277..7d967eca87e 100644
--- a/source/blender/blenlib/tests/BLI_array_test.cc
+++ b/source/blender/blenlib/tests/BLI_array_test.cc
@@ -236,4 +236,18 @@ TEST(array, Last)
   EXPECT_EQ(const_cast<const Array<int> &>(array).last(), 1);
 }
 
+TEST(array, Reinitialize)
+{
+  Array<std::string> array = {"hello", "world"};
+  EXPECT_EQ(array.size(), 2);
+  EXPECT_EQ(array[1], "world");
+  array.reinitialize(3);
+  EXPECT_EQ(array.size(), 3);
+  EXPECT_EQ(array[0], "");
+  EXPECT_EQ(array[1], "");
+  EXPECT_EQ(array[2], "");
+  array.reinitialize(0);
+  EXPECT_EQ(array.size(), 0);
+}
+
 }  // namespace blender::tests
diff --git a/source/blender/blenlib/tests/BLI_exception_safety_test_utils.hh b/source/blender/blenlib/tests/BLI_exception_safety_test_utils.hh
index 5ad7674396b..91270767a25 100644
--- a/source/blender/blenlib/tests/BLI_exception_safety_test_utils.hh
+++ b/source/blender/blenlib/tests/BLI_exception_safety_test_utils.hh
@@ -1,3 +1,4 @@
+#include "BLI_hash.hh"
 #include "BLI_utildefines.h"
 #include "MEM_guardedalloc.h"
 #include "testing/testing.h"
@@ -16,18 +17,21 @@ class ExceptionThrower {
   void *my_memory_;
 
  public:
-  bool throw_during_copy;
-  bool throw_during_move;
+  mutable bool throw_during_copy;
+  mutable bool throw_during_move;
+  /* Used for hashing and comparing. */
+  int value;
 
-  ExceptionThrower()
+  ExceptionThrower(int value = 0)
       : state_(is_alive_state),
         my_memory_(MEM_mallocN(1, AT)),
         throw_during_copy(false),
-        throw_during_move(false)
+        throw_during_move(false),
+        value(value)
   {
   }
 
-  ExceptionThrower(const ExceptionThrower &other) : ExceptionThrower()
+  ExceptionThrower(const ExceptionThrower &other) : ExceptionThrower(other.value)
   {
     EXPECT_EQ(other.state_, is_alive_state);
     if (other.throw_during_copy) {
@@ -35,7 +39,7 @@ class ExceptionThrower {
     }
   }
 
-  ExceptionThrower(ExceptionThrower &&other) : ExceptionThrower()
+  ExceptionThrower(ExceptionThrower &&other) : ExceptionThrower(other.value)
   {
     EXPECT_EQ(other.state_, is_alive_state);
     if (other.throw_during_move) {
@@ -49,6 +53,7 @@ class ExceptionThrower {
     if (throw_during_copy || other.throw_during_copy) {
       throw std::runtime_error("throwing during copy, as requested");
     }
+    value = other.value;
     return *this;
   }
 
@@ -58,15 +63,40 @@ class ExceptionThrower {
     if (throw_during_move || other.throw_during_move) {
       throw std::runtime_error("throwing during move, as requested");
     }
+    value = other.value;
     return *this;
   }
 
   ~ExceptionThrower()
   {
-    EXPECT_EQ(state_, is_alive_state);
+    const char *message = "";
+    if (state_ != is_alive_state) {
+      if (state_ == is_destructed_state) {
+        message = "Trying to destruct an already destructed instance.";
+      }
+      else {
+        message = "Trying to destruct an uninitialized instance.";
+      }
+    }
+    EXPECT_EQ(state_, is_alive_state) << message;
     state_ = is_destructed_state;
     MEM_freeN(my_memory_);
   }
+
+  uint64_t hash() const
+  {
+    return static_cast<uint64_t>(value);
+  }
+
+  friend bool operator==(const ExceptionThrower &a, const ExceptionThrower &b)
+  {
+    return a.value == b.value;
+  }
+
+  friend bool operator!=(const ExceptionThrower &a, const ExceptionThrower &b)
+  {
+    return !(a == b);
+  }
 };
 
 }  // namespace blender::tests
diff --git a/source/blender/blenlib/tests/BLI_map_test.cc b/source/blender/blenlib/tests/BLI_map_test.cc
index fe7b0f01279..7b4a484e736 100644
--- a/source/blender/blenlib/tests/BLI_map_test.cc
+++ b/source/blender/blenlib/tests/BLI_map_test.cc
@@ -1,5 +1,6 @@
 /* Apache License, Version 2.0 */
 
+#include "BLI_exception_safety_test_utils.hh"
 #include "BLI_map.hh"
 #include "BLI_rand.h"
 #include "BLI_set.hh"
@@ -479,6 +480,72 @@ TEST(map, ForeachItem)
   EXPECT_EQ(keys.first_index_of(1), values.first_index_of(8));
 }
 
+TEST(map, CopyConstructorExceptions)
+{
+  using MapType = Map<ExceptionThrower, ExceptionThrower>;
+  MapType map;
+  map.add(2, 2);
+  map.add(4, 4);
+  map.lookup(2).throw_during_copy = true;
+  EXPECT_ANY_THROW({ MapType map_copy(map); });
+}
+
+TEST(map, MoveConstructorExceptions)
+{
+  using MapType = Map<ExceptionThrower, ExceptionThrower>;
+  MapType map;
+  map.add(1, 1);
+  map.add(2, 2);
+  map.lookup(1).throw_during_move = true;
+  EXPECT_ANY_THROW({ MapType map_moved(std::move(map)); });
+  map.add(5, 5);
+}
+
+TEST(map, AddNewExceptions)
+{
+  Map<ExceptionThrower, ExceptionThrower> map;
+  ExceptionThrower key1 = 1;
+  key1.throw_during_copy = true;
+  ExceptionThrower value1;
+  EXPECT_ANY_THROW({ map.add_new(key1, value1); });
+  EXPECT_EQ(map.size(), 0);
+  ExceptionThrower key2 = 2;
+  ExceptionThrower value2;
+  value2.throw_during_copy = true;
+  EXPECT_ANY_THROW({ map.add_new(key2, value2); });
+}
+
+TEST(map, ReserveExceptions)
+{
+  Map<ExceptionThrower, ExceptionThrower> map;
+  map.add(3, 3);
+  map.add(5, 5);
+  map.add(2, 2);
+  map.lookup(2).throw_during_move = true;
+  EXPECT_ANY_THROW({ map.reserve(100); });
+  map.add(1, 1);
+  map.add(5, 5);
+}
+
+TEST(map, PopExceptions)
+{
+  Map<ExceptionThrower, ExceptionThrower> map;
+  map.add(3, 3);
+  map.lookup(3).throw_during_move = true;
+  EXPECT_ANY_THROW({ map.pop(3); });
+  EXPECT_EQ(map.size(), 1);
+  map.add(1, 1);
+  EXPECT_EQ(map.size(), 2);
+}
+
+TEST(map, AddOrModifyExceptions)
+{
+  Map<ExceptionThrower, ExceptionThrower> map;
+  auto create_fn = [](ExceptionThrower *UNUSED(v)) { throw std::runtime_error(""); };
+  auto modify_fn = [](ExceptionThrower *UNUSED(v)) {};
+  EXPECT_ANY_THROW({ map.add_or_modify(3, create_fn, modify_fn); });
+}
+
 /**
  * Set this to 1 to activate the benchmark. It is disabled by default, because it prints a lot.
  */
diff --git a/source/blender/blenlib/tests/BLI_set_test.cc b/source/blender/blenlib/tests/BLI_set_test.cc
index df3f7ab544c..3ea9a59b3db 100644
--- a/source/blender/blenlib/tests/BLI_set_test.cc
+++ b/source/blender/blenlib/tests/BLI_set_test.cc
@@ -3,6 +3,7 @@
 #include <set>
 #include <unordered_set>
 
+#include "BLI_exception_safety_test_utils.hh"
 #include "BLI_ghash.h"
 #include "BLI_rand.h"
 #include "BLI_set.hh"
@@ -462,6 +463,55 @@ TEST(set, StringViewKeys)
   EXPECT_TRUE(set.contains("hello"));
 }
 
+TEST(set, SpanConstructorExceptions)
+{
+  std::array<ExceptionThrower, 5> array = {1, 2, 3, 4, 5};
+  array[3].throw_during_copy = true;
+  Span<ExceptionThrower> span = array;
+
+  EXPECT_ANY_THROW({ Set<ExceptionThrower> set(span); });
+}
+
+TEST(set, CopyConstructorExceptions)
+{
+  Set<ExceptionThrower> set = {1, 2, 3, 4, 5};
+  set.lookup_key(3).throw_during_copy = true;
+  EXPECT_ANY_THROW({ Set<ExceptionThrower> set_copy(set); });
+}
+
+TEST(set, MoveConstructorExceptions)
+{
+  using SetType = Set<ExceptionThrower, 4>;
+  SetType set = {1, 2, 3};
+  set.lookup_key(2).throw_during_move = true;
+  EXPECT_ANY_THROW({ SetType set_moved(std::move(set)); });
+  EXPECT_EQ(set.size(), 0);
+  set.add_multiple({3, 6, 7});
+  EXPECT_EQ(set.size(), 3);
+}
+
+TEST(set, AddNewExceptions)
+{
+  Set<ExceptionThrower> set;
+  ExceptionThrower value;
+  value.throw_during_copy = true;
+  EXPECT_ANY_THROW({ set.add_new(value); });
+  EXPECT_EQ(set.size(), 0);
+  EXPECT_ANY_THROW({ set.add_new(value); });
+  EXPECT_EQ(set.size(), 0);
+}
+
+TEST(set, AddExceptions)
+{
+  Set<ExceptionThrower> set;
+  ExceptionThrower value;
+  value.throw_during_copy = true;
+  EXPECT_ANY_THROW({ set.add(value); });
+  EXPECT_EQ(set.size(), 0);
+  EXPECT_ANY_THROW({ set.add(value); });
+  EXPECT_EQ(set.size(), 0);
+}
+
 /**
  * Set this to 1 to activate the benchmark. It is disabled by default, because it prints a lot.
  */