From 60ce1d781ea86a7bb3d3f8253bfa6bef9fcc891f Mon Sep 17 00:00:00 2001 From: Martin Leitner-Ankerl Date: Wed, 5 Aug 2026 06:45:12 +0200 Subject: [PATCH 1/4] Say length_error where the standard says length_error, and count elements Two things about size limits that did not match std::vector. max_size() answered PTRDIFF_MAX for every T, which is a byte count wearing a count's clothes. It claimed a size no svector could ever reach, because alloc() refuses anything whose bytes pass PTRDIFF_MAX, so the real ceiling has always been PTRDIFF_MAX/sizeof(T) -- which is what std::vector answers. Still static, for the reason the comment there already gives. And asking for more than that threw bad_alloc, where std::vector throws length_error. They are different questions: one is a size that cannot exist, the other is a size that can but that the allocator would not give. This now draws the same line std::vector does, so reserve(max_size()) is still a bad_alloc -- max_size() is a legal size and the allocation is what fails -- while one past it is a length_error. This is a visible behaviour change for anyone catching bad_alloc around a too-large insert or reserve. The two tests that spelled the old behaviour are updated rather than deleted, and there are new ones for the size error itself and for the growth path clamping at max_size() instead of wrapping. --- include/ankerl/svector.h | 14 ++++++++++---- test/unit/bad_alloc.cpp | 31 ++++++++++++++++++++++++++++++- test/unit/insert.cpp | 4 ++-- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/include/ankerl/svector.h b/include/ankerl/svector.h index d980923..eba512c 100644 --- a/include/ankerl/svector.h +++ b/include/ankerl/svector.h @@ -817,8 +817,10 @@ class svector : private detail::allocator_holder { */ [[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 +1139,7 @@ class svector : private detail::allocator_holder { // 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() - s) { @@ -1760,9 +1762,13 @@ class svector : private detail::allocator_holder { * function has been public and static since before there was one -- svector::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::max)(); + return static_cast((std::numeric_limits::max)()) / sizeof(T); } [[nodiscard]] auto get_allocator() const noexcept -> Allocator { diff --git a/test/unit/bad_alloc.cpp b/test/unit/bad_alloc.cpp index ec3cfff..c03398c 100644 --- a/test/unit/bad_alloc.cpp +++ b/test/unit/bad_alloc.cpp @@ -38,7 +38,36 @@ TEST_CASE("reserve_bad_alloc") { } else { auto sv = ankerl::svector(); auto m = sv.max_size(); - REQUIRE(m == std::numeric_limits::max()); + + // 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(std::numeric_limits::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(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(); + REQUIRE_THROWS_AS(sv.reserve(sv.max_size() + 1), std::length_error); + REQUIRE_THROWS_AS(sv.resize(sv.max_size() + 1), std::length_error); + REQUIRE_THROWS_AS(sv.resize(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); +} + +// The doubling in calculate_new_capacity() overflows long before it reaches a count this large, +// which is the branch that clamps to max_size() rather than wrapping to something small. +TEST_CASE("growth_clamps_at_max_size") { + auto sv = ankerl::svector(); + REQUIRE(sv.max_size() == static_cast(std::numeric_limits::max())); + REQUIRE_THROWS_AS(sv.reserve(sv.max_size() - 1), std::bad_alloc); + REQUIRE(sv.empty()); +} diff --git a/test/unit/insert.cpp b/test/unit/insert.cpp index 948cebf..f512802 100644 --- a/test/unit/insert.cpp +++ b/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::max_size()}) { auto v = ankerl::svector{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::max)(), 5), std::bad_alloc); + REQUIRE_THROWS_AS(v.insert(v.begin() + 10, (std::numeric_limits::max)(), 5), std::length_error); REQUIRE(v.size() == 50); REQUIRE(v[10] == 10); } From 8a9eec12e26211e5e2faaeec2954d9e8481f22ca Mon Sep 17 00:00:00 2001 From: Martin Leitner-Ankerl Date: Wed, 5 Aug 2026 06:48:16 +0200 Subject: [PATCH 2/4] Cover the half of the extended move constructor nobody ran Coverage over the header, aggregated across instantiations, had nine source lines that no test reached. One of them was svector(svector&&, Allocator const&) taking over other's allocation. Every test that reached that constructor named an allocator that did not compare equal, so it always took the other branch and moved the elements one at a time -- including the case that is a compile time yes and needs no allocator at all, which is what every user of the default std::allocator gets. That is new code in 1.3.0, in move and relocation logic, which is where this container's bugs have historically been: #54, #63, #74. It works, but nothing was checking. Now covered, in both storage modes, and the stateful cases prove the takeover rather than assume it: the ledger shows no second allocation and the elements are still at the address they started at. The unequal case is kept alongside so the contrast is visible in one place. Also covers construct_each()'s copy loop, which only runs for an allocator with a construct() of its own, and the new length_error. Three of the nine are gone. The remaining six are all provably unreachable rather than merely untested: two are LCOV_EXCL_LINE already, two are byte overflow guards that max_size() dividing by sizeof(T) now makes impossible to reach, realloc()'s direct to direct return cannot happen because reserve() only calls it when growing and shrink_to_fit() returns before it in direct mode, and calculate_new_capacity()'s wrap clamp cannot fire while max_size() is under PTRDIFF_MAX. They are cheap and they document invariants, so they stay. --- test/unit/allocator.cpp | 89 +++++++++++++++++++++++++++++++++++++++++ test/unit/bad_alloc.cpp | 7 ++-- 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/test/unit/allocator.cpp b/test/unit/allocator.cpp index 1735ce2..0b515a5 100644 --- a/test/unit/allocator.cpp +++ b/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(); + 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::move(a), std::allocator{}); + + // 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(); + a.push_back("one"); + a.push_back("two"); + + auto b = ankerl::svector(std::move(a), std::allocator{}); + 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; + + 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; + + 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 algorithms the default allocator gets. +TEST_CASE("range_construct_through_an_allocator_that_constructs") { + auto const source = std::vector{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}; + + g_constructed = 0; + g_destroyed = 0; + { + auto v = ankerl::svector>(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()); +} diff --git a/test/unit/bad_alloc.cpp b/test/unit/bad_alloc.cpp index c03398c..426a53e 100644 --- a/test/unit/bad_alloc.cpp +++ b/test/unit/bad_alloc.cpp @@ -63,9 +63,10 @@ TEST_CASE("reserve_length_error") { REQUIRE(sv.size() == 1); } -// The doubling in calculate_new_capacity() overflows long before it reaches a count this large, -// which is the branch that clamps to max_size() rather than wrapping to something small. -TEST_CASE("growth_clamps_at_max_size") { +// 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(); REQUIRE(sv.max_size() == static_cast(std::numeric_limits::max())); REQUIRE_THROWS_AS(sv.reserve(sv.max_size() - 1), std::bad_alloc); From bdc4c06668e74c1f34e5ff0900cf5090cc0db626 Mon Sep 17 00:00:00 2001 From: Martin Leitner-Ankerl Date: Wed, 5 Aug 2026 06:54:50 +0200 Subject: [PATCH 3/4] Add the C++23 range members, and bump to 1.4.0 for them The README said svector implements all of std::vector's API. C++23 added assign_range, append_range, insert_range and the from_range constructor, and libstdc++ has had them for a while, so that claim had quietly stopped being true. All four 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, which is what is_input_iterator is built on. views::filter over views::iota is both. One that cannot is built into a temporary first, which costs an allocation for exactly the ranges that could not have been sized anyway. Guarded on __cpp_lib_containers_ranges rather than on the language version, because what these need is std::from_range_t and the range concepts, and a C++17 build has neither. That build is exactly what it was: 126 test cases at C++17 and C++20, 131 at C++23. The constraint is one concept spelled the way the standard spells it rather than a requires clause on each member, which also keeps clang-format from folding the clause onto the declaration. New public API, so the version goes to 1.4.0 by the rule the macros state. --- CMakeLists.txt | 2 +- README.md | 4 + include/ankerl/svector.h | 96 +++++++++++++- meson.build | 2 +- scripts/lint/lint-clang-tidy.py | 6 + test/meson.build | 1 + test/unit/ranges.cpp | 217 ++++++++++++++++++++++++++++++++ 7 files changed, 324 insertions(+), 4 deletions(-) create mode 100644 test/unit/ranges.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index a0d64cc..f981906 100644 --- a/CMakeLists.txt +++ b/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") diff --git a/README.md b/README.md index da8f744..50685d7 100644 --- a/README.md +++ b/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. diff --git a/include/ankerl/svector.h b/include/ankerl/svector.h index eba512c..7b8c0e1 100644 --- a/include/ankerl/svector.h +++ b/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 . @@ -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 #include +#if defined(__has_include) +# if __has_include() +# include +# 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 +# include +#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; template using is_input_iterator = std::is_base_of::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 +struct has_iterator_category : std::false_type {}; + +template +struct has_iterator_category::iterator_category>> : std::true_type {}; + +#if ANKERL_SVECTOR_HAS_RANGES +// The standard calls this container-compatible-range and uses it for exactly these members. +template +concept container_compatible_range = std::ranges::input_range && std::convertible_to, T>; +#endif + constexpr auto round_up(size_t n, size_t multiple) -> size_t { return ((n + (multiple - 1)) / multiple) * multiple; } @@ -1380,6 +1412,20 @@ class svector : private detail::allocator_holder { assign(first, last); } +#if ANKERL_SVECTOR_HAS_RANGES + template R> + svector(std::from_range_t /*unused*/, R&& rg) + : svector() { + append_range(std::forward(rg)); + } + + template R> + svector(std::from_range_t /*unused*/, R&& rg, Allocator const& alloc) + : svector(alloc) { + append_range(std::forward(rg)); + } +#endif + /** * @brief Copying asks the allocator which one the copy should use. * @@ -1922,6 +1968,52 @@ class svector : private detail::allocator_holder { return insert(pos, first, last, typename std::iterator_traits::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 + static constexpr bool is_iterator_pair_range = + std::ranges::common_range && detail::has_iterator_category>::value; + + template R> + auto insert_range(const_iterator pos, R&& rg) -> iterator { + if constexpr (is_iterator_pair_range) { + 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(element)); + } + return insert(pos, std::make_move_iterator(tmp.begin()), std::make_move_iterator(tmp.end())); + } + } + + template R> + void append_range(R&& rg) { + static_cast(insert_range(cend(), std::forward(rg))); + } + + template R> + void assign_range(R&& rg) { + if constexpr (is_iterator_pair_range) { + assign(std::ranges::begin(rg), std::ranges::end(rg)); + } else { + clear(); + append_range(std::forward(rg)); + } + } +#endif + auto insert(const_iterator pos, std::initializer_list l) -> iterator { return insert(pos, l.begin(), l.end()); } diff --git a/meson.build b/meson.build index abc0729..2a468e0 100644 --- a/meson.build +++ b/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', diff --git a/scripts/lint/lint-clang-tidy.py b/scripts/lint/lint-clang-tidy.py index e5d3e3d..07209aa 100755 --- a/scripts/lint/lint-clang-tidy.py +++ b/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 diff --git a/test/meson.build b/test/meson.build index e29e5db..3596133 100644 --- a/test/meson.build +++ b/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', diff --git a/test/unit/ranges.cpp b/test/unit/ranges.cpp new file mode 100644 index 0000000..f29e4a8 --- /dev/null +++ b/test/unit/ranges.cpp @@ -0,0 +1,217 @@ +#include + +#include + +#include + +#include +#include + +// The whole file is C++23 library only. A C++17 or C++20 build has neither std::from_range_t nor +// the range concepts these need, and svector deliberately does not grow the members there either. +#if ANKERL_SVECTOR_HAS_RANGES + +# include +# include +# include + +namespace { + +// A range whose sentinel is not its iterator and whose iterator publishes no iterator_category, so +// it cannot be handed to the iterator pair members and takes the materialising path instead. +auto even_numbers(int count) { + return std::views::iota(0) | std::views::filter([](int i) { + return i % 2 == 0; + }) | + std::views::take(count); +} + +auto long_strings(size_t count) -> std::vector { + auto v = std::vector(); + for (size_t i = 0; i < count; ++i) { + v.push_back("element " + std::to_string(i) + ", long enough that it has to allocate for itself"); + } + return v; +} + +} // namespace + +TEST_CASE("from_range_construction") { + SUBCASE("common range, stays inline") { + auto const source = std::vector{1, 2, 3}; + auto v = ankerl::svector(std::from_range, source); + REQUIRE(v.size() == 3); + REQUIRE(std::equal(v.begin(), v.end(), source.begin(), source.end())); + } + + SUBCASE("common range, has to allocate") { + auto const source = long_strings(50); + auto v = ankerl::svector(std::from_range, source); + REQUIRE(v.size() == 50); + REQUIRE(std::equal(v.begin(), v.end(), source.begin(), source.end())); + } + + SUBCASE("range with a sentinel and no iterator_category") { + auto v = ankerl::svector(std::from_range, even_numbers(6)); + REQUIRE(v.size() == 6); + REQUIRE(std::equal(v.begin(), v.end(), std::vector{0, 2, 4, 6, 8, 10}.begin())); + } + + SUBCASE("input only range") { + auto source = std::forward_list{4, 5, 6}; + auto v = ankerl::svector(std::from_range, source); + REQUIRE(v.size() == 3); + REQUIRE(v[0] == 4); + REQUIRE(v[2] == 6); + } + + SUBCASE("with a named allocator") { + auto const source = std::vector{1, 2, 3}; + auto v = ankerl::svector(std::from_range, source, std::allocator{}); + REQUIRE(v.size() == 3); + REQUIRE(v[1] == 2); + } + + SUBCASE("empty range") { + auto v = ankerl::svector(std::from_range, std::vector{}); + REQUIRE(v.empty()); + } +} + +TEST_CASE("append_range") { + SUBCASE("onto empty and then across the inline boundary") { + auto v = ankerl::svector(); + v.append_range(std::vector{1, 2}); + REQUIRE(v.size() == 2); + + v.append_range(std::list{3, 4, 5, 6, 7}); + REQUIRE(v.size() == 7); + REQUIRE(std::equal(v.begin(), v.end(), std::vector{1, 2, 3, 4, 5, 6, 7}.begin())); + } + + SUBCASE("a sentinel range appends too") { + auto v = ankerl::svector{100}; + v.append_range(even_numbers(3)); + REQUIRE(v.size() == 4); + REQUIRE(std::equal(v.begin(), v.end(), std::vector{100, 0, 2, 4}.begin())); + } + + SUBCASE("appending nothing changes nothing") { + auto v = ankerl::svector{1, 2, 3}; + v.append_range(std::vector{}); + REQUIRE(v.size() == 3); + REQUIRE(v[2] == 3); + } + + SUBCASE("strings, so construction and destruction are observable") { + auto counts = Counter(); + { + auto v = ankerl::svector(); + v.append_range(long_strings(30)); + REQUIRE(v.size() == 30); + REQUIRE(v[29] == long_strings(30)[29]); + } + static_cast(counts); + } +} + +TEST_CASE("insert_range") { + auto const source = std::vector{7, 8}; + + SUBCASE("at the front") { + auto v = ankerl::svector{1, 2, 3}; + auto it = v.insert_range(v.begin(), source); + REQUIRE(*it == 7); + REQUIRE(std::equal(v.begin(), v.end(), std::vector{7, 8, 1, 2, 3}.begin())); + } + + SUBCASE("in the middle") { + auto v = ankerl::svector{1, 2, 3}; + v.insert_range(v.begin() + 1, source); + REQUIRE(std::equal(v.begin(), v.end(), std::vector{1, 7, 8, 2, 3}.begin())); + } + + SUBCASE("at the end") { + auto v = ankerl::svector{1, 2, 3}; + v.insert_range(v.end(), source); + REQUIRE(std::equal(v.begin(), v.end(), std::vector{1, 2, 3, 7, 8}.begin())); + } + + SUBCASE("forcing a reallocation") { + auto v = ankerl::svector{1, 2}; + v.insert_range(v.begin() + 1, std::vector{3, 4, 5, 6, 7, 8}); + REQUIRE(std::equal(v.begin(), v.end(), std::vector{1, 3, 4, 5, 6, 7, 8, 2}.begin())); + } + + SUBCASE("a sentinel range in the middle") { + auto v = ankerl::svector{1, 2, 3}; + v.insert_range(v.begin() + 1, even_numbers(2)); + REQUIRE(std::equal(v.begin(), v.end(), std::vector{1, 0, 2, 2, 3}.begin())); + } + + SUBCASE("inserting nothing is not an erase") { + auto v = ankerl::svector{1, 2, 3}; + v.insert_range(v.begin() + 1, std::vector{}); + REQUIRE(std::equal(v.begin(), v.end(), std::vector{1, 2, 3}.begin())); + } +} + +TEST_CASE("assign_range") { + SUBCASE("replaces, growing past inline") { + auto v = ankerl::svector{1, 2}; + v.assign_range(std::vector{5, 6, 7, 8, 9, 10}); + REQUIRE(v.size() == 6); + REQUIRE(std::equal(v.begin(), v.end(), std::vector{5, 6, 7, 8, 9, 10}.begin())); + } + + SUBCASE("replaces, shrinking") { + auto v = ankerl::svector(); + v.append_range(std::vector(50, 3)); + v.assign_range(std::vector{1}); + REQUIRE(v.size() == 1); + REQUIRE(v[0] == 1); + } + + SUBCASE("assigning an empty range clears") { + auto v = ankerl::svector{1, 2, 3}; + v.assign_range(std::vector{}); + REQUIRE(v.empty()); + } + + SUBCASE("a sentinel range assigns too") { + auto v = ankerl::svector{1, 2, 3, 4, 5}; + v.assign_range(even_numbers(3)); + REQUIRE(v.size() == 3); + REQUIRE(std::equal(v.begin(), v.end(), std::vector{0, 2, 4}.begin())); + } + + SUBCASE("strings") { + auto v = ankerl::svector{"x"}; + v.assign_range(long_strings(20)); + REQUIRE(v.size() == 20); + REQUIRE(v[0] == long_strings(20)[0]); + } +} + +// Whatever std::vector does with the same calls, svector does too. +TEST_CASE("range_members_agree_with_std_vector") { + auto const source = long_strings(25); + + auto sv = ankerl::svector(std::from_range, source); + auto v = std::vector(std::from_range, source); + REQUIRE(std::equal(sv.begin(), sv.end(), v.begin(), v.end())); + + sv.append_range(source); + v.append_range(source); + REQUIRE(std::equal(sv.begin(), sv.end(), v.begin(), v.end())); + + sv.insert_range(sv.begin() + 5, source); + v.insert_range(v.begin() + 5, source); + REQUIRE(std::equal(sv.begin(), sv.end(), v.begin(), v.end())); + + sv.assign_range(source); + v.assign_range(source); + REQUIRE(std::equal(sv.begin(), sv.end(), v.begin(), v.end())); +} + +#endif From 07d3bfd7ab01dd1d8f39dba5c4846f743ae7c417 Mon Sep 17 00:00:00 2001 From: Martin Leitner-Ankerl Date: Wed, 5 Aug 2026 06:58:36 +0200 Subject: [PATCH 4/4] Keep a max_size() derived size away from the optimizer The hardened leg broke on the new tests, and not for a reason about svector. max_size() answering PTRDIFF_MAX/sizeof(T) instead of PTRDIFF_MAX made it small enough for gcc to constant fold, so at -O2 it followed max_size() + 1 into reasoning about an array of that many std::string and reported an out of bounds subscript, which -Werror turned into a failure. gcc 13 then hit an internal compiler error on the same test. Nothing about that is wrong with the code under test: a size like this is a runtime value in any real use. So it is one here too, through a volatile, which is also what keeps the existing reserve_bad_alloc test out of the same trap now that its argument folds as well. --- test/unit/bad_alloc.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/test/unit/bad_alloc.cpp b/test/unit/bad_alloc.cpp index 426a53e..8e40f3b 100644 --- a/test/unit/bad_alloc.cpp +++ b/test/unit/bad_alloc.cpp @@ -32,6 +32,16 @@ # 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. @@ -45,7 +55,7 @@ TEST_CASE("reserve_bad_alloc") { // 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(sv.max_size()), std::bad_alloc); + REQUIRE_THROWS_AS(sv.reserve(opaque(sv.max_size())), std::bad_alloc); } } @@ -53,9 +63,9 @@ TEST_CASE("reserve_bad_alloc") { // 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(); - REQUIRE_THROWS_AS(sv.reserve(sv.max_size() + 1), std::length_error); - REQUIRE_THROWS_AS(sv.resize(sv.max_size() + 1), std::length_error); - REQUIRE_THROWS_AS(sv.resize(sv.max_size() + 1, "x"), std::length_error); + 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()); @@ -69,6 +79,6 @@ TEST_CASE("reserve_length_error") { TEST_CASE("growth_below_max_size_reaches_the_allocator") { auto sv = ankerl::svector(); REQUIRE(sv.max_size() == static_cast(std::numeric_limits::max())); - REQUIRE_THROWS_AS(sv.reserve(sv.max_size() - 1), std::bad_alloc); + REQUIRE_THROWS_AS(sv.reserve(opaque(sv.max_size() - 1)), std::bad_alloc); REQUIRE(sv.empty()); }