Do not use std::aligned_storage

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

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 <algorithm>
 #include <array>
+#include <cstddef>
 #include <cstdint>
 #include <initializer_list>
 #include <iterator>
 #include <limits>
 #include <memory>
+#include <new>
 #include <stdexcept>
 #include <type_traits>
+#include <utility>
 
-#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<ValueType*>(data_ptr_);
+        return data_ptr_;
     }
 
     /**
@@ -610,7 +613,7 @@ class SmallVector
      */
     constexpr const ValueType* data() const noexcept
     {
-        return reinterpret_cast<const ValueType*>(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<AlignedStorage[]>(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<ValueType*>(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<AlignedStorage[]>{};
+        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<AlignedStorage[]>(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<ValueType*>(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,24 +1221,33 @@ class SmallVector
     }
 
 private:
-    using AlignedStorage =
-        typename std::aligned_storage<sizeof(ValueType), alignof(ValueType)>::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<AlignedStorage, in_capacity> internal_array_;
+    alignas(ValueType)
+    std::array<std::byte, in_capacity * sizeof(ValueType)> 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)
     SizeType capacity_{ in_capacity };
     /// 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<ValueType*>(
+            ::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<void*>(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<ValueType*>(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<const ValueType*>(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<const ValueType*>(internal_array_.data());
+    }
+
+    /// Return a ValueType pointer to the internal array storage.
+    ValueType* get_internal_array_pointer() noexcept
+    {
+        return reinterpret_cast<ValueType*>(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,20 +1524,22 @@ 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<ValueType>::value)
     {
         // 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<ValueType*>(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_);

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 <typename VectorT>
+std::pair<std::uintptr_t, std::uintptr_t> get_front_and_back_pointers(VectorT& vec)
+{
+    if (vec.empty())
+        throw std::logic_error("Vector is empty");
+
+    return { reinterpret_cast<std::uintptr_t>(&vec.front()),
+             reinterpret_cast<std::uintptr_t>(&vec.back()) };
+}
+
+template <typename VectorT>
+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<SmallVector<TestType, 0>>();
+    test_alignment_with<SmallVector<TestType, 1>>();
+    test_alignment_with<SmallVector<TestType, 2>>();
+    test_alignment_with<SmallVector<TestType, 3>>();
+    test_alignment_with<SmallVector<TestType, 4>>();
+}
+
 
 //
 // Tests for indiviual member functions (in alphabetical order)