diff --git a/data/doxygen.h b/data/doxygen.h index 2f19ed4..f8028f7 100644 --- a/data/doxygen.h +++ b/data/doxygen.h @@ -4,7 +4,7 @@ * \date Created on August 24, 2018 * \brief Doxygen input file for the General Utility Library. * - * \copyright Copyright 2018-2025 Deutsches Elektronen-Synchrotron (DESY), Hamburg + * \copyright Copyright 2018-2026 Deutsches Elektronen-Synchrotron (DESY), Hamburg * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Lesser General Public License as published @@ -146,6 +146,11 @@ namespace gul17 { * * \section changelog_gul17 GUL17 Versions * + * \subsection V26_X_X Unreleased + * + * - Avoid using std::result_of_t which is removed in C++20. + * - Avoid using std::aligned_storage which is deprecated in C++23. + * * \subsection V26_2_0 Version 26.2.0 * * - Add gul17::null_safe_string_view(const char*, std::size_t), which returns an empty diff --git a/include/gul17/SmallVector.h b/include/gul17/SmallVector.h index 337d3cc..030ee99 100644 --- a/include/gul17/SmallVector.h +++ b/include/gul17/SmallVector.h @@ -4,7 +4,7 @@ * \date Created on August 17, 2020 * \brief Definition of the SmallVector class template. * - * \copyright Copyright 2020-2023 Deutsches Elektronen-Synchrotron (DESY), Hamburg + * \copyright Copyright 2020-2026 Deutsches Elektronen-Synchrotron (DESY), Hamburg * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Lesser General Public License as published @@ -25,16 +25,20 @@ #include #include +#include #include #include #include #include #include +#include #include #include +#include -#include "gul17/internal.h" #include "gul17/cat.h" +#include "gul17/finalizer.h" +#include "gul17/internal.h" namespace gul17 { @@ -422,9 +426,8 @@ class SmallVector ~SmallVector() { clear(); - if (is_storage_allocated()) - delete[] data_ptr_; + deallocate_space_for_elements(data_ptr_); } /** @@ -601,7 +604,7 @@ class SmallVector */ constexpr ValueType* data() noexcept { - return reinterpret_cast(data_ptr_); + return data_ptr_; } /** @@ -610,7 +613,7 @@ class SmallVector */ constexpr const ValueType* data() const noexcept { - return reinterpret_cast(data_ptr_); + return data_ptr_; } /** @@ -940,7 +943,7 @@ class SmallVector { clear(); if (is_storage_allocated()) - delete[] data_ptr_; + deallocate_space_for_elements(data_ptr_); move_or_copy_all_elements_from(std::move(other)); } @@ -1075,17 +1078,19 @@ class SmallVector if (new_capacity <= capacity_) return; - auto new_data = std::make_unique(new_capacity); + // Allocate aligned memory for the new "outer" capacity. + auto* new_data = allocate_space_for_elements(new_capacity); + auto _ = finally([&new_data]() { deallocate_space_for_elements(new_data); }); - const auto d_end = data_end(); - - uninitialized_move_or_copy(data(), d_end, reinterpret_cast(new_data.get())); + auto* d_end = data_end(); + uninitialized_move_or_copy(data(), d_end, new_data); destroy_range(data(), d_end); if (is_storage_allocated()) - delete[] data_ptr_; + deallocate_space_for_elements(data_ptr_); - data_ptr_ = new_data.release(); + data_ptr_ = new_data; + new_data = nullptr; capacity_ = new_capacity; } @@ -1161,26 +1166,30 @@ class SmallVector if (new_capacity == capacity_) return; - AlignedStorage* new_data; - auto new_memory = std::unique_ptr{}; + ValueType* new_data{}; + ValueType* allocation{}; + auto _ = finally([&allocation]() { deallocate_space_for_elements(allocation); }); - if (new_capacity == inner_capacity()) { - new_data = internal_array_.data(); - } else { - new_memory = std::make_unique(new_capacity); - new_data = new_memory.get(); + if (new_capacity == inner_capacity()) + { + new_data = get_internal_array_pointer(); + } + else + { + allocation = allocate_space_for_elements(new_capacity); + new_data = allocation; } - const auto d_end = data_end(); - - uninitialized_move_or_copy(data(), d_end, reinterpret_cast(new_data)); + auto* d_end = data_end(); + uninitialized_move_or_copy(data(), d_end, new_data); destroy_range(data(), d_end); if (is_storage_allocated()) - delete[] data_ptr_; + deallocate_space_for_elements(data_ptr_); - data_ptr_ = new_memory ? new_memory.release() : new_data; + data_ptr_ = new_data; capacity_ = new_capacity; + allocation = nullptr; // Avoid deallocation by the "finally" guard } /// Return the number of elements that are currently stored. @@ -1212,17 +1221,15 @@ class SmallVector } private: - using AlignedStorage = - typename std::aligned_storage::type; - /** - * This union either holds the storage for the "internal" elements or, if the vector - * has grown beyond that size, a pointer to storage on the heap. + * Uninitialized storage for the internal elements (used only if size() <= + * inner_capacity()). */ - std::array internal_array_; + alignas(ValueType) + std::array internal_array_; /// Pointer to internal or external contiguous data storage. - AlignedStorage* data_ptr_{ internal_array_.data() }; + ValueType* data_ptr_{ get_internal_array_pointer() }; /// Capacity of the vector (i.e. number of elements that can be stored without /// enlarging the container) @@ -1230,6 +1237,17 @@ class SmallVector /// Number of elements stored in the container. SizeType size_{ 0u }; + /** + * Allocate aligned, uninitialized memory for storing a certain number of elements. + * The memory has to be deallocated with deallocate_space_for_elements() after use. + */ + static ValueType* allocate_space_for_elements(std::size_t num_elements) + { + return static_cast( + ::operator new[](num_elements * sizeof(ValueType), + std::align_val_t{ alignof(ValueType) })); + } + /** * Copy a range of values into uninitialized cells. * @@ -1280,16 +1298,22 @@ class SmallVector ::new(static_cast(p)) ValueType(value); } - /// Return a non-dereferencable pointer past the last element. + /// Return a non-dereferenceable pointer past the last element. constexpr ValueType* data_end() noexcept { - return reinterpret_cast(data_ptr_) + size_; + return data_ptr_ + size_; } - /// Return a non-dereferencable pointer past the last element. + /// Return a non-dereferenceable pointer past the last element. constexpr const ValueType* data_end() const noexcept { - return reinterpret_cast(data_ptr_) + size_; + return data_ptr_ + size_; + } + + /// Deallocate aligned memory that was reserved with allocate_space_for_elements(). + static void deallocate_space_for_elements(ValueType* ptr) noexcept + { + ::operator delete[](ptr, std::align_val_t{ alignof(ValueType) }); } /** @@ -1372,6 +1396,18 @@ class SmallVector } } + /// Return a ValueType pointer to the internal array storage. + const ValueType* get_internal_array_pointer() const noexcept + { + return reinterpret_cast(internal_array_.data()); + } + + /// Return a ValueType pointer to the internal array storage. + ValueType* get_internal_array_pointer() noexcept + { + return reinterpret_cast(internal_array_.data()); + } + /** * Increase the capacity of the vector, typically by ~50%. * @@ -1430,7 +1466,7 @@ class SmallVector /// Determine if this vector is using allocated external storage. bool is_storage_allocated() const noexcept { - return data_ptr_ != internal_array_.data(); + return data_ptr_ != get_internal_array_pointer(); } /** @@ -1488,6 +1524,8 @@ class SmallVector * performs no check for self-assignment. * * \pre `size() == 0 and is_allocated() == false and &other != this` + * \post `data_ptr_ != nullptr` (it either points to the allocated storage stolen from + * `other` or to the internal array) */ void move_or_copy_all_elements_from(SmallVector&& other) noexcept(std::is_nothrow_move_constructible::value) @@ -1495,13 +1533,13 @@ class SmallVector // Can we simply steal the complete allocated storage? if (other.is_storage_allocated()) { - data_ptr_ = other.data_ptr_; other.data_ptr_ = other.internal_array_.data(); - capacity_ = other.capacity_; other.capacity_ = other.inner_capacity(); - size_ = other.size_; other.size_ = 0u; + data_ptr_ = std::exchange(other.data_ptr_, other.get_internal_array_pointer()); + capacity_ = std::exchange(other.capacity_, other.inner_capacity()); + size_ = std::exchange(other.size_, 0u); } else // otherwise fall back to moving (or at least copying) all elements { - data_ptr_ = internal_array_.data(); + data_ptr_ = get_internal_array_pointer(); capacity_ = in_capacity; uninitialized_move_or_copy(other.begin(), other.end(), data()); size_ = other.size(); @@ -1529,12 +1567,10 @@ class SmallVector */ static void swap_heap_with_internal(SmallVector &a, SmallVector &b) { - uninitialized_move_or_copy(b.begin(), b.end(), - reinterpret_cast(a.internal_array_.data())); + uninitialized_move_or_copy(b.begin(), b.end(), a.get_internal_array_pointer()); destroy_range(b.begin(), b.end()); - b.data_ptr_ = a.data_ptr_; - a.data_ptr_ = a.internal_array_.data(); + b.data_ptr_ = std::exchange(a.data_ptr_, a.get_internal_array_pointer()); std::swap(a.capacity_, b.capacity_); std::swap(a.size_, b.size_); diff --git a/tests/test_SmallVector.cc b/tests/test_SmallVector.cc index eddf602..44741e2 100644 --- a/tests/test_SmallVector.cc +++ b/tests/test_SmallVector.cc @@ -575,6 +575,68 @@ TEMPLATE_TEST_CASE("SmallVector: Iterator traits", "[SmallVector]", >::value == true); } +namespace { + +struct OverAligned +{ + alignas(1024) char data[64]; +}; + +template +std::pair get_front_and_back_pointers(VectorT& vec) +{ + if (vec.empty()) + throw std::logic_error("Vector is empty"); + + return { reinterpret_cast(&vec.front()), + reinterpret_cast(&vec.back()) }; +} + +template +void test_alignment_with() +{ + VectorT vec; + using ValueT = typename VectorT::ValueType; + + CAPTURE(vec.inner_capacity()); + CAPTURE(alignof(ValueT)); + + for (unsigned int i = 1u; i <= 50; ++i) + { + vec.push_back(ValueT{}); + CAPTURE(vec.size()); + + auto [front_ptr, back_ptr] = get_front_and_back_pointers(vec); + REQUIRE(front_ptr % alignof(ValueT) == 0); + REQUIRE(back_ptr % alignof(ValueT) == 0); + REQUIRE(back_ptr - front_ptr == sizeof(ValueT) * (vec.size() - 1u)); + } + + while (vec.size() > 1u) + { + vec.pop_back(); + vec.shrink_to_fit(); + CAPTURE(vec.size()); + + auto [front_ptr, back_ptr] = get_front_and_back_pointers(vec); + REQUIRE(front_ptr % alignof(ValueT) == 0); + REQUIRE(back_ptr % alignof(ValueT) == 0); + REQUIRE(back_ptr - front_ptr == sizeof(ValueT) * (vec.size() - 1u)); + } +} + +} // anonymous namespace + +TEMPLATE_TEST_CASE("SmallVector: Correct alignment after push_back() and shrink_to_fit()", + "[SmallVector]", char, short, double, std::string, OverAligned) +{ + test_alignment_with>(); + test_alignment_with>(); + test_alignment_with>(); + test_alignment_with>(); + test_alignment_with>(); +} + // // Tests for indiviual member functions (in alphabetical order)