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 d980923..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; } @@ -817,8 +849,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 +1171,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) { @@ -1378,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. * @@ -1760,9 +1808,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 { @@ -1916,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/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 ec3cfff..8e40f3b 100644 --- a/test/unit/bad_alloc.cpp +++ b/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(); auto m = sv.max_size(); - REQUIRE(m == std::numeric_limits::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(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(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(); + 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(); + REQUIRE(sv.max_size() == static_cast(std::numeric_limits::max())); + REQUIRE_THROWS_AS(sv.reserve(opaque(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); } 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