From 70ca08451fe825b274ab01dfb6a9d359bd7565ae Mon Sep 17 00:00:00 2001 From: Nana Sakisaka <1901813+saki7@users.noreply.github.com> Date: Mon, 5 Oct 2026 12:33:43 +0900 Subject: [PATCH 1/5] Implement `transfer_from` --- include/iris/container_traits.hpp | 113 +++++++++++++++++++++++++++++- 1 file changed, 112 insertions(+), 1 deletion(-) diff --git a/include/iris/container_traits.hpp b/include/iris/container_traits.hpp index 47a0b19..5519db7 100644 --- a/include/iris/container_traits.hpp +++ b/include/iris/container_traits.hpp @@ -8,14 +8,38 @@ #include // IWYU pragma: keep #include // IWYU pragma: keep +#include #include #include #include #include #include +#include + namespace iris::container { +namespace detail { + +template +concept allocator_aware_impl = requires(T const& c) { + typename T::allocator_type; + { c.get_allocator() } -> std::same_as; +}; + +} // detail + +template +concept allocator_aware = detail::allocator_aware_impl>; + +template +concept has_stateless_allocator = std::allocator_traits::allocator_type>::is_always_equal::value; + +template +concept has_same_allocator_type = + allocator_aware && allocator_aware && + std::same_as::allocator_type, typename std::remove_cvref_t::allocator_type>; + template concept mapping_container = ranges::mapping_range && @@ -584,7 +608,6 @@ struct erase_back_fn template concept front_erasable = requires(ContainerT& cont) { erase_front(cont); }; template concept back_erasable = requires(ContainerT& cont) { erase_back(cont); }; - // ------------------------------------------------------------ template @@ -729,6 +752,94 @@ struct clear_fn [[maybe_unused]] inline constexpr detail::append_range_fn append_range{}; [[maybe_unused]] inline constexpr detail::clear_fn clear{}; +// ------------------------------------------------------------ + +namespace detail { + +template +concept element_wise_transferable_from = + requires(DstT& dst, SrcT src) { + append_range(dst, std::forward(src) | std::views::as_rvalue); + }; + +template +concept has_splice = requires(DstT& dst, SrcT src) { + dst.splice(std::ranges::end(dst), std::forward(src)); +}; + +template +concept has_merge = requires(DstT& dst, SrcT src) { + typename std::remove_cvref_t::key_type; // `std::list::merge` merges sorted lists + dst.merge(std::forward(src)); +}; + +template +concept node_family_transferable_from = + has_splice || + has_merge; + // TODO: `std::flat_map`, `std::flat_set` + +template +concept node_transferable_from = + has_same_allocator_type && + node_family_transferable_from && + ( + has_stateless_allocator || + // When we merge containers with stateful allocator, it is + // UB unless `dst.get_allocator() == src.get_allocator()`. + // Which means we must ensure they *always* have well-defined + // element-wise operation available for fallback. + element_wise_transferable_from + ); + +struct transfer_from_fn +{ + template + requires (!node_transferable_from) && element_wise_transferable_from + static constexpr void operator()(DstT& dst, SrcT&& src) + { + assert( + static_cast(std::addressof(src)) != static_cast(std::addressof(dst)) && + "self-transfer cannot be defined as a generic operation, even though some " + "containers define it" + ); + append_range(dst, std::forward(src) | std::views::as_rvalue); + } + + template + requires node_transferable_from + static constexpr void operator()(DstT& dst, SrcT&& src) + { + assert( + static_cast(std::addressof(src)) != static_cast(std::addressof(dst)) && + "self-transfer cannot be defined as a generic operation, even though some " + "containers define it" + ); + + if constexpr (!has_stateless_allocator) { + if (dst.get_allocator() != src.get_allocator()) { + append_range(dst, std::forward(src) | std::views::as_rvalue); + return; + } + } + + if constexpr (has_splice) { + dst.splice(std::ranges::end(dst), std::forward(src)); + + } else if constexpr (has_merge) { + dst.merge(std::forward(src)); + + } else { + static_assert(false); + } + } +}; + +} // detail + +[[maybe_unused]] inline constexpr detail::transfer_from_fn transfer_from{}; + + } // iris::container namespace iris::container::dummy { From 5fb72282b39da7ce1c967662db3399f7e3327dc4 Mon Sep 17 00:00:00 2001 From: Nana Sakisaka <1901813+saki7@users.noreply.github.com> Date: Mon, 5 Oct 2026 13:42:51 +0900 Subject: [PATCH 2/5] Add tests for `container::transfer_from` --- include/iris/container_traits.hpp | 76 +++++++++++++++++++- test/container_traits.cpp | 113 ++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 3 deletions(-) diff --git a/include/iris/container_traits.hpp b/include/iris/container_traits.hpp index 5519db7..a7b926b 100644 --- a/include/iris/container_traits.hpp +++ b/include/iris/container_traits.hpp @@ -35,6 +35,9 @@ concept allocator_aware = detail::allocator_aware_impl>; template concept has_stateless_allocator = std::allocator_traits::allocator_type>::is_always_equal::value; +template +concept has_interchangeable_allocator = !allocator_aware || has_stateless_allocator; + template concept has_same_allocator_type = allocator_aware && allocator_aware && @@ -777,7 +780,54 @@ template concept node_family_transferable_from = has_splice || has_merge; - // TODO: `std::flat_map`, `std::flat_set` + +template +using extracted_t = decltype(std::declval&&>().extract()); + +template +[[nodiscard]] constexpr auto moved_elements(Extracted& extracted) noexcept +{ + if constexpr (requires { extracted.keys; extracted.values; }) { + // `std::views::zip` would be simpler, but Clang 22 rejects inserting two kinds of `zip_view` into libc++'s + // flat containers in one translation unit (an access check on a constraint of `__product_iterator_traits`). + using key_type = std::ranges::range_value_t; + using mapped_type = std::ranges::range_value_t; + return std::views::zip_transform( + [](auto&& key, auto&& value) { return std::pair(std::move(key), std::move(value)); }, + extracted.keys, extracted.values + ); + } else { + return extracted | std::views::as_rvalue; + } +} + +template +concept flat_transferable_from = + ( + std::same_as, typename std::remove_cvref_t::containers> || + std::same_as, typename std::remove_cvref_t::container_type> + ) && + requires(DstT& dst, extracted_t& extracted) { + append_range(dst, detail::moved_elements(extracted)); + }; + +// An empty destination takes the underlying containers as they are. They are sorted by the source's +// comparator, which orders the same as the destination's only when the comparator has no state. +template +concept flat_replaceable_from = + std::same_as, std::remove_cvref_t> && + std::is_empty_v::key_compare> && + ( + requires(DstT& dst, extracted_t& extracted) { + requires has_interchangeable_allocator; + requires has_interchangeable_allocator; + dst.replace(std::move(extracted.keys), std::move(extracted.values)); + } || + requires(DstT& dst, extracted_t& extracted) { + requires has_interchangeable_allocator>; + dst.replace(std::move(extracted)); + } + ); template concept node_transferable_from = @@ -795,7 +845,9 @@ concept node_transferable_from = struct transfer_from_fn { template - requires (!node_transferable_from) && element_wise_transferable_from + requires + (!node_transferable_from) && + (flat_transferable_from || element_wise_transferable_from) static constexpr void operator()(DstT& dst, SrcT&& src) { assert( @@ -803,7 +855,25 @@ struct transfer_from_fn "self-transfer cannot be defined as a generic operation, even though some " "containers define it" ); - append_range(dst, std::forward(src) | std::views::as_rvalue); + + if constexpr (flat_transferable_from) { + auto extracted = std::move(src).extract(); + + if constexpr (flat_replaceable_from) { + if (std::ranges::empty(dst)) { + if constexpr (requires { extracted.keys; extracted.values; }) { + dst.replace(std::move(extracted.keys), std::move(extracted.values)); + } else { + dst.replace(std::move(extracted)); + } + return; + } + } + append_range(dst, detail::moved_elements(extracted)); + + } else { + append_range(dst, std::forward(src) | std::views::as_rvalue); + } } template diff --git a/test/container_traits.cpp b/test/container_traits.cpp index 0590bf0..16e1888 100644 --- a/test/container_traits.cpp +++ b/test/container_traits.cpp @@ -17,6 +17,10 @@ #include // TODO #include #include +#include +#include +#include +#include #include #include #include @@ -845,6 +849,115 @@ TEST_CASE("container: append_range") STATIC_CHECK(!std::invocable&, std::vector>); } +// orders in reverse when `is_reversed`, so that two objects of this type can order differently +struct stateful_less +{ + bool is_reversed = false; + bool operator()(int a, int b) const { return is_reversed ? b < a : a < b; } +}; + +TEST_CASE("container: transfer_from") +{ + using iris::container::transfer_from; + + { + std::list dst{"a"}, src{"b", "c"}; + auto const* const node = &src.front(); + transfer_from(dst, src); + CHECK(dst == std::list{"a", "b", "c"}); + CHECK(&*std::next(dst.begin()) == node); + CHECK(src.empty()); + } + { + std::map dst{{1, "a"}}; + std::multimap src{{1, "b"}, {2, "c"}}; + auto const* const node = &src.find(2)->second; + transfer_from(dst, src); + CHECK(dst == std::map{{1, "a"}, {2, "c"}}); + CHECK(&dst.at(2) == node); + CHECK(src == std::multimap{{1, "b"}}); + } + { + std::unordered_map dst{{1, 10}}; + transfer_from(dst, std::unordered_map{{1, 20}, {2, 30}}); + CHECK(dst == std::unordered_map{{1, 10}, {2, 30}}); + } + + // element-wise + { + std::vector dst{"a"}, src{"b"}; + transfer_from(dst, src); + CHECK(dst == std::vector{"a", "b"}); + CHECK(src == std::vector{""}); + } + { + std::map dst{{1, "a"}}; + std::vector> src{{1, "b"}, {2, "c"}}; + transfer_from(dst, src); + CHECK(dst == std::map{{1, "a"}, {2, "c"}}); + } + { + // nodes are not moved between different memory resources + std::pmr::monotonic_buffer_resource dst_resource, src_resource; + std::pmr::list dst(&dst_resource), src({1, 2}, &src_resource); + auto const* const node = &src.front(); + transfer_from(dst, src); + CHECK(dst == std::pmr::list{1, 2}); + CHECK(&dst.front() != node); + CHECK(dst.get_allocator().resource() == &dst_resource); + + std::pmr::list same_resource_src({3}, &dst_resource); + auto const* const same_resource_node = &same_resource_src.front(); + transfer_from(dst, same_resource_src); + CHECK(&dst.back() == same_resource_node); + } + + // flat containers + { + std::flat_map dst, src{{"a", 1}, {"b", 2}}; + auto const* const keys = src.keys().data(); + transfer_from(dst, src); + CHECK(dst == std::flat_map{{"a", 1}, {"b", 2}}); + CHECK(dst.keys().data() == keys); + CHECK(src.empty()); + + std::flat_map more{{"a", 3}, {"c", 4}}; + transfer_from(dst, more); + CHECK(dst == std::flat_map{{"a", 1}, {"b", 2}, {"c", 4}}); + } + { + std::flat_set dst, src{"b", "a"}; + transfer_from(dst, src); + CHECK(dst == std::flat_set{"a", "b"}); + + transfer_from(dst, std::flat_set{"c", "a"}); + CHECK(dst == std::flat_set{"a", "b", "c"}); + } + { + std::flat_multimap dst{{1, 10}}; + transfer_from(dst, std::flat_multimap{{1, 20}, {2, 30}}); + CHECK(dst == std::flat_multimap{{1, 10}, {1, 20}, {2, 30}}); + } + { + // a flat container yields its mapped values as lvalues, so they are moved out of the underlying containers + std::map> dst; + std::flat_map> src; + src.emplace("a", std::make_unique(1)); + transfer_from(dst, src); + REQUIRE(dst.contains("a")); + CHECK(*dst.at("a") == 1); + } + { + // the source is sorted by its own comparator, so an empty destination does not take it as it is + std::flat_set dst(stateful_less{false}), src({1, 2, 3}, stateful_less{true}); + transfer_from(dst, src); + CHECK(std::ranges::equal(dst, std::vector{1, 2, 3})); + } + + STATIC_CHECK(!std::invocable&, std::vector&>); + STATIC_CHECK(!std::invocable&, std::vector&>); +} + TEST_CASE("container: clear") { std::vector v{1, 2}; From 153f40ab6448b36ee717bc64a787ca7e936086e9 Mon Sep 17 00:00:00 2001 From: Nana Sakisaka <1901813+saki7@users.noreply.github.com> Date: Mon, 5 Oct 2026 14:10:33 +0900 Subject: [PATCH 3/5] Optimize transfer on flat_map/flat_set --- include/iris/container_traits.hpp | 83 ++++++++++++++++++++++--------- test/container_traits.cpp | 64 ++++++++++++++++++++++-- 2 files changed, 121 insertions(+), 26 deletions(-) diff --git a/include/iris/container_traits.hpp b/include/iris/container_traits.hpp index a7b926b..2a43bd8 100644 --- a/include/iris/container_traits.hpp +++ b/include/iris/container_traits.hpp @@ -8,10 +8,12 @@ #include // IWYU pragma: keep #include // IWYU pragma: keep +#include #include #include #include #include +#include #include #include @@ -36,7 +38,10 @@ template concept has_stateless_allocator = std::allocator_traits::allocator_type>::is_always_equal::value; template -concept has_interchangeable_allocator = !allocator_aware || has_stateless_allocator; +concept keeps_allocator_on_move_assignment = + !allocator_aware || + has_stateless_allocator || + !std::allocator_traits::allocator_type>::propagate_on_container_move_assignment::value; template concept has_same_allocator_type = @@ -811,23 +816,16 @@ concept flat_transferable_from = append_range(dst, detail::moved_elements(extracted)); }; -// An empty destination takes the underlying containers as they are. They are sorted by the source's -// comparator, which orders the same as the destination's only when the comparator has no state. template -concept flat_replaceable_from = +concept flat_sorted_transferable_from = + flat_transferable_from && std::same_as, std::remove_cvref_t> && - std::is_empty_v::key_compare> && - ( - requires(DstT& dst, extracted_t& extracted) { - requires has_interchangeable_allocator; - requires has_interchangeable_allocator; - dst.replace(std::move(extracted.keys), std::move(extracted.values)); - } || - requires(DstT& dst, extracted_t& extracted) { - requires has_interchangeable_allocator>; - dst.replace(std::move(extracted)); - } - ); + std::is_empty_v::key_compare>; + +template +concept has_unique_keys = requires(ContainerT& cont, std::ranges::range_value_t&& value) { + cont.insert(std::move(value)).second; +}; template concept node_transferable_from = @@ -856,19 +854,58 @@ struct transfer_from_fn "containers define it" ); - if constexpr (flat_transferable_from) { + if constexpr (flat_sorted_transferable_from) { auto extracted = std::move(src).extract(); - - if constexpr (flat_replaceable_from) { - if (std::ranges::empty(dst)) { - if constexpr (requires { extracted.keys; extracted.values; }) { + auto const comp = dst.key_comp(); + auto const is_equivalent = [&comp](auto const& a, auto const& b) { return !comp(a, b) && !comp(b, a); }; + + if constexpr (requires { extracted.keys; extracted.values; }) { + if constexpr ( + keeps_allocator_on_move_assignment && + keeps_allocator_on_move_assignment + ) { + if (std::ranges::empty(dst)) { dst.replace(std::move(extracted.keys), std::move(extracted.values)); - } else { + return; + } + } + auto merged = std::move(dst).extract(); + auto const middle = std::ranges::ssize(merged.keys); + append_range(merged.keys, extracted.keys | std::views::as_rvalue); + append_range(merged.values, extracted.values | std::views::as_rvalue); + + auto zipped = std::views::zip(merged.keys, merged.values); + auto const key = [](auto const& element) noexcept -> auto const& { return std::get<0>(element); }; + std::ranges::inplace_merge(zipped, std::ranges::begin(zipped) + middle, comp, key); + if constexpr (has_unique_keys>) { + auto const unique_end = std::ranges::begin(std::ranges::unique(zipped, is_equivalent, key)); + auto const size = unique_end - std::ranges::begin(zipped); + merged.keys.erase(std::ranges::begin(merged.keys) + size, std::ranges::end(merged.keys)); + merged.values.erase(std::ranges::begin(merged.values) + size, std::ranges::end(merged.values)); + } + dst.replace(std::move(merged.keys), std::move(merged.values)); + + } else { + if constexpr (keeps_allocator_on_move_assignment) { + if (std::ranges::empty(dst)) { dst.replace(std::move(extracted)); + return; } - return; } + auto merged = std::move(dst).extract(); + auto const middle = std::ranges::ssize(merged); + append_range(merged, extracted | std::views::as_rvalue); + + std::ranges::inplace_merge(merged, std::ranges::begin(merged) + middle, comp); + if constexpr (has_unique_keys>) { + auto const removed = std::ranges::unique(merged, is_equivalent); + merged.erase(std::ranges::begin(removed), std::ranges::end(removed)); + } + dst.replace(std::move(merged)); } + + } else if constexpr (flat_transferable_from) { + auto extracted = std::move(src).extract(); append_range(dst, detail::moved_elements(extracted)); } else { diff --git a/test/container_traits.cpp b/test/container_traits.cpp index 16e1888..3151501 100644 --- a/test/container_traits.cpp +++ b/test/container_traits.cpp @@ -849,6 +849,27 @@ TEST_CASE("container: append_range") STATIC_CHECK(!std::invocable&, std::vector>); } +// propagates on move assignment, and two objects compare equal only when they have the same tag +template +struct tagged_allocator +{ + using value_type = T; + using propagate_on_container_move_assignment = std::true_type; + + int tag = 0; + + tagged_allocator() = default; + explicit tagged_allocator(int tag) noexcept : tag(tag) {} + template + tagged_allocator(tagged_allocator const& other) noexcept : tag(other.tag) {} // NOLINT(google-explicit-constructor) + + T* allocate(std::size_t n) { return std::allocator{}.allocate(n); } + void deallocate(T* p, std::size_t n) noexcept { std::allocator{}.deallocate(p, n); } + + template + bool operator==(tagged_allocator const& other) const noexcept { return tag == other.tag; } +}; + // orders in reverse when `is_reversed`, so that two objects of this type can order differently struct stateful_less { @@ -897,7 +918,7 @@ TEST_CASE("container: transfer_from") CHECK(dst == std::map{{1, "a"}, {2, "c"}}); } { - // nodes are not moved between different memory resources + // Nodes are not moved between different memory resources std::pmr::monotonic_buffer_resource dst_resource, src_resource; std::pmr::list dst(&dst_resource), src({1, 2}, &src_resource); auto const* const node = &src.front(); @@ -939,7 +960,7 @@ TEST_CASE("container: transfer_from") CHECK(dst == std::flat_multimap{{1, 10}, {1, 20}, {2, 30}}); } { - // a flat container yields its mapped values as lvalues, so they are moved out of the underlying containers + // A flat container yields its mapped values as lvalues, so they are moved out of the underlying containers std::map> dst; std::flat_map> src; src.emplace("a", std::make_unique(1)); @@ -948,7 +969,44 @@ TEST_CASE("container: transfer_from") CHECK(*dst.at("a") == 1); } { - // the source is sorted by its own comparator, so an empty destination does not take it as it is + // The destination keeps the memory resource of its underlying containers + using pmr_flat_map = std::flat_map, std::pmr::vector, std::pmr::vector>; + std::pmr::monotonic_buffer_resource dst_resource, src_resource; + pmr_flat_map dst{std::pmr::polymorphic_allocator(&dst_resource)}, src{std::pmr::polymorphic_allocator(&src_resource)}; + src.emplace(2, 20); + src.emplace(1, 10); + transfer_from(dst, src); + CHECK(dst == pmr_flat_map{{1, 10}, {2, 20}}); + CHECK(dst.keys().get_allocator().resource() == &dst_resource); + + pmr_flat_map more{std::pmr::polymorphic_allocator(&src_resource)}; + more.emplace(0, 0); + more.emplace(2, 200); + transfer_from(dst, more); + CHECK(dst == pmr_flat_map{{0, 0}, {1, 10}, {2, 20}}); + CHECK(dst.values().get_allocator().resource() == &dst_resource); + } + { + // An empty destination takes the buffers of a source that has an equal memory resource + using pmr_flat_set = std::flat_set, std::pmr::vector>; + std::pmr::monotonic_buffer_resource resource; + pmr_flat_set dst{std::pmr::polymorphic_allocator(&resource)}, src{std::pmr::polymorphic_allocator(&resource)}; + src.insert(1); + auto const* const element = &*src.begin(); + transfer_from(dst, src); + CHECK(&*dst.begin() == element); + } + { + // An allocator that propagates on move assignment would replace the allocator of an empty destination + using tagged_flat_set = std::flat_set, std::vector>>; + tagged_flat_set dst{tagged_allocator(1)}, src{tagged_allocator(2)}; + src.insert(1); + transfer_from(dst, src); + CHECK(dst == tagged_flat_set{1}); + CHECK(std::move(dst).extract().get_allocator().tag == 1); + } + { + // The source is sorted by its own comparator, which the destination cannot rely on std::flat_set dst(stateful_less{false}), src({1, 2, 3}, stateful_less{true}); transfer_from(dst, src); CHECK(std::ranges::equal(dst, std::vector{1, 2, 3})); From 00dad74845552bd6423e02bf7db96f5f0f512bdd Mon Sep 17 00:00:00 2001 From: Nana Sakisaka <1901813+saki7@users.noreply.github.com> Date: Mon, 5 Oct 2026 14:32:31 +0900 Subject: [PATCH 4/5] Optimize `try_emplace` --- include/iris/container_traits.hpp | 18 +++++++++++++++++- test/container_traits.cpp | 16 ++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/include/iris/container_traits.hpp b/include/iris/container_traits.hpp index 2a43bd8..d9ec52a 100644 --- a/include/iris/container_traits.hpp +++ b/include/iris/container_traits.hpp @@ -449,6 +449,15 @@ concept back_emplace_back_accessible = (has_end_emplace || has_end_insert) ); +template +concept try_emplaceable = + unique_mapping_container && + std::is_constructible_v(std::declval()))> && + std::is_constructible_v(std::declval()))> && + requires(ContainerT& cont, ElementT&& element) { + cont.try_emplace(std::ranges::end(cont), std::get<0>(std::forward(element)), std::get<1>(std::forward(element))); + }; + template struct append_fn { @@ -498,7 +507,14 @@ struct append_fn static constexpr decltype(auto) operator()(ContainerT& cont, FirstT&& first, Rest&&... rest) { - if constexpr (has_emplace_back) { + if constexpr (sizeof...(Rest) == 0 && try_emplaceable) { + if constexpr (NeedReturn) { + return *cont.try_emplace(std::ranges::end(cont), std::forward_like(first.first), std::forward_like(first.second)); + } else { + (void)cont.try_emplace(std::ranges::end(cont), std::forward_like(first.first), std::forward_like(first.second)); + } + + } else if constexpr (has_emplace_back) { if constexpr (NeedReturn) { if constexpr (std::is_void_v(first), std::forward(rest)...))>) { cont.emplace_back(std::forward(first), std::forward(rest)...); diff --git a/test/container_traits.cpp b/test/container_traits.cpp index 3151501..b05f3af 100644 --- a/test/container_traits.cpp +++ b/test/container_traits.cpp @@ -817,6 +817,22 @@ TEST_CASE("container: append into associative containers") std::map m; iris::container::append(m, std::pair{1, 10}); CHECK(m == std::map{{1, 10}}); + + std::map strings{{"k", "a"}}; + std::pair duplicate{"k", "b"}; + iris::container::append(strings, std::move(duplicate)); + CHECK(strings == std::map{{"k", "a"}}); + CHECK(duplicate == std::pair{"k", "b"}); // NOLINT(bugprone-use-after-move) + CHECK(&iris::container::append_return(strings, std::pair{"k", "c"}) == &*strings.begin()); + + std::flat_map flat{{"k", "a"}}; + iris::container::append(flat, std::move(duplicate)); + CHECK(flat == std::flat_map{{"k", "a"}}); + CHECK(duplicate == std::pair{"k", "b"}); // NOLINT(bugprone-use-after-move) + + std::map converted; + iris::container::append(converted, std::pair{"k", 1}); + CHECK(converted == std::map{{"k", 1}}); } TEST_CASE("container: append_range") From 43127a1facd1418851867d1326262af22548f137 Mon Sep 17 00:00:00 2001 From: Nana Sakisaka <1901813+saki7@users.noreply.github.com> Date: Mon, 5 Oct 2026 14:46:47 +0900 Subject: [PATCH 5/5] Workaround `std::ranges::inplace_merge` on libstdc++ --- include/iris/container_traits.hpp | 51 +++++++++++++++++++++---------- test/container_traits.cpp | 13 ++++---- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/include/iris/container_traits.hpp b/include/iris/container_traits.hpp index d9ec52a..a35e8f3 100644 --- a/include/iris/container_traits.hpp +++ b/include/iris/container_traits.hpp @@ -13,7 +13,6 @@ #include #include #include -#include #include #include @@ -509,9 +508,9 @@ struct append_fn { if constexpr (sizeof...(Rest) == 0 && try_emplaceable) { if constexpr (NeedReturn) { - return *cont.try_emplace(std::ranges::end(cont), std::forward_like(first.first), std::forward_like(first.second)); + return *cont.try_emplace(std::ranges::end(cont), std::get<0>(std::forward(first)), std::get<1>(std::forward(first))); } else { - (void)cont.try_emplace(std::ranges::end(cont), std::forward_like(first.first), std::forward_like(first.second)); + (void)cont.try_emplace(std::ranges::end(cont), std::get<0>(std::forward(first)), std::get<1>(std::forward(first))); } } else if constexpr (has_emplace_back) { @@ -873,7 +872,6 @@ struct transfer_from_fn if constexpr (flat_sorted_transferable_from) { auto extracted = std::move(src).extract(); auto const comp = dst.key_comp(); - auto const is_equivalent = [&comp](auto const& a, auto const& b) { return !comp(a, b) && !comp(b, a); }; if constexpr (requires { extracted.keys; extracted.values; }) { if constexpr ( @@ -885,19 +883,39 @@ struct transfer_from_fn return; } } - auto merged = std::move(dst).extract(); - auto const middle = std::ranges::ssize(merged.keys); - append_range(merged.keys, extracted.keys | std::views::as_rvalue); - append_range(merged.values, extracted.values | std::views::as_rvalue); - auto zipped = std::views::zip(merged.keys, merged.values); - auto const key = [](auto const& element) noexcept -> auto const& { return std::get<0>(element); }; - std::ranges::inplace_merge(zipped, std::ranges::begin(zipped) + middle, comp, key); - if constexpr (has_unique_keys>) { - auto const unique_end = std::ranges::begin(std::ranges::unique(zipped, is_equivalent, key)); - auto const size = unique_end - std::ranges::begin(zipped); - merged.keys.erase(std::ranges::begin(merged.keys) + size, std::ranges::end(merged.keys)); - merged.values.erase(std::ranges::begin(merged.values) + size, std::ranges::end(merged.values)); + // TODO: use `std::ranges::inplace_merge`. + // Currently merged into new containers, because `std::ranges::inplace_merge` on `std::views::zip` + // does not compile on libstdc++, which dispatches on `input_iterator_tag` + auto existing = std::move(dst).extract(); + auto merged = std::move(dst).extract(); // empty, with the allocators of the destination + if constexpr (requires { merged.keys.reserve(0); merged.values.reserve(0); }) { + auto const size = std::ranges::size(existing.keys) + std::ranges::size(extracted.keys); + merged.keys.reserve(size); + merged.values.reserve(size); + } + + auto key_it = std::ranges::begin(existing.keys); + auto value_it = std::ranges::begin(existing.values); + auto const key_end = std::ranges::end(existing.keys); + auto source_key_it = std::ranges::begin(extracted.keys); + auto source_value_it = std::ranges::begin(extracted.values); + auto const source_key_end = std::ranges::end(extracted.keys); + while (key_it != key_end || source_key_it != source_key_end) { + if (key_it == key_end || (source_key_it != source_key_end && comp(*source_key_it, *key_it))) { + append(merged.keys, std::move(*source_key_it++)); + append(merged.values, std::move(*source_value_it++)); + continue; + } + if constexpr (has_unique_keys>) { + // the destination keeps its own element for an equivalent key + if (source_key_it != source_key_end && !comp(*key_it, *source_key_it)) { + ++source_key_it; + ++source_value_it; + } + } + append(merged.keys, std::move(*key_it++)); + append(merged.values, std::move(*value_it++)); } dst.replace(std::move(merged.keys), std::move(merged.values)); @@ -914,6 +932,7 @@ struct transfer_from_fn std::ranges::inplace_merge(merged, std::ranges::begin(merged) + middle, comp); if constexpr (has_unique_keys>) { + auto const is_equivalent = [&comp](auto const& a, auto const& b) { return !comp(a, b) && !comp(b, a); }; auto const removed = std::ranges::unique(merged, is_equivalent); merged.erase(std::ranges::begin(removed), std::ranges::end(removed)); } diff --git a/test/container_traits.cpp b/test/container_traits.cpp index b05f3af..ef05325 100644 --- a/test/container_traits.cpp +++ b/test/container_traits.cpp @@ -865,7 +865,6 @@ TEST_CASE("container: append_range") STATIC_CHECK(!std::invocable&, std::vector>); } -// propagates on move assignment, and two objects compare equal only when they have the same tag template struct tagged_allocator { @@ -875,9 +874,11 @@ struct tagged_allocator int tag = 0; tagged_allocator() = default; + explicit tagged_allocator(int tag) noexcept : tag(tag) {} + template - tagged_allocator(tagged_allocator const& other) noexcept : tag(other.tag) {} // NOLINT(google-explicit-constructor) + tagged_allocator(tagged_allocator const& other) noexcept : tag(other.tag) {} // NOLINT(misc-explicit-constructor) T* allocate(std::size_t n) { return std::allocator{}.allocate(n); } void deallocate(T* p, std::size_t n) noexcept { std::allocator{}.deallocate(p, n); } @@ -886,7 +887,6 @@ struct tagged_allocator bool operator==(tagged_allocator const& other) const noexcept { return tag == other.tag; } }; -// orders in reverse when `is_reversed`, so that two objects of this type can order differently struct stateful_less { bool is_reversed = false; @@ -920,7 +920,6 @@ TEST_CASE("container: transfer_from") CHECK(dst == std::unordered_map{{1, 10}, {2, 30}}); } - // element-wise { std::vector dst{"a"}, src{"b"}; transfer_from(dst, src); @@ -986,7 +985,7 @@ TEST_CASE("container: transfer_from") } { // The destination keeps the memory resource of its underlying containers - using pmr_flat_map = std::flat_map, std::pmr::vector, std::pmr::vector>; + using pmr_flat_map = std::flat_map, std::pmr::vector, std::pmr::vector>; std::pmr::monotonic_buffer_resource dst_resource, src_resource; pmr_flat_map dst{std::pmr::polymorphic_allocator(&dst_resource)}, src{std::pmr::polymorphic_allocator(&src_resource)}; src.emplace(2, 20); @@ -1004,7 +1003,7 @@ TEST_CASE("container: transfer_from") } { // An empty destination takes the buffers of a source that has an equal memory resource - using pmr_flat_set = std::flat_set, std::pmr::vector>; + using pmr_flat_set = std::flat_set, std::pmr::vector>; std::pmr::monotonic_buffer_resource resource; pmr_flat_set dst{std::pmr::polymorphic_allocator(&resource)}, src{std::pmr::polymorphic_allocator(&resource)}; src.insert(1); @@ -1014,7 +1013,7 @@ TEST_CASE("container: transfer_from") } { // An allocator that propagates on move assignment would replace the allocator of an empty destination - using tagged_flat_set = std::flat_set, std::vector>>; + using tagged_flat_set = std::flat_set, std::vector>>; tagged_flat_set dst{tagged_allocator(1)}, src{tagged_allocator(2)}; src.insert(1); transfer_from(dst, src);