[libcxx-commits] [libcxx] [libc++] Refactor __tree::__find_equal to not have an out parameter (PR #147345)

[Louis Dionne via libcxx-commits]

================
@@ -1109,15 +1107,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 _Pair = pair<__end_node_pointer, __node_base_pointer&>;
+
+    __node_pointer __nd = __root();
+
+    if (__nd == nullptr) {
+      auto __end = __end_node();
+      return _Pair(__end, __end->__left_);
+    }
+
+    __node_base_pointer* __node_ptr = __root_ptr();
+    while (true) {
+      if (value_comp()(__v, __nd->__get_value())) {
+        if (__nd->__left_ == nullptr)
+          return _Pair(static_cast<__end_node_pointer>(__nd), __nd->__left_);
+
+        __node_ptr = std::addressof(__nd->__left_);
+        __nd       = static_cast<__node_pointer>(__nd->__left_);
+      } else if (value_comp()(__nd->__get_value(), __v)) {
+        if (__nd->__right_ == nullptr)
+          return _Pair(static_cast<__end_node_pointer>(__nd), __nd->__right_);
+
+        __node_ptr = std::addressof(__nd->__right_);
+        __nd       = static_cast<__node_pointer>(__nd->__right_);
+      } else {
+        return _Pair(static_cast<__end_node_pointer>(__nd), *__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) {
----------------
ldionne wrote:

Can we please avoid moving this code around in this patch? That can be done in a follow-up, but I'd like to keep the diff easy to look at and also this will help with per-commit regression testing in case this changes inlining decisions.

https://github.com/llvm/llvm-project/pull/147345