diff options
| -rw-r--r-- | source/blender/blenlib/BLI_array.hh | 68 | ||||
| -rw-r--r-- | source/blender/blenlib/BLI_memory_utils.hh | 56 | ||||
| -rw-r--r-- | source/blender/blenlib/BLI_stack.hh | 95 | ||||
| -rw-r--r-- | source/blender/blenlib/BLI_vector.hh | 105 | ||||
| -rw-r--r-- | source/blender/blenlib/CMakeLists.txt | 2 | ||||
| -rw-r--r-- | source/blender/blenlib/tests/BLI_array_test.cc | 39 | ||||
| -rw-r--r-- | source/blender/blenlib/tests/BLI_exception_safety_test_utils.hh | 60 | ||||
| -rw-r--r-- | source/blender/blenlib/tests/BLI_memory_utils_test.cc | 2 | ||||
| -rw-r--r-- | source/blender/blenlib/tests/BLI_stack_cxx_test.cc | 56 | ||||
| -rw-r--r-- | source/blender/blenlib/tests/BLI_vector_test.cc | 84 |
10 files changed, 445 insertions, 122 deletions
diff --git a/source/blender/blenlib/BLI_array.hh b/source/blender/blenlib/BLI_array.hh index 9d09bb3559e..0ffe6b9a750 100644 --- a/source/blender/blenlib/BLI_array.hh +++ b/source/blender/blenlib/BLI_array.hh @@ -77,32 +77,39 @@ class Array { /** * By default an empty array is created. */ - Array() + Array(Allocator allocator = {}) noexcept : allocator_(allocator) { data_ = inline_buffer_; size_ = 0; } + Array(NoExceptConstructor, Allocator allocator = {}) noexcept : Array(allocator) + { + } + /** * Create a new array that contains copies of all values. */ template<typename U, typename std::enable_if_t<std::is_convertible_v<U, T>> * = nullptr> - Array(Span<U> values, Allocator allocator = {}) : allocator_(allocator) + Array(Span<U> values, Allocator allocator = {}) : Array(NoExceptConstructor(), allocator) { - size_ = values.size(); - data_ = this->get_buffer_for_size(values.size()); - uninitialized_convert_n<U, T>(values.data(), size_, data_); + const int64_t size = values.size(); + data_ = this->get_buffer_for_size(size); + uninitialized_convert_n<U, T>(values.data(), size, data_); + size_ = size; } /** * Create a new array that contains copies of all values. */ template<typename U, typename std::enable_if_t<std::is_convertible_v<U, T>> * = nullptr> - Array(const std::initializer_list<U> &values) : Array(Span<U>(values)) + Array(const std::initializer_list<U> &values, Allocator allocator = {}) + : Array(Span<U>(values), allocator) { } - Array(const std::initializer_list<T> &values) : Array(Span<T>(values)) + Array(const std::initializer_list<T> &values, Allocator allocator = {}) + : Array(Span<T>(values), allocator) { } @@ -114,23 +121,24 @@ class Array { * even for non-trivial types. This should not be the default though, because one can easily mess * up when dealing with uninitialized memory. */ - explicit Array(int64_t size) + explicit Array(int64_t size, Allocator allocator = {}) : Array(NoExceptConstructor(), allocator) { - size_ = size; data_ = this->get_buffer_for_size(size); default_construct_n(data_, size); + size_ = size; } /** * Create a new array with the given size. All values will be initialized by copying the given * default. */ - Array(int64_t size, const T &value) + Array(int64_t size, const T &value, Allocator allocator = {}) + : Array(NoExceptConstructor(), allocator) { BLI_assert(size >= 0); - size_ = size; data_ = this->get_buffer_for_size(size); - uninitialized_fill_n(data_, size_, value); + uninitialized_fill_n(data_, size, value); + size_ = size; } /** @@ -145,28 +153,28 @@ class Array { * Usage: * Array<std::string> my_strings(10, NoInitialization()); */ - Array(int64_t size, NoInitialization) + Array(int64_t size, NoInitialization, Allocator allocator = {}) + : Array(NoExceptConstructor(), allocator) { BLI_assert(size >= 0); - size_ = size; data_ = this->get_buffer_for_size(size); + size_ = size; } Array(const Array &other) : Array(other.as_span(), other.allocator_) { } - Array(Array &&other) noexcept : allocator_(other.allocator_) + Array(Array &&other) noexcept(std::is_nothrow_move_constructible_v<T>) + : Array(NoExceptConstructor(), other.allocator_) { - size_ = other.size_; - - if (!other.uses_inline_buffer()) { - data_ = other.data_; + if (other.uses_inline_buffer()) { + uninitialized_relocate_n(other.data_, other.size_, data_); } else { - data_ = this->get_buffer_for_size(size_); - uninitialized_relocate_n(other.data_, size_, data_); + data_ = other.data_; } + size_ = other.size_; other.data_ = other.inline_buffer_; other.size_ = 0; @@ -182,24 +190,12 @@ class Array { Array &operator=(const Array &other) { - if (this == &other) { - return *this; - } - - this->~Array(); - new (this) Array(other); - return *this; + return copy_assign_container(*this, other); } - Array &operator=(Array &&other) + Array &operator=(Array &&other) noexcept(std::is_nothrow_move_constructible_v<T>) { - if (this == &other) { - return *this; - } - - this->~Array(); - new (this) Array(std::move(other)); - return *this; + return move_assign_container(*this, std::move(other)); } T &operator[](int64_t index) diff --git a/source/blender/blenlib/BLI_memory_utils.hh b/source/blender/blenlib/BLI_memory_utils.hh index 7216536a884..49076bb1aae 100644 --- a/source/blender/blenlib/BLI_memory_utils.hh +++ b/source/blender/blenlib/BLI_memory_utils.hh @@ -162,7 +162,7 @@ void uninitialized_convert_n(const From *src, int64_t n, To *dst) int64_t current = 0; try { for (; current < n; current++) { - new (static_cast<void *>(dst + current)) To((To)src[current]); + new (static_cast<void *>(dst + current)) To(static_cast<To>(src[current])); } } catch (...) { @@ -410,6 +410,15 @@ class NoInitialization { }; /** + * This can be used to mark a constructor of an object that does not throw exceptions. Other + * constructors can delegate to this constructor to make sure that the object lifetime starts. + * With this, the destructor of the object will be called, even when the remaining constructor + * throws. + */ +class NoExceptConstructor { +}; + +/** * Helper variable that checks if a pointer type can be converted into another pointer type without * issues. Possible issues are casting away const and casting a pointer to a child class. * Adding const or casting to a parent class is fine. @@ -427,4 +436,49 @@ inline constexpr int64_t default_inline_buffer_capacity(size_t element_size) return (static_cast<int64_t>(element_size) < 100) ? 4 : 0; } +/** + * This can be used by containers to implement an exception-safe copy-assignment-operator. + * It assumes that the container has an exception safe copy constructor and an exception-safe + * move-assignment-operator. + */ +template<typename Container> Container ©_assign_container(Container &dst, const Container &src) +{ + if (&src == &dst) { + return dst; + } + + Container container_copy{src}; + dst = std::move(container_copy); + return dst; +} + +/** + * This can be used by containers to implement an exception-safe move-assignment-operator. + * It assumes that the container has an exception-safe move-constructor and a noexcept constructor + * tagged with the NoExceptConstructor tag. + */ +template<typename Container> +Container &move_assign_container(Container &dst, Container &&src) noexcept( + std::is_nothrow_move_constructible_v<Container>) +{ + if (&dst == &src) { + return dst; + } + + dst.~Container(); + if constexpr (std::is_nothrow_move_constructible_v<Container>) { + new (&dst) Container(std::move(src)); + } + else { + try { + new (&dst) Container(std::move(src)); + } + catch (...) { + new (&dst) Container(NoExceptConstructor()); + throw; + } + } + return dst; +} + } // namespace blender diff --git a/source/blender/blenlib/BLI_stack.hh b/source/blender/blenlib/BLI_stack.hh index 8eca356ec54..a463ac102f1 100644 --- a/source/blender/blenlib/BLI_stack.hh +++ b/source/blender/blenlib/BLI_stack.hh @@ -117,7 +117,7 @@ class Stack { /** * Initialize an empty stack. No heap allocation is done. */ - Stack(Allocator allocator = {}) : allocator_(allocator) + Stack(Allocator allocator = {}) noexcept : allocator_(allocator) { inline_chunk_.below = nullptr; inline_chunk_.above = nullptr; @@ -129,11 +129,15 @@ class Stack { size_ = 0; } + Stack(NoExceptConstructor, Allocator allocator = {}) noexcept : Stack(allocator) + { + } + /** * Create a new stack that contains the given elements. The values are pushed to the stack in * the order they are in the array. */ - Stack(Span<T> values) : Stack() + Stack(Span<T> values, Allocator allocator = {}) : Stack(NoExceptConstructor(), allocator) { this->push_multiple(values); } @@ -147,11 +151,12 @@ class Stack { * assert(stack.pop() == 6); * assert(stack.pop() == 5); */ - Stack(const std::initializer_list<T> &values) : Stack(Span<T>(values)) + Stack(const std::initializer_list<T> &values, Allocator allocator = {}) + : Stack(Span<T>(values), allocator) { } - Stack(const Stack &other) : Stack(other.allocator_) + Stack(const Stack &other) : Stack(NoExceptConstructor(), other.allocator_) { for (const Chunk *chunk = &other.inline_chunk_; chunk; chunk = chunk->above) { const T *begin = chunk->begin; @@ -160,7 +165,8 @@ class Stack { } } - Stack(Stack &&other) noexcept : Stack(other.allocator_) + Stack(Stack &&other) noexcept(std::is_nothrow_move_constructible_v<T>) + : Stack(NoExceptConstructor(), other.allocator_) { uninitialized_relocate_n<T>( other.inline_buffer_, std::min(other.size_, InlineBufferCapacity), inline_buffer_); @@ -197,28 +203,14 @@ class Stack { } } - Stack &operator=(const Stack &stack) + Stack &operator=(const Stack &other) { - if (this == &stack) { - return *this; - } - - this->~Stack(); - new (this) Stack(stack); - - return *this; + return copy_assign_container(*this, other); } - Stack &operator=(Stack &&stack) + Stack &operator=(Stack &&other) { - if (this == &stack) { - return *this; - } - - this->~Stack(); - new (this) Stack(std::move(stack)); - - return *this; + return move_assign_container(*this, std::move(other)); } /** @@ -226,21 +218,26 @@ class Stack { */ void push(const T &value) { - if (top_ == top_chunk_->capacity_end) { - this->activate_next_chunk(1); - } - new (top_) T(value); - top_++; - size_++; + this->push_as(value); } void push(T &&value) { + this->push_as(std::move(value)); + } + template<typename ForwardT> void push_as(ForwardT &&value) + { if (top_ == top_chunk_->capacity_end) { this->activate_next_chunk(1); } - new (top_) T(std::move(value)); - top_++; - size_++; + try { + new (top_) T(std::forward<ForwardT>(value)); + top_++; + size_++; + } + catch (...) { + this->move_top_pointer_back_to_below_chunk(); + throw; + } } /** @@ -250,8 +247,8 @@ class Stack { T pop() { BLI_assert(size_ > 0); + T value = std::move(*(top_ - 1)); top_--; - T value = std::move(*top_); top_->~T(); size_--; @@ -296,13 +293,18 @@ class Stack { const int64_t remaining_capacity = top_chunk_->capacity_end - top_; const int64_t amount = std::min(remaining_values.size(), remaining_capacity); - uninitialized_copy_n(remaining_values.data(), amount, top_); + try { + uninitialized_copy_n(remaining_values.data(), amount, top_); + } + catch (...) { + this->move_top_pointer_back_to_below_chunk(); + throw; + } top_ += amount; + size_ += amount; remaining_values = remaining_values.drop_front(amount); } - - size_ += values.size(); } /** @@ -332,6 +334,15 @@ class Stack { top_ = top_chunk_->begin; } + /* This should only be called by unit tests. */ + bool is_invariant_maintained() const + { + if (size_ == 0) { + return top_ == inline_chunk_.begin; + } + return top_ > top_chunk_->begin; + } + private: /** * Changes top_chunk_ to point to a new chunk that is above the current one. The new chunk might @@ -365,6 +376,18 @@ class Stack { top_ = top_chunk_->begin; } + void move_top_pointer_back_to_below_chunk() + { + /* This makes sure that the invariant stays intact after a failed push. */ + if (size_ == 0) { + top_ = inline_chunk_.begin; + } + else if (top_ == top_chunk_->begin) { + top_chunk_ = top_chunk_->below; + top_ = top_chunk_->capacity_end; + } + } + void destruct_all_elements() { for (T *value = top_chunk_->begin; value != top_; value++) { diff --git a/source/blender/blenlib/BLI_vector.hh b/source/blender/blenlib/BLI_vector.hh index 74ce8dd42e7..06bafce2dd0 100644 --- a/source/blender/blenlib/BLI_vector.hh +++ b/source/blender/blenlib/BLI_vector.hh @@ -118,7 +118,7 @@ class Vector { * Create an empty vector. * This does not do any memory allocation. */ - Vector(Allocator allocator = {}) : allocator_(allocator) + Vector(Allocator allocator = {}) noexcept : allocator_(allocator) { begin_ = inline_buffer_; end_ = begin_; @@ -126,12 +126,17 @@ class Vector { UPDATE_VECTOR_SIZE(this); } + Vector(NoExceptConstructor, Allocator allocator = {}) noexcept : Vector(allocator) + { + } + /** * Create a vector with a specific size. * The elements will be default constructed. * If T is trivially constructible, the elements in the vector are not touched. */ - explicit Vector(int64_t size) : Vector() + explicit Vector(int64_t size, Allocator allocator = {}) + : Vector(NoExceptConstructor(), allocator) { this->resize(size); } @@ -139,7 +144,8 @@ class Vector { /** * Create a vector filled with a specific value. */ - Vector(int64_t size, const T &value) : Vector() + Vector(int64_t size, const T &value, Allocator allocator = {}) + : Vector(NoExceptConstructor(), allocator) { this->resize(size, value); } @@ -148,12 +154,12 @@ class Vector { * Create a vector from an array ref. The values in the vector are copy constructed. */ template<typename U, typename std::enable_if_t<std::is_convertible_v<U, T>> * = nullptr> - Vector(Span<U> values, Allocator allocator = {}) : Vector(allocator) + Vector(Span<U> values, Allocator allocator = {}) : Vector(NoExceptConstructor(), allocator) { const int64_t size = values.size(); this->reserve(size); - this->increase_size_by_unchecked(size); uninitialized_convert_n<U, T>(values.data(), size, begin_); + this->increase_size_by_unchecked(size); } /** @@ -182,7 +188,8 @@ class Vector { /* This constructor should not be called with e.g. Vector(3, 10), because that is expected to produce the vector (10, 10, 10). */ typename std::enable_if_t<!std::is_convertible_v<InputIt, int>> * = nullptr> - Vector(InputIt first, InputIt last, Allocator allocator = {}) : Vector(std::move(allocator)) + Vector(InputIt first, InputIt last, Allocator allocator = {}) + : Vector(NoExceptConstructor(), allocator) { for (InputIt current = first; current != last; ++current) { this->append(*current); @@ -196,7 +203,7 @@ class Vector { * Example Usage: * Vector<ModifierData *> modifiers(ob->modifiers); */ - Vector(ListBase &values) : Vector() + Vector(ListBase &values, Allocator allocator = {}) : Vector(NoExceptConstructor(), allocator) { LISTBASE_FOREACH (T, value, &values) { this->append(value); @@ -226,27 +233,26 @@ class Vector { * have zero elements afterwards. */ template<int64_t OtherInlineBufferCapacity> - Vector(Vector<T, OtherInlineBufferCapacity, Allocator> &&other) noexcept - : allocator_(other.allocator_) + Vector(Vector<T, OtherInlineBufferCapacity, Allocator> &&other) noexcept( + std::is_nothrow_move_constructible_v<T>) + : Vector(NoExceptConstructor(), other.allocator_) { const int64_t size = other.size(); if (other.is_inline()) { if (size <= InlineBufferCapacity) { /* Copy between inline buffers. */ - begin_ = inline_buffer_; - end_ = begin_ + size; - capacity_end_ = begin_ + InlineBufferCapacity; uninitialized_relocate_n(other.begin_, size, begin_); + end_ = begin_ + size; } else { /* Copy from inline buffer to newly allocated buffer. */ const int64_t capacity = size; begin_ = static_cast<T *>( allocator_.allocate(sizeof(T) * static_cast<size_t>(capacity), alignof(T), AT)); - end_ = begin_ + size; capacity_end_ = begin_ + capacity; uninitialized_relocate_n(other.begin_, size, begin_); + end_ = begin_ + size; } } else { @@ -273,28 +279,12 @@ class Vector { Vector &operator=(const Vector &other) { - if (this == &other) { - return *this; - } - - this->~Vector(); - new (this) Vector(other); - - return *this; + return copy_assign_container(*this, other); } Vector &operator=(Vector &&other) { - if (this == &other) { - return *this; - } - - /* This can be incorrect, when the vector is used to build a recursive data structure. However, - we don't take care of it at this low level. See https://youtu.be/7Qgd9B1KuMQ?t=840. */ - this->~Vector(); - new (this) Vector(std::move(other)); - - return *this; + return move_assign_container(*this, std::move(other)); } /** @@ -474,17 +464,10 @@ class Vector { * behavior when not enough capacity has been reserved beforehand. Only use this in performance * critical code. */ - void append_unchecked(const T &value) - { - BLI_assert(end_ < capacity_end_); - new (end_) T(value); - end_++; - UPDATE_VECTOR_SIZE(this); - } - void append_unchecked(T &&value) + template<typename ForwardT> void append_unchecked(ForwardT &&value) { BLI_assert(end_ < capacity_end_); - new (end_) T(std::move(value)); + new (end_) T(std::forward<ForwardT>(value)); end_++; UPDATE_VECTOR_SIZE(this); } @@ -497,7 +480,7 @@ class Vector { { BLI_assert(n >= 0); this->reserve(this->size() + n); - blender::uninitialized_fill_n(end_, n, value); + uninitialized_fill_n(end_, n, value); this->increase_size_by_unchecked(n); } @@ -507,7 +490,7 @@ class Vector { * useful when you want to call constructors in the vector yourself. This should only be done in * very rare cases and has to be justified every time. */ - void increase_size_by_unchecked(const int64_t n) + void increase_size_by_unchecked(const int64_t n) noexcept { BLI_assert(end_ + n <= capacity_end_); end_ += n; @@ -553,7 +536,7 @@ class Vector { { BLI_assert(amount >= 0); BLI_assert(begin_ + amount <= capacity_end_); - blender::uninitialized_copy_n(start, amount, end_); + uninitialized_copy_n(start, amount, end_); end_ += amount; UPDATE_VECTOR_SIZE(this); } @@ -600,11 +583,29 @@ class Vector { for (int64_t i = 0; i < move_amount; i++) { const int64_t src_index = insert_index + move_amount - i - 1; const int64_t dst_index = new_size - i - 1; - new (static_cast<void *>(begin_ + dst_index)) T(std::move(begin_[src_index])); + try { + new (static_cast<void *>(begin_ + dst_index)) T(std::move(begin_[src_index])); + } + catch (...) { + /* Destruct all values that have been moved already. */ + destruct_n(begin_ + dst_index + 1, i); + end_ = begin_ + src_index; + UPDATE_VECTOR_SIZE(this); + throw; + } begin_[src_index].~T(); } - std::uninitialized_copy_n(first, insert_amount, begin_ + insert_index); + try { + std::uninitialized_copy_n(first, insert_amount, begin_ + insert_index); + } + catch (...) { + /* Destruct all values that have been moved. */ + destruct_n(begin_ + new_size - move_amount, move_amount); + end_ = begin_ + insert_index; + UPDATE_VECTOR_SIZE(this); + throw; + } end_ = begin_ + new_size; UPDATE_VECTOR_SIZE(this); } @@ -686,8 +687,8 @@ class Vector { T pop_last() { BLI_assert(!this->is_empty()); + T value = std::move(*(end_ - 1)); end_--; - T value = std::move(*end_); end_->~T(); UPDATE_VECTOR_SIZE(this); return value; @@ -702,10 +703,10 @@ class Vector { BLI_assert(index >= 0); BLI_assert(index < this->size()); T *element_to_remove = begin_ + index; - end_--; if (element_to_remove < end_) { - *element_to_remove = std::move(*end_); + *element_to_remove = std::move(*(end_ - 1)); } + end_--; end_->~T(); UPDATE_VECTOR_SIZE(this); } @@ -901,7 +902,13 @@ class Vector { T *new_array = static_cast<T *>( allocator_.allocate(static_cast<size_t>(new_capacity) * sizeof(T), alignof(T), AT)); - uninitialized_relocate_n(begin_, size, new_array); + try { + uninitialized_relocate_n(begin_, size, new_array); + } + catch (...) { + allocator_.deallocate(new_array); + throw; + } if (!this->is_inline()) { allocator_.deallocate(begin_); diff --git a/source/blender/blenlib/CMakeLists.txt b/source/blender/blenlib/CMakeLists.txt index 819c74b6946..2f1c3436806 100644 --- a/source/blender/blenlib/CMakeLists.txt +++ b/source/blender/blenlib/CMakeLists.txt @@ -390,6 +390,8 @@ if(WITH_GTESTS) tests/BLI_task_test.cc tests/BLI_vector_set_test.cc tests/BLI_vector_test.cc + + tests/BLI_exception_safety_test_utils.hh ) set(TEST_INC ../imbuf 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 |