Match std::vector on size limits, cover the untested move path, add the C++23 range members

CMakeLists.txt

@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.12)
 project("svector"
-    VERSION 1.3.0
+    VERSION 1.4.0
     DESCRIPTION " Compact SVO optimized vector for C++17 or higher"
     HOMEPAGE_URL "https://github.com/martinus/svector")
 

README.md

@@ -267,6 +267,10 @@ being a third the size of the next smallest object.
 
 ## Differences from std::vector
 
+The C++23 range members — `assign_range`, `append_range`, `insert_range` and the `std::from_range` constructor —
+are there when the standard library has `std::from_range_t` to build them on, and absent otherwise. A C++17
+build is exactly what it was.
+
 `ankerl::svector` implements all of `std::vector`'s API, plus `std::erase`/`std::erase_if`, and comparison
 operators that work between svectors of different inline capacities.
 

include/ankerl/svector.h

@@ -1,5 +1,5 @@
 // ┌─┐┬  ┬┌─┐┌─┐┌┬┐┌─┐┬─┐   Compact SVO optimized vector C++17 or higher
-// └─┐└┐┌┘├┤ │   │ │ │├┬┘   Version 1.3.0
+// └─┐└┐┌┘├┤ │   │ │ │├┬┘   Version 1.4.0
 // └─┘ └┘ └─┘└─┘ ┴ └─┘┴└─   https://github.com/martinus/svector
 //
 // Licensed under the MIT License <http://opensource.org/licenses/MIT>.
@@ -29,7 +29,7 @@
 
 // see https://semver.org/spec/v2.0.0.html
 #define ANKERL_SVECTOR_VERSION_MAJOR 1 // incompatible API changes
-#define ANKERL_SVECTOR_VERSION_MINOR 3 // add functionality in a backwards compatible manner
+#define ANKERL_SVECTOR_VERSION_MINOR 4 // add functionality in a backwards compatible manner
 #define ANKERL_SVECTOR_VERSION_PATCH 0 // backwards compatible bug fixes
 
 // API versioning with inline namespace, see https://www.foonathan.net/2018/11/inline-namespaces/
@@ -67,6 +67,23 @@
 #include <type_traits>
 #include <utility>
 
