From 8e18a9984505514a229d66b38fff31d930367968 Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Mon, 24 Aug 2020 17:24:13 +0200 Subject: BLI: improve exception safety of Set and Map For more information see rB2aff45146f1464ba8899368ad004522cb6a1a98c. --- source/blender/blenlib/tests/BLI_array_test.cc | 14 +++++ .../tests/BLI_exception_safety_test_utils.hh | 44 +++++++++++--- source/blender/blenlib/tests/BLI_map_test.cc | 67 ++++++++++++++++++++++ source/blender/blenlib/tests/BLI_set_test.cc | 50 ++++++++++++++++ 4 files changed, 168 insertions(+), 7 deletions(-) (limited to 'source/blender/blenlib/tests') 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 &>(array).last(), 1); } +TEST(array, Reinitialize) +{ + Array 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(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; + 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; + 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 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 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 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 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 #include +#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 array = {1, 2, 3, 4, 5}; + array[3].throw_during_copy = true; + Span span = array; + + EXPECT_ANY_THROW({ Set set(span); }); +} + +TEST(set, CopyConstructorExceptions) +{ + Set set = {1, 2, 3, 4, 5}; + set.lookup_key(3).throw_during_copy = true; + EXPECT_ANY_THROW({ Set set_copy(set); }); +} + +TEST(set, MoveConstructorExceptions) +{ + using SetType = Set; + 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 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 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. */ -- cgit v1.2.3