[libcxx-commits] [libcxx] [libc++] Refactor __tree::__find_equal to not have an out parameter (PR #147345)
Nikolas Klauser via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Aug 7 00:58:11 PDT 2025
https://github.com/philnik777 updated https://github.com/llvm/llvm-project/pull/147345
>From cda1e94899b97dd86742f105813b55c1985a7f22 Mon Sep 17 00:00:00 2001
From: Nikolas Klauser <nikolasklauser at berlin.de>
Date: Mon, 7 Jul 2025 18:42:36 +0200
Subject: [PATCH] [libc++] Refactor __tree::__find_equal to not have an out
parameter
---
libcxx/include/__tree | 210 +++++++++++++++++++-----------------------
libcxx/include/map | 11 +--
2 files changed, 98 insertions(+), 123 deletions(-)
diff --git a/libcxx/include/__tree b/libcxx/include/__tree
index 9d4ba3ad0639c..7c2165044b281 100644
--- a/libcxx/include/__tree
+++ b/libcxx/include/__tree
@@ -1169,15 +1169,89 @@ public:
// FIXME: Make this function const qualified. Unfortunately doing so
// breaks existing code which uses non-const callable comparators.
+
+ // Find place to insert if __v doesn't exist
+ // Set __parent to parent of null leaf
+ // Return reference to null leaf
+ // If __v exists, set parent to node of __v and return reference to node of __v
template <class _Key>
- _LIBCPP_HIDE_FROM_ABI __node_base_pointer& __find_equal(__end_node_pointer& __parent, const _Key& __v);
+ _LIBCPP_HIDE_FROM_ABI pair<__end_node_pointer, __node_base_pointer&> __find_equal(const _Key& __v) {
+ using _PairT = pair<__end_node_pointer, __node_base_pointer&>;
+
+ __node_pointer __node = __root();
+
+ if (__node == nullptr) {
+ auto __end = __end_node();
+ return _PairT(__end, __end->__left_);
+ }
+
+ __node_base_pointer* __node_ptr = __root_ptr();
+ while (true) {
+ if (value_comp()(__v, __node->__value_)) {
+ if (__node->__left_ == nullptr)
+ return _PairT(static_cast<__end_node_pointer>(__node), __node->__left_);
+
+ __node_ptr = std::addressof(__node->__left_);
+ __node = static_cast<__node_pointer>(__node->__left_);
+ } else if (value_comp()(__node->__value_, __v)) {
+ if (__node->__right_ == nullptr)
+ return _PairT(__node, __node->__right_);
+
+ __node_ptr = std::addressof(__node->__right_);
+ __node = static_cast<__node_pointer>(__node->__right_);
+ } else {
+ return _PairT(static_cast<__end_node_pointer>(__node), *__node_ptr);
+ }
+ }
+ }
+
template <class _Key>
- _LIBCPP_HIDE_FROM_ABI __node_base_pointer& __find_equal(__end_node_pointer& __parent, const _Key& __v) const {
- return const_cast<__tree*>(this)->__find_equal(__parent, __v);
+ _LIBCPP_HIDE_FROM_ABI pair<__end_node_pointer, __node_base_pointer&> __find_equal(const _Key& __v) const {
+ return const_cast<__tree*>(this)->__find_equal(__v);
}
+
+ // Find place to insert if __v doesn't exist
+ // First check prior to __hint.
+ // Next check after __hint.
+ // Next do O(log N) search.
+ // Set __parent to parent of null leaf
+ // Return reference to null leaf
+ // If __v exists, set parent to node of __v and return reference to node of __v
template <class _Key>
- _LIBCPP_HIDE_FROM_ABI __node_base_pointer&
- __find_equal(const_iterator __hint, __end_node_pointer& __parent, __node_base_pointer& __dummy, const _Key& __v);
+ _LIBCPP_HIDE_FROM_ABI pair<__end_node_pointer, __node_base_pointer&>
+ __find_equal(const_iterator __hint, __node_base_pointer& __dummy, const _Key& __v) {
+ using _PairT = pair<__end_node_pointer, __node_base_pointer&>;
+
+ if (__hint == end() || value_comp()(__v, *__hint)) { // check before
+ // __v < *__hint
+ const_iterator __prior = __hint;
+ if (__prior == begin() || value_comp()(*--__prior, __v)) {
+ // *prev(__hint) < __v < *__hint
+ if (__hint.__ptr_->__left_ == nullptr)
+ return _PairT(__hint.__ptr_, __hint.__ptr_->__left_);
+ return _PairT(__prior.__ptr_, static_cast<__node_pointer>(__prior.__ptr_)->__right_);
+ }
+ // __v <= *prev(__hint)
+ return __find_equal(__v);
+ }
+
+ if (value_comp()(*__hint, __v)) { // check after
+ // *__hint < __v
+ const_iterator __next = std::next(__hint);
+ if (__next == end() || value_comp()(__v, *__next)) {
+ // *__hint < __v < *std::next(__hint)
+ if (__hint.__get_np()->__right_ == nullptr)
+ return _PairT(__hint.__ptr_, static_cast<__node_pointer>(__hint.__ptr_)->__right_);
+ return _PairT(__next.__ptr_, __next.__ptr_->__left_);
+ }
+ // *next(__hint) <= __v
+ return __find_equal(__v);
+ }
+
+ // else __v == *__hint
+ __dummy = static_cast<__node_base_pointer>(__hint.__ptr_);
+ return _PairT(__hint.__ptr_, __dummy);
+ }
_LIBCPP_HIDE_FROM_ABI void __copy_assign_alloc(const __tree& __t) {
__copy_assign_alloc(__t, integral_constant<bool, __node_traits::propagate_on_container_copy_assignment::value>());
@@ -1620,94 +1694,6 @@ typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer& __tree<_Tp, _Co
return __find_leaf_low(__parent, __v);
}
-// Find place to insert if __v doesn't exist
-// Set __parent to parent of null leaf
-// Return reference to null leaf
-// If __v exists, set parent to node of __v and return reference to node of __v
-template <class _Tp, class _Compare, class _Allocator>
-template <class _Key>
-typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer&
-__tree<_Tp, _Compare, _Allocator>::__find_equal(__end_node_pointer& __parent, const _Key& __v) {
- __node_pointer __nd = __root();
- __node_base_pointer* __nd_ptr = __root_ptr();
- if (__nd != nullptr) {
- while (true) {
- if (value_comp()(__v, __nd->__value_)) {
- if (__nd->__left_ != nullptr) {
- __nd_ptr = std::addressof(__nd->__left_);
- __nd = static_cast<__node_pointer>(__nd->__left_);
- } else {
- __parent = static_cast<__end_node_pointer>(__nd);
- return __parent->__left_;
- }
- } else if (value_comp()(__nd->__value_, __v)) {
- if (__nd->__right_ != nullptr) {
- __nd_ptr = std::addressof(__nd->__right_);
- __nd = static_cast<__node_pointer>(__nd->__right_);
- } else {
- __parent = static_cast<__end_node_pointer>(__nd);
- return __nd->__right_;
- }
- } else {
- __parent = static_cast<__end_node_pointer>(__nd);
- return *__nd_ptr;
- }
- }
- }
- __parent = __end_node();
- return __parent->__left_;
-}
-
-// Find place to insert if __v doesn't exist
-// First check prior to __hint.
-// Next check after __hint.
-// Next do O(log N) search.
-// Set __parent to parent of null leaf
-// Return reference to null leaf
-// If __v exists, set parent to node of __v and return reference to node of __v
-template <class _Tp, class _Compare, class _Allocator>
-template <class _Key>
-typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer& __tree<_Tp, _Compare, _Allocator>::__find_equal(
- const_iterator __hint, __end_node_pointer& __parent, __node_base_pointer& __dummy, const _Key& __v) {
- if (__hint == end() || value_comp()(__v, *__hint)) // check before
- {
- // __v < *__hint
- const_iterator __prior = __hint;
- if (__prior == begin() || value_comp()(*--__prior, __v)) {
- // *prev(__hint) < __v < *__hint
- if (__hint.__ptr_->__left_ == nullptr) {
- __parent = __hint.__ptr_;
- return __parent->__left_;
- } else {
- __parent = __prior.__ptr_;
- return static_cast<__node_base_pointer>(__prior.__ptr_)->__right_;
- }
- }
- // __v <= *prev(__hint)
- return __find_equal(__parent, __v);
- } else if (value_comp()(*__hint, __v)) // check after
- {
- // *__hint < __v
- const_iterator __next = std::next(__hint);
- if (__next == end() || value_comp()(__v, *__next)) {
- // *__hint < __v < *std::next(__hint)
- if (__hint.__get_np()->__right_ == nullptr) {
- __parent = __hint.__ptr_;
- return static_cast<__node_base_pointer>(__hint.__ptr_)->__right_;
- } else {
- __parent = __next.__ptr_;
- return __parent->__left_;
- }
- }
- // *next(__hint) <= __v
- return __find_equal(__parent, __v);
- }
- // else __v == *__hint
- __parent = __hint.__ptr_;
- __dummy = static_cast<__node_base_pointer>(__hint.__ptr_);
- return __dummy;
-}
-
template <class _Tp, class _Compare, class _Allocator>
void __tree<_Tp, _Compare, _Allocator>::__insert_node_at(
__end_node_pointer __parent, __node_base_pointer& __child, __node_base_pointer __new_node) _NOEXCEPT {
@@ -1726,10 +1712,9 @@ template <class _Tp, class _Compare, class _Allocator>
template <class _Key, class... _Args>
pair<typename __tree<_Tp, _Compare, _Allocator>::iterator, bool>
__tree<_Tp, _Compare, _Allocator>::__emplace_unique_key_args(_Key const& __k, _Args&&... __args) {
- __end_node_pointer __parent;
- __node_base_pointer& __child = __find_equal(__parent, __k);
- __node_pointer __r = static_cast<__node_pointer>(__child);
- bool __inserted = false;
+ auto [__parent, __child] = __find_equal(__k);
+ __node_pointer __r = static_cast<__node_pointer>(__child);
+ bool __inserted = false;
if (__child == nullptr) {
__node_holder __h = __construct_node(std::forward<_Args>(__args)...);
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
@@ -1744,11 +1729,10 @@ template <class _Key, class... _Args>
pair<typename __tree<_Tp, _Compare, _Allocator>::iterator, bool>
__tree<_Tp, _Compare, _Allocator>::__emplace_hint_unique_key_args(
const_iterator __p, _Key const& __k, _Args&&... __args) {
- __end_node_pointer __parent;
__node_base_pointer __dummy;
- __node_base_pointer& __child = __find_equal(__p, __parent, __dummy, __k);
- __node_pointer __r = static_cast<__node_pointer>(__child);
- bool __inserted = false;
+ auto [__parent, __child] = __find_equal(__p, __dummy, __k);
+ __node_pointer __r = static_cast<__node_pointer>(__child);
+ bool __inserted = false;
if (__child == nullptr) {
__node_holder __h = __construct_node(std::forward<_Args>(__args)...);
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
@@ -1774,8 +1758,7 @@ template <class... _Args>
pair<typename __tree<_Tp, _Compare, _Allocator>::iterator, bool>
__tree<_Tp, _Compare, _Allocator>::__emplace_unique_impl(_Args&&... __args) {
__node_holder __h = __construct_node(std::forward<_Args>(__args)...);
- __end_node_pointer __parent;
- __node_base_pointer& __child = __find_equal(__parent, __h->__value_);
+ auto [__parent, __child] = __find_equal(__h->__value_);
__node_pointer __r = static_cast<__node_pointer>(__child);
bool __inserted = false;
if (__child == nullptr) {
@@ -1791,10 +1774,9 @@ template <class... _Args>
typename __tree<_Tp, _Compare, _Allocator>::iterator
__tree<_Tp, _Compare, _Allocator>::__emplace_hint_unique_impl(const_iterator __p, _Args&&... __args) {
__node_holder __h = __construct_node(std::forward<_Args>(__args)...);
- __end_node_pointer __parent;
__node_base_pointer __dummy;
- __node_base_pointer& __child = __find_equal(__p, __parent, __dummy, __h->__value_);
- __node_pointer __r = static_cast<__node_pointer>(__child);
+ auto [__parent, __child] = __find_equal(__p, __dummy, __h->__value_);
+ __node_pointer __r = static_cast<__node_pointer>(__child);
if (__child == nullptr) {
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
__r = __h.release();
@@ -1827,8 +1809,7 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_hint_multi(const_iterator __p, _Arg
template <class _Tp, class _Compare, class _Allocator>
pair<typename __tree<_Tp, _Compare, _Allocator>::iterator, bool>
__tree<_Tp, _Compare, _Allocator>::__node_assign_unique(const value_type& __v, __node_pointer __nd) {
- __end_node_pointer __parent;
- __node_base_pointer& __child = __find_equal(__parent, __v);
+ auto [__parent, __child] = __find_equal(__v);
__node_pointer __r = static_cast<__node_pointer>(__child);
bool __inserted = false;
if (__child == nullptr) {
@@ -1879,8 +1860,7 @@ __tree<_Tp, _Compare, _Allocator>::__node_handle_insert_unique(_NodeHandle&& __n
return _InsertReturnType{end(), false, _NodeHandle()};
__node_pointer __ptr = __nh.__ptr_;
- __end_node_pointer __parent;
- __node_base_pointer& __child = __find_equal(__parent, __ptr->__value_);
+ auto [__parent, __child] = __find_equal(__ptr->__value_);
if (__child != nullptr)
return _InsertReturnType{iterator(static_cast<__node_pointer>(__child)), false, std::move(__nh)};
@@ -1897,10 +1877,9 @@ __tree<_Tp, _Compare, _Allocator>::__node_handle_insert_unique(const_iterator __
return end();
__node_pointer __ptr = __nh.__ptr_;
- __end_node_pointer __parent;
__node_base_pointer __dummy;
- __node_base_pointer& __child = __find_equal(__hint, __parent, __dummy, __ptr->__value_);
- __node_pointer __r = static_cast<__node_pointer>(__child);
+ auto [__parent, __child] = __find_equal(__hint, __dummy, __ptr->__value_);
+ __node_pointer __r = static_cast<__node_pointer>(__child);
if (__child == nullptr) {
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__ptr));
__r = __ptr;
@@ -1932,9 +1911,8 @@ _LIBCPP_HIDE_FROM_ABI void __tree<_Tp, _Compare, _Allocator>::__node_handle_merg
static_assert(is_same<typename _Tree::__node_pointer, __node_pointer>::value, "");
for (typename _Tree::iterator __i = __source.begin(); __i != __source.end();) {
- __node_pointer __src_ptr = __i.__get_np();
- __end_node_pointer __parent;
- __node_base_pointer& __child = __find_equal(__parent, __src_ptr->__value_);
+ __node_pointer __src_ptr = __i.__get_np();
+ auto [__parent, __child] = __find_equal(__src_ptr->__value_);
++__i;
if (__child != nullptr)
continue;
diff --git a/libcxx/include/map b/libcxx/include/map
index 2070c91cfc46d..676712aad9472 100644
--- a/libcxx/include/map
+++ b/libcxx/include/map
@@ -1426,9 +1426,8 @@ map<_Key, _Tp, _Compare, _Allocator>::__construct_node_with_key(const key_type&
template <class _Key, class _Tp, class _Compare, class _Allocator>
_Tp& map<_Key, _Tp, _Compare, _Allocator>::operator[](const key_type& __k) {
- __parent_pointer __parent;
- __node_base_pointer& __child = __tree_.__find_equal(__parent, __k);
- __node_pointer __r = static_cast<__node_pointer>(__child);
+ auto [__parent, __child] = __tree_.__find_equal(__k);
+ __node_pointer __r = static_cast<__node_pointer>(__child);
if (__child == nullptr) {
__node_holder __h = __construct_node_with_key(__k);
__tree_.__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
@@ -1441,8 +1440,7 @@ _Tp& map<_Key, _Tp, _Compare, _Allocator>::operator[](const key_type& __k) {
template <class _Key, class _Tp, class _Compare, class _Allocator>
_Tp& map<_Key, _Tp, _Compare, _Allocator>::at(const key_type& __k) {
- __parent_pointer __parent;
- __node_base_pointer& __child = __tree_.__find_equal(__parent, __k);
+ auto [_, __child] = __tree_.__find_equal(__k);
if (__child == nullptr)
std::__throw_out_of_range("map::at: key not found");
return static_cast<__node_pointer>(__child)->__value_.second;
@@ -1450,8 +1448,7 @@ _Tp& map<_Key, _Tp, _Compare, _Allocator>::at(const key_type& __k) {
template <class _Key, class _Tp, class _Compare, class _Allocator>
const _Tp& map<_Key, _Tp, _Compare, _Allocator>::at(const key_type& __k) const {
- __parent_pointer __parent;
- __node_base_pointer __child = __tree_.__find_equal(__parent, __k);
+ auto [_, __child] = __tree_.__find_equal(__k);
if (__child == nullptr)
std::__throw_out_of_range("map::at: key not found");
return static_cast<__node_pointer>(__child)->__value_.second;
More information about the libcxx-commits
mailing list