From aeb2dc9c34e2dbddb5762d2ffca356e23eb55b56 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Tue, 31 May 2022 09:13:25 -0700 Subject: Allow for using b-tree with `value_type`s that can only be constructed by the allocator (ignoring copy/move constructors). We were using `init_type`s for temp values that we would move into slots, but in this case, we need to have actual slots. We use node handles for managing slots outside of nodes. Also, in btree::copy_or_move_values_in_order, pass the slots from the iterators rather than references to values. This allows for moving from map keys instead of copying for standard layout types. In the test, fix a couple of ClangTidy warnings from missing includes and calling `new` instead of `make_unique`. PiperOrigin-RevId: 452062967 Change-Id: I870e89ae1aa5b3cfa62ae6e75b73ffc3d52e731c --- absl/container/internal/btree.h | 24 +++++++++------------- absl/container/internal/btree_container.h | 33 +++++++++++++++++++++--------- absl/container/internal/common.h | 11 +++++----- absl/container/internal/container_memory.h | 9 ++++++++ 4 files changed, 48 insertions(+), 29 deletions(-) (limited to 'absl/container/internal') diff --git a/absl/container/internal/btree.h b/absl/container/internal/btree.h index cb39b161..79b56e0d 100644 --- a/absl/container/internal/btree.h +++ b/absl/container/internal/btree.h @@ -1196,7 +1196,9 @@ class btree_iterator { } const key_type &key() const { return node_->key(position_); } - slot_type *slot() { return node_->slot(position_); } + decltype(std::declval()->slot(0)) slot() { + return node_->slot(position_); + } void assert_valid_generation() const { #ifdef ABSL_BTREE_ENABLE_GENERATIONS @@ -1225,7 +1227,6 @@ template class btree { using node_type = btree_node; using is_key_compare_to = typename Params::is_key_compare_to; - using init_type = typename Params::init_type; using field_type = typename node_type::field_type; // We use a static empty node for the root/leftmost/rightmost of empty btrees @@ -1309,14 +1310,6 @@ class btree { using slot_type = typename Params::slot_type; private: - // For use in copy_or_move_values_in_order. - const value_type &maybe_move_from_iterator(const_iterator it) { return *it; } - value_type &&maybe_move_from_iterator(iterator it) { - // This is a destructive operation on the other container so it's safe for - // us to const_cast and move from the keys here even if it's a set. - return std::move(const_cast(*it)); - } - // Copies or moves (depending on the template parameter) the values in // other into this btree in their order in other. This btree must be empty // before this method is called. This method is used in copy construction, @@ -2063,12 +2056,12 @@ void btree

::copy_or_move_values_in_order(Btree &other) { // values is the same order we'll store them in. auto iter = other.begin(); if (iter == other.end()) return; - insert_multi(maybe_move_from_iterator(iter)); + insert_multi(iter.slot()); ++iter; for (; iter != other.end(); ++iter) { // If the btree is not empty, we can just insert the new value at the end // of the tree. - internal_emplace(end(), maybe_move_from_iterator(iter)); + internal_emplace(end(), iter.slot()); } } @@ -2205,8 +2198,11 @@ template template void btree

::insert_iterator_unique(InputIterator b, InputIterator e, char) { for (; b != e; ++b) { - init_type value(*b); - insert_hint_unique(end(), params_type::key(value), std::move(value)); + // Use a node handle to manage a temp slot. + auto node_handle = + CommonAccess::Construct(get_allocator(), *b); + slot_type *slot = CommonAccess::GetSlot(node_handle); + insert_hint_unique(end(), params_type::key(slot), slot); } } diff --git a/absl/container/internal/btree_container.h b/absl/container/internal/btree_container.h index cc2e1793..fc2f740a 100644 --- a/absl/container/internal/btree_container.h +++ b/absl/container/internal/btree_container.h @@ -166,9 +166,10 @@ class btree_container { // Extract routines. node_type extract(iterator position) { - // Use Move instead of Transfer, because the rebalancing code expects to - // have a valid object to scribble metadata bits on top of. - auto node = CommonAccess::Move(get_allocator(), position.slot()); + // Use Construct instead of Transfer because the rebalancing code will + // destroy the slot later. + auto node = + CommonAccess::Construct(get_allocator(), position.slot()); erase(position); return node; } @@ -291,8 +292,11 @@ class btree_set_container : public btree_container { } template std::pair emplace(Args &&... args) { - init_type v(std::forward(args)...); - return this->tree_.insert_unique(params_type::key(v), std::move(v)); + // Use a node handle to manage a temp slot. + auto node = CommonAccess::Construct(this->get_allocator(), + std::forward(args)...); + auto *slot = CommonAccess::GetSlot(node); + return this->tree_.insert_unique(params_type::key(slot), slot); } iterator insert(const_iterator hint, const value_type &v) { return this->tree_ @@ -306,9 +310,12 @@ class btree_set_container : public btree_container { } template iterator emplace_hint(const_iterator hint, Args &&... args) { - init_type v(std::forward(args)...); + // Use a node handle to manage a temp slot. + auto node = CommonAccess::Construct(this->get_allocator(), + std::forward(args)...); + auto *slot = CommonAccess::GetSlot(node); return this->tree_ - .insert_hint_unique(iterator(hint), params_type::key(v), std::move(v)) + .insert_hint_unique(iterator(hint), params_type::key(slot), slot) .first; } template @@ -598,12 +605,18 @@ class btree_multiset_container : public btree_container { } template iterator emplace(Args &&... args) { - return this->tree_.insert_multi(init_type(std::forward(args)...)); + // Use a node handle to manage a temp slot. + auto node = CommonAccess::Construct(this->get_allocator(), + std::forward(args)...); + return this->tree_.insert_multi(CommonAccess::GetSlot(node)); } template iterator emplace_hint(const_iterator hint, Args &&... args) { - return this->tree_.insert_hint_multi( - iterator(hint), init_type(std::forward(args)...)); + // Use a node handle to manage a temp slot. + auto node = CommonAccess::Construct(this->get_allocator(), + std::forward(args)...); + return this->tree_.insert_hint_multi(iterator(hint), + CommonAccess::GetSlot(node)); } iterator insert(node_type &&node) { if (!node) return this->end(); diff --git a/absl/container/internal/common.h b/absl/container/internal/common.h index 030e9d4a..416d9aa3 100644 --- a/absl/container/internal/common.h +++ b/absl/container/internal/common.h @@ -84,10 +84,11 @@ class node_handle_base { PolicyTraits::transfer(alloc(), slot(), s); } - struct move_tag_t {}; - node_handle_base(move_tag_t, const allocator_type& a, slot_type* s) + struct construct_tag_t {}; + template + node_handle_base(construct_tag_t, const allocator_type& a, Args&&... args) : alloc_(a) { - PolicyTraits::construct(alloc(), slot(), s); + PolicyTraits::construct(alloc(), slot(), std::forward(args)...); } void destroy() { @@ -186,8 +187,8 @@ struct CommonAccess { } template - static T Move(Args&&... args) { - return T(typename T::move_tag_t{}, std::forward(args)...); + static T Construct(Args&&... args) { + return T(typename T::construct_tag_t{}, std::forward(args)...); } }; diff --git a/absl/container/internal/container_memory.h b/absl/container/internal/container_memory.h index e67529ec..df492236 100644 --- a/absl/container/internal/container_memory.h +++ b/absl/container/internal/container_memory.h @@ -402,6 +402,15 @@ struct map_slot_policy { } } + // Construct this slot by copying from another slot. + template + static void construct(Allocator* alloc, slot_type* slot, + const slot_type* other) { + emplace(slot); + absl::allocator_traits::construct(*alloc, &slot->value, + other->value); + } + template static void destroy(Allocator* alloc, slot_type* slot) { if (kMutableKeys::value) { -- cgit v1.2.3