+#if defined(__has_include)
+#    if __has_include(<version>)
+#        include <version>
+#    endif
+#endif
+
+// The C++23 range members. Guarded on the library rather than on the language, because what they
+// need is std::from_range_t and the range concepts, and a C++17 build has neither. Everything else
+// in this header stays exactly as it was for such a build.
+#if defined(__cpp_lib_containers_ranges) && __cpp_lib_containers_ranges >= 202202L
+#    define ANKERL_SVECTOR_HAS_RANGES 1
+#    include <concepts>
+#    include <ranges>
+#else
+#    define ANKERL_SVECTOR_HAS_RANGES 0
+#endif
+
 namespace ankerl {
 inline namespace ANKERL_SVECTOR_NAMESPACE {
 namespace detail {
@@ -77,6 +94,21 @@ using enable_if_t = std::enable_if_t<Condition::value, T>;
 template <typename It>
 using is_input_iterator = std::is_base_of<std::input_iterator_tag, typename std::iterator_traits<It>::iterator_category>;
 
+// A C++20 iterator need not publish an iterator_category, only an iterator_concept, and then
+// is_input_iterator above cannot even be formed. Used to decide whether a range can be handed to
+// the iterator pair members as it is.
+template <typename It, typename = void>
+struct has_iterator_category : std::false_type {};
+
+template <typename It>
+struct has_iterator_category<It, std::void_t<typename std::iterator_traits<It>::iterator_category>> : std::true_type {};
+
+#if ANKERL_SVECTOR_HAS_RANGES
+// The standard calls this container-compatible-range<T> and uses it for exactly these members.
+template <typename R, typename T>
+concept container_compatible_range = std::ranges::input_range<R> && std::convertible_to<std::ranges::range_reference_t<R>, T>;
+#endif
+
 constexpr auto round_up(size_t n, size_t multiple) -> size_t {
     return ((n + (multiple - 1)) / multiple) * multiple;
 }
@@ -817,8 +849,10 @@ class svector : private detail::allocator_holder<Allocator> {
      */
     [[nodiscard]] static auto calculate_new_capacity(size_t size_to_fit, size_t starting_capacity) -> size_t {
         if (size_to_fit > max_size()) {
-            // not enough space
-            throw std::bad_alloc();
+            // Asking for more elements than can exist is a size error, not a failure to find
+            // memory, and std::vector spells it the same way. bad_alloc stays for the case that
+            // really is one: a size that is legal but that the allocator cannot satisfy.
+            throw std::length_error("svector: requested size exceeds max_size()");
         }
 
         if (size_to_fit == 0) {
@@ -1137,7 +1171,7 @@ class svector : private detail::allocator_holder<Allocator> {
         // and the wrapped sum then looked small enough to fit, so the in place shift below ran
         // straight past the end of the buffer. See issue #69.
         if (count > max_size() - s) {
-            throw std::bad_alloc();
+            throw std::length_error("svector: requested size exceeds max_size()");
         }
 
         if (count > capacity<D>() - s) {
@@ -1378,6 +1412,20 @@ class svector : private detail::allocator_holder<Allocator> {
         assign(first, last);
     }
 
+#if ANKERL_SVECTOR_HAS_RANGES
+    template <detail::container_compatible_range<T> R>
+    svector(std::from_range_t /*unused*/, R&& rg)
+        : svector() {
+        append_range(std::forward<R>(rg));
+    }
+
+    template <detail::container_compatible_range<T> R>
+    svector(std::from_range_t /*unused*/, R&& rg, Allocator const& alloc)
+        : svector(alloc) {
+        append_range(std::forward<R>(rg));
+    }
+#endif
+
     /**
      * @brief Copying asks the allocator which one the copy should use.
      *
@@ -1760,9 +1808,13 @@ class svector : private detail::allocator_holder<Allocator> {
      * function has been public and static since before there was one -- svector<T, N>::max_size()
      * is spelled that way in test/unit/insert.cpp. Making it a non-static member is a breaking
      * change and belongs to a major version, not to adding an allocator.
+     *
+     * Divided by sizeof(T) because a count is not a byte count. It used to answer PTRDIFF_MAX for
+     * every T, which claimed a size no svector could reach: alloc() refuses anything whose bytes
+     * pass PTRDIFF_MAX, so the real ceiling has always been this. std::vector answers the same.
      */
     [[nodiscard]] static auto max_size() noexcept -> size_t {
-        return (std::numeric_limits<std::ptrdiff_t>::max)();
+        return static_cast<size_t>((std::numeric_limits<std::ptrdiff_t>::max)()) / sizeof(T);
     }
 
     [[nodiscard]] auto get_allocator() const noexcept -> Allocator {
@@ -1916,6 +1968,52 @@ class svector : private detail::allocator_holder<Allocator> {
         return insert(pos, first, last, typename std::iterator_traits<InputIt>::iterator_category());
     }
 
+#if ANKERL_SVECTOR_HAS_RANGES
+    /**
+     * @brief The C++23 range members, which std::vector has and this did not.
+     *
+     * All of them go through the iterator pair members, so a range gets the same growth, the same
+     * exception guarantees and the same self referencing checks an iterator pair already got,
+     * rather than a second implementation of all three.
+     *
+     * A range cannot always be handed over as a pair: its sentinel need not be its iterator, and
+     * its iterator need not publish an iterator_category. One that cannot is built into a
+     * temporary first. That costs an allocation for exactly the ranges that could not have been
+     * sized anyway.
+     */
+    template <typename R>
+    static constexpr bool is_iterator_pair_range =
+        std::ranges::common_range<R> && detail::has_iterator_category<std::ranges::iterator_t<R>>::value;
+
+    template <detail::container_compatible_range<T> R>
+    auto insert_range(const_iterator pos, R&& rg) -> iterator {
+        if constexpr (is_iterator_pair_range<R>) {
+            return insert(pos, std::ranges::begin(rg), std::ranges::end(rg));
+        } else {
+            auto tmp = svector(allocator());
+            for (auto&& element : rg) {
+                tmp.emplace_back(std::forward<decltype(element)>(element));
+            }
+            return insert(pos, std::make_move_iterator(tmp.begin()), std::make_move_iterator(tmp.end()));
+        }
+    }
+
+    template <detail::container_compatible_range<T> R>
+    void append_range(R&& rg) {
+        static_cast<void>(insert_range(cend(), std::forward<R>(rg)));
+    }
+
+    template <detail::container_compatible_range<T> R>
+    void assign_range(R&& rg) {
+        if constexpr (is_iterator_pair_range<R>) {
+            assign(std::ranges::begin(rg), std::ranges::end(rg));
+        } else {
+            clear();
+            append_range(std::forward<R>(rg));
+        }
+    }
+#endif
+
     auto insert(const_iterator pos, std::initializer_list<T> l) -> iterator {
         return insert(pos, l.begin(), l.end());
     }

meson.build

@@ -18,7 +18,7 @@
 #
 
 project('svector', 'cpp',
-    version: '1.3.0',
+    version: '1.4.0',
     license: 'MIT',
     default_options : [
         'cpp_std=c++17',

scripts/lint/lint-clang-tidy.py

@@ -15,6 +15,12 @@
 svector is a template, so a check only sees what something instantiated. The translation unit
 below therefore exercises a broad spread of the API: with only a push_back, most of the container
 is never looked at.
+
+Known gap: the C++23 range members are invisible here. The pinned image's standard library has no
+std::from_range_t, so ANKERL_SVECTOR_HAS_RANGES is 0 and that code is not compiled at all,
+whatever -std is passed. Raising the pin would cover them, at the cost of re-curating the check
+list against whatever the newer clang-tidy has added. The C++23 build legs compile those members
+with -Werror in the meantime.
 """
 
 import os

test/meson.build

@@ -35,6 +35,7 @@ test_sources = [
     'unit/noexcept_contract.cpp',
     'unit/pop_back.cpp',
     'unit/push_back.cpp',
+    'unit/ranges.cpp',
     'unit/reserve.cpp',
     'unit/resize.cpp',
     'unit/resize_and_overwrite.cpp',

test/unit/allocator.cpp

@@ -585,3 +585,92 @@ TEST_CASE("allocator_pmr_passes_its_resource_on") {
     REQUIRE(sv.front() == "built by emplace, and long enough to have to allocate for itself");
 }
 #endif
+
+// The extended move constructor has two halves and only one of them was ever run: every test that
+// reached it named an allocator that did not compare equal, so it always moved the elements one at
+// a time. This is the other half, where the allocation itself is taken over.
+TEST_CASE("extended_move_ctor_takes_over_when_allocators_match") {
+    SUBCASE("always equal allocator, indirect") {
+        // std::allocator is is_always_equal, so can_take_over() is a compile time yes and no
+        // allocator is looked at. This is the common case and nothing covered it.
+        auto a = ankerl::svector<std::string, 3>();
+        for (int i = 0; i < 50; ++i) {
+            a.push_back("element number " + std::to_string(i) + ", long enough to allocate");
+        }
+        auto const* const data_before = a.data();
+
+        auto b = ankerl::svector<std::string, 3>(std::move(a), std::allocator<std::string>{});
+
+        // taken over, not copied: the elements are still where they were
+        REQUIRE(b.data() == data_before);
+        REQUIRE(b.size() == 50);
+        REQUIRE(b[0] == "element number 0, long enough to allocate");
+        REQUIRE(b[49] == "element number 49, long enough to allocate");
+    }
+
+    SUBCASE("always equal allocator, direct") {
+        auto a = ankerl::svector<std::string, 3>();
+        a.push_back("one");
+        a.push_back("two");
+
+        auto b = ankerl::svector<std::string, 3>(std::move(a), std::allocator<std::string>{});
+        REQUIRE(b.size() == 2);
+        REQUIRE(b[0] == "one");
+        REQUIRE(b[1] == "two");
+    }
+
+    SUBCASE("stateful allocators that compare equal") {
+        // is_always_equal is false here, so this is the runtime yes: same id, so the allocation
+        // can go back to it and is taken over rather than rebuilt.
+        auto ledger = Ledger();
+        using Vec = ankerl::svector<int, 2, PoccaNone>;
+
+        auto a = Vec(size_t{40}, 7, PoccaNone(3, &ledger));
+        auto const allocations_before = ledger.allocations;
+        auto const* const data_before = a.data();
+
+        auto b = Vec(std::move(a), PoccaNone(3, &ledger));
+
+        // no second allocation: the first one was adopted
+        REQUIRE(ledger.allocations == allocations_before);
+        REQUIRE(b.data() == data_before);
+        REQUIRE(b.get_allocator().id == 3);
+        REQUIRE(b.size() == 40);
+        REQUIRE(b.front() == 7);
+        REQUIRE(b.back() == 7);
+    }
+
+    SUBCASE("stateful allocators that do not compare equal still copy") {
+        // the half that was already covered, kept next to the other one so the contrast is visible
+        auto ledger = Ledger();
+        using Vec = ankerl::svector<int, 2, PoccaNone>;
+
+        auto a = Vec(size_t{40}, 7, PoccaNone(3, &ledger));
+        auto const allocations_before = ledger.allocations;
+
+        auto b = Vec(std::move(a), PoccaNone(4, &ledger));
+
+        REQUIRE(ledger.allocations > allocations_before);
+        REQUIRE(b.get_allocator().id == 4);
+        REQUIRE(b.size() == 40);
+        REQUIRE(b.front() == 7);
+    }
+}
+
+// An allocator with its own construct() takes the element by element path through construct_each(),
+// which is a different loop from the <memory> algorithms the default allocator gets.
+TEST_CASE("range_construct_through_an_allocator_that_constructs") {
+    auto const source = std::vector<int>{1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
+
+    g_constructed = 0;
+    g_destroyed = 0;
+    {
+        auto v = ankerl::svector<int, 2, MarkingAllocator<int>>(source.begin(), source.end());
+        REQUIRE(v.size() == source.size());
+        for (size_t i = 0; i < source.size(); ++i) {
+            REQUIRE(v[i] == source[i]);
+        }
+        REQUIRE(g_constructed >= source.size());
+    }
+    REQUIRE(g_destroyed >= source.size());
+}

test/unit/bad_alloc.cpp

@@ -32,13 +32,53 @@
 #    define SANITIZER_ACTIVE 1
 #endif
 
+// gcc constant folds a size derived from max_size() and then reasons about an array of that many
+// elements, which at -O2 with the hardening flags is an -Warray-bounds error and, on gcc 13, an
+// internal compiler error. The size is a runtime value in any real use, so make it one here too.
+namespace {
+auto opaque(size_t value) -> size_t {
+    volatile size_t hidden = value;
+    return hidden;
+}
+} // namespace
+
 TEST_CASE("reserve_bad_alloc") {
     if constexpr (RUNNING_ON_VALGRIND || SANITIZER_ACTIVE) {
         // this test doesn't work with valgrind or some sanitizers.
     } else {
         auto sv = ankerl::svector<std::string, 3>();
         auto m = sv.max_size();
-        REQUIRE(m == std::numeric_limits<std::ptrdiff_t>::max());
-        REQUIRE_THROWS_AS(sv.reserve(sv.max_size()), std::bad_alloc);
+
+        // A count, not a byte count: the ceiling is what alloc() will let through, and it refuses
+        // anything whose bytes pass PTRDIFF_MAX. std::vector answers the same.
+        REQUIRE(m == static_cast<size_t>(std::numeric_limits<std::ptrdiff_t>::max()) / sizeof(std::string));
+
+        // max_size() itself is a legal size, so this is a real allocation failure rather than a
+        // size error, and stays bad_alloc. One past it is the size error, see reserve_length_error.
+        REQUIRE_THROWS_AS(sv.reserve(opaque(sv.max_size())), std::bad_alloc);
     }
 }
+
+// Anything past max_size() is a size error rather than a failure to find memory, which is what
+// std::vector throws and what this used to get wrong by answering bad_alloc for both.
+TEST_CASE("reserve_length_error") {
+    auto sv = ankerl::svector<std::string, 3>();
+    REQUIRE_THROWS_AS(sv.reserve(opaque(sv.max_size() + 1)), std::length_error);
+    REQUIRE_THROWS_AS(sv.resize(opaque(sv.max_size() + 1)), std::length_error);
+    REQUIRE_THROWS_AS(sv.resize(opaque(sv.max_size() + 1), "x"), std::length_error);
+
+    // and it is still usable afterwards
+    REQUIRE(sv.empty());
+    sv.push_back("a");
+    REQUIRE(sv.size() == 1);
+}
+
+// A count just under max_size() is a legal size, so it goes all the way to the allocator rather
+// than being refused. What is being checked is that the doubling on the way there does not wrap
+// and hand the allocation something small: it is clamped to max_size(), and that is what fails.
+TEST_CASE("growth_below_max_size_reaches_the_allocator") {
+    auto sv = ankerl::svector<uint8_t, 7>();
+    REQUIRE(sv.max_size() == static_cast<size_t>(std::numeric_limits<std::ptrdiff_t>::max()));
+    REQUIRE_THROWS_AS(sv.reserve(opaque(sv.max_size() - 1)), std::bad_alloc);
+    REQUIRE(sv.empty());
+}

test/unit/insert.cpp

@@ -225,7 +225,7 @@ TEST_CASE("insert_count_too_large_throws") {
     // max_size() is the first count that cannot work, huge is the one that used to wrap
     for (auto const count : {huge, ankerl::svector<int, 4>::max_size()}) {
         auto v = ankerl::svector<int, 4>{1, 2, 3};
-        REQUIRE_THROWS_AS(v.insert(v.begin(), count, 5), std::bad_alloc);
+        REQUIRE_THROWS_AS(v.insert(v.begin(), count, 5), std::length_error);
 
         // the failed insert left it alone
         REQUIRE(v.size() == 3);
@@ -240,7 +240,7 @@ TEST_CASE("insert_count_too_large_throws_indirect") {
         v.push_back(i);
     }
 
-    REQUIRE_THROWS_AS(v.insert(v.begin() + 10, (std::numeric_limits<size_t>::max)(), 5), std::bad_alloc);
+    REQUIRE_THROWS_AS(v.insert(v.begin() + 10, (std::numeric_limits<size_t>::max)(), 5), std::length_error);
     REQUIRE(v.size() == 50);
     REQUIRE(v[10] == 10);
 }