Skip to content

Commit 4a2dd31

Browse files
philnik777philnik777
authored
[libc++] Refactor __tree::__find_equal to not have an out parameter (#147345)
1 parent 7624c61 commit 4a2dd31

File tree

2 files changed

+86
-104
lines changed

2 files changed

+86
-104
lines changed

‎libcxx/include/__tree‎

Lines changed: 82 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -910,10 +910,9 @@ public:
910910
_LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __emplace_unique(_Args&&... __args) {
911911
return std::__try_key_extraction<key_type>(
912912
[this](const key_type& __key, _Args&&... __args2) {
913-
__end_node_pointer __parent;
914-
__node_base_pointer& __child = __find_equal(__parent, __key);
915-
__node_pointer __r = static_cast<__node_pointer>(__child);
916-
bool __inserted = false;
913+
auto [__parent, __child] = __find_equal(__key);
914+
__node_pointer __r = static_cast<__node_pointer>(__child);
915+
bool __inserted = false;
917916
if (__child == nullptr) {
918917
__node_holder __h = __construct_node(std::forward<_Args>(__args2)...);
919918
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
@@ -923,11 +922,10 @@ public:
923922
return pair<iterator, bool>(iterator(__r), __inserted);
924923
},
925924
[this](_Args&&... __args2) {
926-
__node_holder __h = __construct_node(std::forward<_Args>(__args2)...);
927-
__end_node_pointer __parent;
928-
__node_base_pointer& __child = __find_equal(__parent, __h->__get_value());
929-
__node_pointer __r = static_cast<__node_pointer>(__child);
930-
bool __inserted = false;
925+
__node_holder __h = __construct_node(std::forward<_Args>(__args2)...);
926+
auto [__parent, __child] = __find_equal(__h->__get_value());
927+
__node_pointer __r = static_cast<__node_pointer>(__child);
928+
bool __inserted = false;
931929
if (__child == nullptr) {
932930
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
933931
__r = __h.release();
@@ -942,11 +940,10 @@ public:
942940
_LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __emplace_hint_unique(const_iterator __p, _Args&&... __args) {
943941
return std::__try_key_extraction<key_type>(
944942
[this, __p](const key_type& __key, _Args&&... __args2) {
945-
__end_node_pointer __parent;
946943
__node_base_pointer __dummy;
947-
__node_base_pointer& __child = __find_equal(__p, __parent, __dummy, __key);
948-
__node_pointer __r = static_cast<__node_pointer>(__child);
949-
bool __inserted = false;
944+
auto [__parent, __child] = __find_equal(__p, __dummy, __key);
945+
__node_pointer __r = static_cast<__node_pointer>(__child);
946+
bool __inserted = false;
950947
if (__child == nullptr) {
951948
__node_holder __h = __construct_node(std::forward<_Args>(__args2)...);
952949
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
@@ -957,10 +954,9 @@ public:
957954
},
958955
[this, __p](_Args&&... __args2) {
959956
__node_holder __h = __construct_node(std::forward<_Args>(__args2)...);
960-
__end_node_pointer __parent;
961957
__node_base_pointer __dummy;
962-
__node_base_pointer& __child = __find_equal(__p, __parent, __dummy, __h->__get_value());
963-
__node_pointer __r = static_cast<__node_pointer>(__child);
958+
auto [__parent, __child] = __find_equal(__p, __dummy, __h->__get_value());
959+
__node_pointer __r = static_cast<__node_pointer>(__child);
964960
if (__child == nullptr) {
965961
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
966962
__r = __h.release();
@@ -1112,17 +1108,15 @@ public:
11121108

11131109
template <class _Key>
11141110
_LIBCPP_HIDE_FROM_ABI iterator find(const _Key& __key) {
1115-
__end_node_pointer __parent;
1116-
__node_base_pointer __match = __find_equal(__parent, __key);
1111+
auto [__, __match] = __find_equal(__key);
11171112
if (__match == nullptr)
11181113
return end();
11191114
return iterator(static_cast<__node_pointer>(__match));
11201115
}
11211116

11221117
template <class _Key>
11231118
_LIBCPP_HIDE_FROM_ABI const_iterator find(const _Key& __key) const {
1124-
__end_node_pointer __parent;
1125-
__node_base_pointer __match = __find_equal(__parent, __key);
1119+
auto [__, __match] = __find_equal(__key);
11261120
if (__match == nullptr)
11271121
return end();
11281122
return const_iterator(static_cast<__node_pointer>(__match));
@@ -1177,14 +1171,16 @@ public:
11771171
// FIXME: Make this function const qualified. Unfortunately doing so
11781172
// breaks existing code which uses non-const callable comparators.
11791173
template <class _Key>
1180-
_LIBCPP_HIDE_FROM_ABI __node_base_pointer& __find_equal(__end_node_pointer& __parent, const _Key& __v);
1174+
_LIBCPP_HIDE_FROM_ABI pair<__end_node_pointer, __node_base_pointer&> __find_equal(const _Key& __v);
1175+
11811176
template <class _Key>
1182-
_LIBCPP_HIDE_FROM_ABI __node_base_pointer& __find_equal(__end_node_pointer& __parent, const _Key& __v) const {
1183-
return const_cast<__tree*>(this)->__find_equal(__parent, __v);
1177+
_LIBCPP_HIDE_FROM_ABI pair<__end_node_pointer, __node_base_pointer&> __find_equal(const _Key& __v) const {
1178+
return const_cast<__tree*>(this)->__find_equal(__v);
11841179
}
1180+
11851181
template <class _Key>
1186-
_LIBCPP_HIDE_FROM_ABI __node_base_pointer&
1187-
__find_equal(const_iterator __hint, __end_node_pointer& __parent, __node_base_pointer& __dummy, const _Key& __v);
1182+
_LIBCPP_HIDE_FROM_ABI pair<__end_node_pointer, __node_base_pointer&>
1183+
__find_equal(const_iterator __hint, __node_base_pointer& __dummy, const _Key& __v);
11881184

11891185
_LIBCPP_HIDE_FROM_ABI void __copy_assign_alloc(const __tree& __t) {
11901186
__copy_assign_alloc(__t, integral_constant<bool, __node_traits::propagate_on_container_copy_assignment::value>());
@@ -1737,92 +1733,85 @@ typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer& __tree<_Tp, _Co
17371733
return __find_leaf_low(__parent, __v);
17381734
}
17391735

1740-
// Find place to insert if __v doesn't exist
1741-
// Set __parent to parent of null leaf
1742-
// Return reference to null leaf
1743-
// If __v exists, set parent to node of __v and return reference to node of __v
1736+
// Find __v
1737+
// If __v exists, return the parent of the node of __v and a reference to the pointer to the node of __v.
1738+
// If __v doesn't exist, return the parent of the null leaf and a reference to the pointer to the null leaf.
17441739
template <class _Tp, class _Compare, class _Allocator>
17451740
template <class _Key>
1746-
typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer&
1747-
__tree<_Tp, _Compare, _Allocator>::__find_equal(__end_node_pointer& __parent, const _Key& __v) {
1748-
__node_pointer __nd = __root();
1749-
__node_base_pointer* __nd_ptr = __root_ptr();
1750-
if (__nd != nullptr) {
1751-
while (true) {
1752-
if (value_comp()(__v, __nd->__get_value())) {
1753-
if (__nd->__left_ != nullptr) {
1754-
__nd_ptr = std::addressof(__nd->__left_);
1755-
__nd = static_cast<__node_pointer>(__nd->__left_);
1756-
} else {
1757-
__parent = static_cast<__end_node_pointer>(__nd);
1758-
return __parent->__left_;
1759-
}
1760-
} else if (value_comp()(__nd->__get_value(), __v)) {
1761-
if (__nd->__right_ != nullptr) {
1762-
__nd_ptr = std::addressof(__nd->__right_);
1763-
__nd = static_cast<__node_pointer>(__nd->__right_);
1764-
} else {
1765-
__parent = static_cast<__end_node_pointer>(__nd);
1766-
return __nd->__right_;
1767-
}
1768-
} else {
1769-
__parent = static_cast<__end_node_pointer>(__nd);
1770-
return *__nd_ptr;
1771-
}
1741+
_LIBCPP_HIDE_FROM_ABI pair<typename __tree<_Tp, _Compare, _Allocator>::__end_node_pointer,
1742+
typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer&>
1743+
__tree<_Tp, _Compare, _Allocator>::__find_equal(const _Key& __v) {
1744+
using _Pair = pair<__end_node_pointer, __node_base_pointer&>;
1745+
1746+
__node_pointer __nd = __root();
1747+
1748+
if (__nd == nullptr) {
1749+
auto __end = __end_node();
1750+
return _Pair(__end, __end->__left_);
1751+
}
1752+
1753+
__node_base_pointer* __node_ptr = __root_ptr();
1754+
while (true) {
1755+
if (value_comp()(__v, __nd->__get_value())) {
1756+
if (__nd->__left_ == nullptr)
1757+
return _Pair(static_cast<__end_node_pointer>(__nd), __nd->__left_);
1758+
1759+
__node_ptr = std::addressof(__nd->__left_);
1760+
__nd = static_cast<__node_pointer>(__nd->__left_);
1761+
} else if (value_comp()(__nd->__get_value(), __v)) {
1762+
if (__nd->__right_ == nullptr)
1763+
return _Pair(static_cast<__end_node_pointer>(__nd), __nd->__right_);
1764+
1765+
__node_ptr = std::addressof(__nd->__right_);
1766+
__nd = static_cast<__node_pointer>(__nd->__right_);
1767+
} else {
1768+
return _Pair(static_cast<__end_node_pointer>(__nd), *__node_ptr);
17721769
}
17731770
}
1774-
__parent = __end_node();
1775-
return __parent->__left_;
17761771
}
17771772

1778-
// Find place to insert if __v doesn't exist
1773+
// Find __v
17791774
// First check prior to __hint.
17801775
// Next check after __hint.
17811776
// Next do O(log N) search.
1782-
// Set __parent to parent of null leaf
1783-
// Return reference to null leaf
1784-
// If __v exists, set parent to node of __v and return reference to node of __v
1777+
// If __v exists, return the parent of the node of __v and a reference to the pointer to the node of __v.
1778+
// If __v doesn't exist, return the parent of the null leaf and a reference to the pointer to the null leaf.
17851779
template <class _Tp, class _Compare, class _Allocator>
17861780
template <class _Key>
1787-
typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer& __tree<_Tp, _Compare, _Allocator>::__find_equal(
1788-
const_iterator __hint, __end_node_pointer& __parent, __node_base_pointer& __dummy, const _Key& __v) {
1789-
if (__hint == end() || value_comp()(__v, *__hint)) // check before
1790-
{
1781+
_LIBCPP_HIDE_FROM_ABI pair<typename __tree<_Tp, _Compare, _Allocator>::__end_node_pointer,
1782+
typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer&>
1783+
__tree<_Tp, _Compare, _Allocator>::__find_equal(const_iterator __hint, __node_base_pointer& __dummy, const _Key& __v) {
1784+
using _Pair = pair<__end_node_pointer, __node_base_pointer&>;
1785+
1786+
if (__hint == end() || value_comp()(__v, *__hint)) { // check before
17911787
// __v < *__hint
17921788
const_iterator __prior = __hint;
17931789
if (__prior == begin() || value_comp()(*--__prior, __v)) {
17941790
// *prev(__hint) < __v < *__hint
1795-
if (__hint.__ptr_->__left_ == nullptr) {
1796-
__parent = __hint.__ptr_;
1797-
return __parent->__left_;
1798-
} else {
1799-
__parent = __prior.__ptr_;
1800-
return static_cast<__node_base_pointer>(__prior.__ptr_)->__right_;
1801-
}
1791+
if (__hint.__ptr_->__left_ == nullptr)
1792+
return _Pair(__hint.__ptr_, __hint.__ptr_->__left_);
1793+
return _Pair(__prior.__ptr_, static_cast<__node_pointer>(__prior.__ptr_)->__right_);
18021794
}
18031795
// __v <= *prev(__hint)
1804-
return __find_equal(__parent, __v);
1805-
} else if (value_comp()(*__hint, __v)) // check after
1806-
{
1796+
return __find_equal(__v);
1797+
}
1798+
1799+
if (value_comp()(*__hint, __v)) { // check after
18071800
// *__hint < __v
18081801
const_iterator __next = std::next(__hint);
18091802
if (__next == end() || value_comp()(__v, *__next)) {
18101803
// *__hint < __v < *std::next(__hint)
1811-
if (__hint.__get_np()->__right_ == nullptr) {
1812-
__parent = __hint.__ptr_;
1813-
return static_cast<__node_base_pointer>(__hint.__ptr_)->__right_;
1814-
} else {
1815-
__parent = __next.__ptr_;
1816-
return __parent->__left_;
1817-
}
1804+
if (__hint.__get_np()->__right_ == nullptr)
1805+
return _Pair(__hint.__ptr_, static_cast<__node_pointer>(__hint.__ptr_)->__right_);
1806+
return _Pair(__next.__ptr_, __next.__ptr_->__left_);
18181807
}
18191808
// *next(__hint) <= __v
1820-
return __find_equal(__parent, __v);
1809+
return __find_equal(__v);
18211810
}
1811+
18221812
// else __v == *__hint
1823-
__parent = __hint.__ptr_;
1824-
__dummy = static_cast<__node_base_pointer>(__hint.__ptr_);
1825-
return __dummy;
1813+
__dummy = static_cast<__node_base_pointer>(__hint.__ptr_);
1814+
return _Pair(__hint.__ptr_, __dummy);
18261815
}
18271816

18281817
template <class _Tp, class _Compare, class _Allocator>
@@ -1875,10 +1864,9 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_hint_multi(const_iterator __p, _Arg
18751864
template <class _Tp, class _Compare, class _Allocator>
18761865
pair<typename __tree<_Tp, _Compare, _Allocator>::iterator, bool>
18771866
__tree<_Tp, _Compare, _Allocator>::__node_assign_unique(const value_type& __v, __node_pointer __nd) {
1878-
__end_node_pointer __parent;
1879-
__node_base_pointer& __child = __find_equal(__parent, __v);
1880-
__node_pointer __r = static_cast<__node_pointer>(__child);
1881-
bool __inserted = false;
1867+
auto [__parent, __child] = __find_equal(__v);
1868+
__node_pointer __r = static_cast<__node_pointer>(__child);
1869+
bool __inserted = false;
18821870
if (__child == nullptr) {
18831871
__assign_value(__nd->__get_value(), __v);
18841872
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__nd));
@@ -1927,8 +1915,7 @@ __tree<_Tp, _Compare, _Allocator>::__node_handle_insert_unique(_NodeHandle&& __n
19271915
return _InsertReturnType{end(), false, _NodeHandle()};
19281916

19291917
__node_pointer __ptr = __nh.__ptr_;
1930-
__end_node_pointer __parent;
1931-
__node_base_pointer& __child = __find_equal(__parent, __ptr->__get_value());
1918+
auto [__parent, __child] = __find_equal(__ptr->__get_value());
19321919
if (__child != nullptr)
19331920
return _InsertReturnType{iterator(static_cast<__node_pointer>(__child)), false, std::move(__nh)};
19341921

@@ -1945,10 +1932,9 @@ __tree<_Tp, _Compare, _Allocator>::__node_handle_insert_unique(const_iterator __
19451932
return end();
19461933

19471934
__node_pointer __ptr = __nh.__ptr_;
1948-
__end_node_pointer __parent;
19491935
__node_base_pointer __dummy;
1950-
__node_base_pointer& __child = __find_equal(__hint, __parent, __dummy, __ptr->__get_value());
1951-
__node_pointer __r = static_cast<__node_pointer>(__child);
1936+
auto [__parent, __child] = __find_equal(__hint, __dummy, __ptr->__get_value());
1937+
__node_pointer __r = static_cast<__node_pointer>(__child);
19521938
if (__child == nullptr) {
19531939
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__ptr));
19541940
__r = __ptr;
@@ -1981,8 +1967,7 @@ _LIBCPP_HIDE_FROM_ABI void __tree<_Tp, _Compare, _Allocator>::__node_handle_merg
19811967

19821968
for (typename _Tree::iterator __i = __source.begin(); __i != __source.end();) {
19831969
__node_pointer __src_ptr = __i.__get_np();
1984-
__end_node_pointer __parent;
1985-
__node_base_pointer& __child = __find_equal(__parent, __src_ptr->__get_value());
1970+
auto [__parent, __child] = __find_equal(__src_ptr->__get_value());
19861971
++__i;
19871972
if (__child != nullptr)
19881973
continue;

‎libcxx/include/map‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,9 +1412,8 @@ map<_Key, _Tp, _Compare, _Allocator>::__construct_node_with_key(const key_type&
14121412

14131413
template <class _Key, class _Tp, class _Compare, class _Allocator>
14141414
_Tp& map<_Key, _Tp, _Compare, _Allocator>::operator[](const key_type& __k) {
1415-
__parent_pointer __parent;
1416-
__node_base_pointer& __child = __tree_.__find_equal(__parent, __k);
1417-
__node_pointer __r = static_cast<__node_pointer>(__child);
1415+
auto [__parent, __child] = __tree_.__find_equal(__k);
1416+
__node_pointer __r = static_cast<__node_pointer>(__child);
14181417
if (__child == nullptr) {
14191418
__node_holder __h = __construct_node_with_key(__k);
14201419
__tree_.__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
@@ -1427,17 +1426,15 @@ _Tp& map<_Key, _Tp, _Compare, _Allocator>::operator[](const key_type& __k) {
14271426

14281427
template <class _Key, class _Tp, class _Compare, class _Allocator>
14291428
_Tp& map<_Key, _Tp, _Compare, _Allocator>::at(const key_type& __k) {
1430-
__parent_pointer __parent;
1431-
__node_base_pointer& __child = __tree_.__find_equal(__parent, __k);
1429+
auto [_, __child] = __tree_.__find_equal(__k);
14321430
if (__child == nullptr)
14331431
std::__throw_out_of_range("map::at: key not found");
14341432
return static_cast<__node_pointer>(__child)->__get_value().second;
14351433
}
14361434

14371435
template <class _Key, class _Tp, class _Compare, class _Allocator>
14381436
const _Tp& map<_Key, _Tp, _Compare, _Allocator>::at(const key_type& __k) const {
1439-
__parent_pointer __parent;
1440-
__node_base_pointer __child = __tree_.__find_equal(__parent, __k);
1437+
auto [_, __child] = __tree_.__find_equal(__k);
14411438
if (__child == nullptr)
14421439
std::__throw_out_of_range("map::at: key not found");
14431440
return static_cast<__node_pointer>(__child)->__get_value().second;

0 commit comments

Comments
 (0)