diff options
author | Jacques Lucke <jacques@blender.org> | 2020-08-19 17:44:53 +0300 |
---|---|---|
committer | Jacques Lucke <jacques@blender.org> | 2020-08-19 17:44:59 +0300 |
commit | 2aff45146f1464ba8899368ad004522cb6a1a98c (patch) | |
tree | 9841430147c351566bebeb2e0460efb534f2ebe3 /source/blender/blenlib/tests | |
parent | aeaf2b0dd437f1a03ed30142678cbf44d0414ea1 (diff) |
BLI: improve exception safety of Vector, Array and Stack
Using C++ exceptions in Blender is difficult, due to the large
number of C functions in the call stack. However, C++ data
structures in blenlib should at least try to be exception safe,
so that they can be used if someone wants to use exceptions
in some isolated area.
This patch improves the exception safety of the Vector, Array
and Stack data structure. This is mainly achieved by reordering
some lines and doing some explicit exception handling.
I don't expect performance of common operations to be affected
by this change.
The three containers are supposed to provide at least the
basic exception guarantee for most methods (except for e.g.
`*_unchecked` methods). So, resources should not leak when
the contained type throws an exception.
I also added new unit tests that test the exception handling
in various cases.
Diffstat (limited to 'source/blender/blenlib/tests')
5 files changed, 241 insertions, 0 deletions
diff --git a/source/blender/blenlib/tests/BLI_array_test.cc b/source/blender/blenlib/tests/BLI_array_test.cc index 38ab695d238..251cff833f7 100644 --- a/source/blender/blenlib/tests/BLI_array_test.cc +++ b/source/blender/blenlib/tests/BLI_array_test.cc @@ -1,6 +1,7 @@ /* Apache License, Version 2.0 */ #include "BLI_array.hh" +#include "BLI_exception_safety_test_utils.hh" #include "BLI_strict_flags.h" #include "BLI_vector.hh" #include "testing/testing.h" @@ -188,4 +189,42 @@ TEST(array, ReverseIterator) EXPECT_EQ_ARRAY(array.data(), Span({13, 14, 15, 16}).data(), 4); } +TEST(array, SpanConstructorExceptions) +{ + std::array<ExceptionThrower, 4> values; + values[2].throw_during_copy = true; + Span<ExceptionThrower> span{values}; + EXPECT_ANY_THROW({ Array<ExceptionThrower> array(span); }); +} + +TEST(array, SizeValueConstructorExceptions) +{ + ExceptionThrower value; + value.throw_during_copy = true; + EXPECT_ANY_THROW({ Array<ExceptionThrower> array(5, value); }); +} + +TEST(array, MoveConstructorExceptions) +{ + Array<ExceptionThrower, 4> array(3); + array[1].throw_during_move = true; + EXPECT_ANY_THROW({ Array<ExceptionThrower> array_copy(std::move(array)); }); +} + +TEST(array, CopyAssignmentExceptions) +{ + Array<ExceptionThrower> array(5); + array[3].throw_during_copy = true; + Array<ExceptionThrower> array_copy(10); + EXPECT_ANY_THROW({ array_copy = array; }); +} + +TEST(array, MoveAssignmentExceptions) +{ + Array<ExceptionThrower, 4> array(4); + array[2].throw_during_move = true; + Array<ExceptionThrower> array_moved(10); + EXPECT_ANY_THROW({ array_moved = std::move(array); }); +} + } // 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 new file mode 100644 index 00000000000..1e85dd4bab2 --- /dev/null +++ b/source/blender/blenlib/tests/BLI_exception_safety_test_utils.hh @@ -0,0 +1,60 @@ +#include "BLI_utildefines.h" +#include "testing/testing.h" + +namespace blender::tests { + +struct ExceptionThrower { + static constexpr uint32_t is_alive_state = 0x21254634; + static constexpr uint32_t is_destructed_state = 0xFA4BC327; + uint32_t state; + bool throw_during_copy; + bool throw_during_move; + + ExceptionThrower() : state(is_alive_state), throw_during_copy(false), throw_during_move(false) + { + } + + ExceptionThrower(const ExceptionThrower &other) + : state(is_alive_state), throw_during_copy(false), throw_during_move(false) + { + EXPECT_EQ(other.state, is_alive_state); + if (other.throw_during_copy) { + throw std::runtime_error("throwing during copy, as requested"); + } + } + + ExceptionThrower(ExceptionThrower &&other) + : state(is_alive_state), throw_during_copy(false), throw_during_move(false) + { + EXPECT_EQ(other.state, is_alive_state); + if (other.throw_during_move) { + throw std::runtime_error("throwing during move, as requested"); + } + } + + ExceptionThrower &operator=(const ExceptionThrower &other) + { + EXPECT_EQ(other.state, is_alive_state); + if (throw_during_copy || other.throw_during_copy) { + throw std::runtime_error("throwing during copy, as requested"); + } + return *this; + } + + ExceptionThrower &operator=(ExceptionThrower &&other) + { + EXPECT_EQ(other.state, is_alive_state); + if (throw_during_move || other.throw_during_move) { + throw std::runtime_error("throwing during move, as requested"); + } + return *this; + } + + ~ExceptionThrower() + { + EXPECT_EQ(state, is_alive_state); + state = is_destructed_state; + } +}; + +} // namespace blender::tests diff --git a/source/blender/blenlib/tests/BLI_memory_utils_test.cc b/source/blender/blenlib/tests/BLI_memory_utils_test.cc index f3cb02b63d7..fcef2f8688a 100644 --- a/source/blender/blenlib/tests/BLI_memory_utils_test.cc +++ b/source/blender/blenlib/tests/BLI_memory_utils_test.cc @@ -7,6 +7,7 @@ namespace blender::tests { +namespace { struct MyValue { static inline int alive = 0; @@ -33,6 +34,7 @@ struct MyValue { alive--; } }; +} // namespace TEST(memory_utils, DefaultConstructN_ActuallyCallsConstructor) { diff --git a/source/blender/blenlib/tests/BLI_stack_cxx_test.cc b/source/blender/blenlib/tests/BLI_stack_cxx_test.cc index 3572e751b88..c03893c5596 100644 --- a/source/blender/blenlib/tests/BLI_stack_cxx_test.cc +++ b/source/blender/blenlib/tests/BLI_stack_cxx_test.cc @@ -1,5 +1,6 @@ /* Apache License, Version 2.0 */ +#include "BLI_exception_safety_test_utils.hh" #include "BLI_stack.hh" #include "BLI_strict_flags.h" #include "BLI_vector.hh" @@ -185,4 +186,59 @@ TEST(stack, OveralignedValues) } } +TEST(stack, SpanConstructorExceptions) +{ + std::array<ExceptionThrower, 5> values; + values[3].throw_during_copy = true; + EXPECT_ANY_THROW({ Stack<ExceptionThrower> stack(values); }); +} + +TEST(stack, MoveConstructorExceptions) +{ + Stack<ExceptionThrower, 4> stack; + stack.push({}); + stack.push({}); + stack.peek().throw_during_move = true; + EXPECT_ANY_THROW({ Stack<ExceptionThrower> moved_stack{std::move(stack)}; }); +} + +TEST(stack, PushExceptions) +{ + Stack<ExceptionThrower, 2> stack; + stack.push({}); + stack.push({}); + ExceptionThrower *ptr1 = &stack.peek(); + ExceptionThrower value; + value.throw_during_copy = true; + EXPECT_ANY_THROW({ stack.push(value); }); + EXPECT_EQ(stack.size(), 2); + ExceptionThrower *ptr2 = &stack.peek(); + EXPECT_EQ(ptr1, ptr2); + EXPECT_TRUE(stack.is_invariant_maintained()); +} + +TEST(stack, PopExceptions) +{ + Stack<ExceptionThrower> stack; + stack.push({}); + stack.peek().throw_during_move = true; + stack.push({}); + stack.pop(); /* NOLINT: bugprone-throw-keyword-missing */ + EXPECT_ANY_THROW({ stack.pop(); }); /* NOLINT: bugprone-throw-keyword-missing */ + EXPECT_EQ(stack.size(), 1); + EXPECT_TRUE(stack.is_invariant_maintained()); +} + +TEST(stack, PushMultipleExceptions) +{ + Stack<ExceptionThrower> stack; + stack.push({}); + std::array<ExceptionThrower, 100> values; + values[6].throw_during_copy = true; + EXPECT_ANY_THROW({ stack.push_multiple(values); }); + EXPECT_TRUE(stack.is_invariant_maintained()); + EXPECT_ANY_THROW({ stack.push_multiple(values); }); + EXPECT_TRUE(stack.is_invariant_maintained()); +} + } // namespace blender::tests diff --git a/source/blender/blenlib/tests/BLI_vector_test.cc b/source/blender/blenlib/tests/BLI_vector_test.cc index 792e120d2c0..056a7aa3924 100644 --- a/source/blender/blenlib/tests/BLI_vector_test.cc +++ b/source/blender/blenlib/tests/BLI_vector_test.cc @@ -1,5 +1,6 @@ /* Apache License, Version 2.0 */ +#include "BLI_exception_safety_test_utils.hh" #include "BLI_strict_flags.h" #include "BLI_vector.hh" #include "testing/testing.h" @@ -709,4 +710,87 @@ TEST(vector, ReverseIterator) EXPECT_EQ_ARRAY(reversed_vec.data(), Span({7, 6, 5, 4}).data(), 4); } +TEST(vector, SizeValueConstructorExceptions) +{ + ExceptionThrower value; + value.throw_during_copy = true; + EXPECT_ANY_THROW({ Vector<ExceptionThrower> vec(5, value); }); +} + +TEST(vector, SpanConstructorExceptions) +{ + std::array<ExceptionThrower, 5> values; + values[3].throw_during_copy = true; + EXPECT_ANY_THROW({ Vector<ExceptionThrower> vec(values); }); +} + +TEST(vector, MoveConstructorExceptions) +{ + Vector<ExceptionThrower, 4> vec(3); + vec[2].throw_during_move = true; + EXPECT_ANY_THROW({ Vector<ExceptionThrower> moved_vector{std::move(vec)}; }); +} + +TEST(vector, AppendExceptions) +{ + Vector<ExceptionThrower, 4> vec(2); + ExceptionThrower *ptr1 = &vec.last(); + ExceptionThrower value; + value.throw_during_copy = true; + EXPECT_ANY_THROW({ vec.append(value); }); + EXPECT_EQ(vec.size(), 2); + ExceptionThrower *ptr2 = &vec.last(); + EXPECT_EQ(ptr1, ptr2); +} + +TEST(vector, ExtendExceptions) +{ + Vector<ExceptionThrower> vec(5); + std::array<ExceptionThrower, 10> values; + values[6].throw_during_copy = true; + EXPECT_ANY_THROW({ vec.extend(values); }); + EXPECT_EQ(vec.size(), 5); +} + +TEST(vector, Insert1Exceptions) +{ + Vector<ExceptionThrower> vec(10); + std::array<ExceptionThrower, 5> values; + values[3].throw_during_copy = true; + EXPECT_ANY_THROW({ vec.insert(7, values); }); +} + +TEST(vector, Insert2Exceptions) +{ + Vector<ExceptionThrower> vec(10); + vec.reserve(100); + vec[8].throw_during_move = true; + std::array<ExceptionThrower, 5> values; + EXPECT_ANY_THROW({ vec.insert(3, values); }); +} + +TEST(vector, PopLastExceptions) +{ + Vector<ExceptionThrower> vec(10); + vec.last().throw_during_move = true; + EXPECT_ANY_THROW({ vec.pop_last(); }); /* NOLINT: bugprone-throw-keyword-missing */ + EXPECT_EQ(vec.size(), 10); +} + +TEST(vector, RemoveAndReorderExceptions) +{ + Vector<ExceptionThrower> vec(10); + vec.last().throw_during_move = true; + EXPECT_ANY_THROW({ vec.remove_and_reorder(3); }); + EXPECT_EQ(vec.size(), 10); +} + +TEST(vector, RemoveExceptions) +{ + Vector<ExceptionThrower> vec(10); + vec[8].throw_during_move = true; + EXPECT_ANY_THROW({ vec.remove(2); }); + EXPECT_EQ(vec.size(), 10); +} + } // namespace blender::tests |