[libc++] Remove a bunch of now unnecessary indirections in __tree #142397
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesMost notably, this removes the notion of a distinct This is a follow-up to #134819. Patch is 26.27 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/142397.diff 3 Files Affected:
diff --git a/libcxx/include/__tree b/libcxx/include/__tree
index 403cfe1ba4036..25d098351d572 100644
--- a/libcxx/include/__tree
+++ b/libcxx/include/__tree
@@ -35,6 +35,7 @@
#include <__type_traits/is_nothrow_constructible.h>
#include <__type_traits/is_same.h>
#include <__type_traits/is_swappable.h>
+#include <__type_traits/remove_const.h>
#include <__type_traits/remove_const_ref.h>
#include <__type_traits/remove_cvref.h>
#include <__utility/forward.h>
@@ -504,29 +505,6 @@ struct __is_tree_value_type : false_type {};
template <class _One>
struct __is_tree_value_type<_One> : __is_tree_value_type_imp<__remove_cvref_t<_One> > {};
-template <class _Tp>
-struct __tree_key_value_types {
- typedef _Tp key_type;
- typedef _Tp __container_value_type;
- static const bool __is_map = false;
-
- _LIBCPP_HIDE_FROM_ABI static key_type const& __get_key(_Tp const& __v) { return __v; }
-};
-
-template <class _Key, class _Tp>
-struct __tree_key_value_types<__value_type<_Key, _Tp> > {
- typedef _Key key_type;
- typedef _Tp mapped_type;
- typedef pair<const _Key, _Tp> __container_value_type;
- typedef __container_value_type __map_value_type;
- static const bool __is_map = true;
-
- template <class _Up, __enable_if_t<__is_same_uncvref<_Up, __container_value_type>::value, int> = 0>
- _LIBCPP_HIDE_FROM_ABI static key_type const& __get_key(_Up& __t) {
- return __t.first;
- }
-};
-
template <class _VoidPtr>
struct __tree_node_base_types {
typedef _VoidPtr __void_pointer;
@@ -555,16 +533,19 @@ private:
"_VoidPtr does not point to unqualified void type");
};
-template <class _Tp, class _AllocPtr, class _KVTypes = __tree_key_value_types<_Tp>, bool = _KVTypes::__is_map>
-struct __tree_map_pointer_types {};
+template <class _Tp>
+struct __get_tree_key_type {
+ using type _LIBCPP_NODEBUG = _Tp;
+};
-template <class _Tp, class _AllocPtr, class _KVTypes>
-struct __tree_map_pointer_types<_Tp, _AllocPtr, _KVTypes, true> {
- typedef typename _KVTypes::__map_value_type _Mv;
- typedef __rebind_pointer_t<_AllocPtr, _Mv> __map_value_type_pointer;
- typedef __rebind_pointer_t<_AllocPtr, const _Mv> __const_map_value_type_pointer;
+template <class _Key, class _ValueT>
+struct __get_tree_key_type<__value_type<_Key, _ValueT> > {
+ using type _LIBCPP_NODEBUG = _Key;
};
+template <class _Tp>
+using __get_tree_key_type_t _LIBCPP_NODEBUG = typename __get_tree_key_type<_Tp>::type;
+
template <class _Tp>
struct __get_node_value_type {
using type _LIBCPP_NODEBUG = _Tp;
@@ -582,8 +563,7 @@ template <class _NodePtr, class _NodeT = typename pointer_traits<_NodePtr>::elem
struct __tree_node_types;
template <class _NodePtr, class _Tp, class _VoidPtr>
-struct __tree_node_types<_NodePtr, __tree_node<_Tp, _VoidPtr> >
- : public __tree_node_base_types<_VoidPtr>, __tree_key_value_types<_Tp>, __tree_map_pointer_types<_Tp, _VoidPtr> {
+struct __tree_node_types<_NodePtr, __tree_node<_Tp, _VoidPtr> > : public __tree_node_base_types<_VoidPtr> {
typedef __tree_node_base_types<_VoidPtr> __base;
public:
@@ -592,7 +572,6 @@ public:
using __node_value_type _LIBCPP_NODEBUG = __get_node_value_type_t<_Tp>;
typedef __rebind_pointer_t<_VoidPtr, __node_value_type> __node_value_type_pointer;
- typedef __rebind_pointer_t<_VoidPtr, const __node_value_type> __const_node_value_type_pointer;
private:
static_assert(!is_const<__node_type>::value, "_NodePtr should never be a pointer to const");
@@ -700,16 +679,15 @@ class __tree_iterator {
typedef _NodePtr __node_pointer;
typedef typename _NodeTypes::__node_base_pointer __node_base_pointer;
typedef typename _NodeTypes::__end_node_pointer __end_node_pointer;
- typedef pointer_traits<__node_pointer> __pointer_traits;
__end_node_pointer __ptr_;
public:
- typedef bidirectional_iterator_tag iterator_category;
- using value_type = __get_node_value_type_t<_Tp>;
- typedef _DiffType difference_type;
- typedef value_type& reference;
- typedef typename _NodeTypes::__node_value_type_pointer pointer;
+ using iterator_category = bidirectional_iterator_tag;
+ using value_type = __get_node_value_type_t<_Tp>;
+ using difference_type = _DiffType;
+ using reference = value_type&;
+ using pointer = __rebind_pointer_t<_NodePtr, value_type>;
_LIBCPP_HIDE_FROM_ABI __tree_iterator() _NOEXCEPT
#if _LIBCPP_STD_VER >= 14
@@ -774,16 +752,15 @@ class __tree_const_iterator {
typedef typename _NodeTypes::__node_pointer __node_pointer;
typedef typename _NodeTypes::__node_base_pointer __node_base_pointer;
typedef typename _NodeTypes::__end_node_pointer __end_node_pointer;
- typedef pointer_traits<__node_pointer> __pointer_traits;
__end_node_pointer __ptr_;
public:
- typedef bidirectional_iterator_tag iterator_category;
- using value_type = __get_node_value_type_t<_Tp>;
- typedef _DiffType difference_type;
- typedef const value_type& reference;
- typedef typename _NodeTypes::__const_node_value_type_pointer pointer;
+ using iterator_category = bidirectional_iterator_tag;
+ using value_type = __get_node_value_type_t<_Tp>;
+ using difference_type = _DiffType;
+ using reference = const value_type&;
+ using pointer = __rebind_pointer_t<_NodePtr, const value_type>;
_LIBCPP_HIDE_FROM_ABI __tree_const_iterator() _NOEXCEPT
#if _LIBCPP_STD_VER >= 14
@@ -859,18 +836,17 @@ int __diagnose_non_const_comparator();
template <class _Tp, class _Compare, class _Allocator>
class __tree {
public:
- typedef _Tp value_type;
+ using value_type = __get_node_value_type_t<_Tp>;
typedef _Compare value_compare;
typedef _Allocator allocator_type;
private:
typedef allocator_traits<allocator_type> __alloc_traits;
- typedef typename __make_tree_node_types<value_type, typename __alloc_traits::void_pointer>::type _NodeTypes;
- typedef typename _NodeTypes::key_type key_type;
+ typedef typename __make_tree_node_types<_Tp, typename __alloc_traits::void_pointer>::type _NodeTypes;
+ using key_type = __get_tree_key_type_t<_Tp>;
public:
typedef typename _NodeTypes::__node_value_type __node_value_type;
- typedef typename _NodeTypes::__container_value_type __container_value_type;
typedef typename __alloc_traits::pointer pointer;
typedef typename __alloc_traits::const_pointer const_pointer;
@@ -945,8 +921,8 @@ public:
return std::addressof(__end_node()->__left_);
}
- typedef __tree_iterator<value_type, __node_pointer, difference_type> iterator;
- typedef __tree_const_iterator<value_type, __node_pointer, difference_type> const_iterator;
+ typedef __tree_iterator<_Tp, __node_pointer, difference_type> iterator;
+ typedef __tree_const_iterator<_Tp, __node_pointer, difference_type> const_iterator;
_LIBCPP_HIDE_FROM_ABI explicit __tree(const value_compare& __comp) _NOEXCEPT_(
is_nothrow_default_constructible<__node_allocator>::value&& is_nothrow_copy_constructible<value_compare>::value);
@@ -1012,7 +988,7 @@ public:
template <class _First,
class _Second,
- __enable_if_t<__can_extract_map_key<_First, key_type, __container_value_type>::value, int> = 0>
+ __enable_if_t<__can_extract_map_key<_First, key_type, value_type>::value, int> = 0>
_LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __emplace_unique(_First&& __f, _Second&& __s) {
return __emplace_unique_key_args(__f, std::forward<_First>(__f), std::forward<_Second>(__s));
}
@@ -1044,7 +1020,7 @@ public:
template <class _First,
class _Second,
- __enable_if_t<__can_extract_map_key<_First, key_type, __container_value_type>::value, int> = 0>
+ __enable_if_t<__can_extract_map_key<_First, key_type, value_type>::value, int> = 0>
_LIBCPP_HIDE_FROM_ABI iterator __emplace_hint_unique(const_iterator __p, _First&& __f, _Second&& __s) {
return __emplace_hint_unique_key_args(__p, __f, std::forward<_First>(__f), std::forward<_Second>(__s)).first;
}
@@ -1072,28 +1048,28 @@ public:
return __emplace_hint_unique_key_args(__p, __x.first, std::forward<_Pp>(__x)).first;
}
- _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __insert_unique(const __container_value_type& __v) {
- return __emplace_unique_key_args(_NodeTypes::__get_key(__v), __v);
+ _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __insert_unique(const value_type& __v) {
+ return __emplace_unique_key_args(__v, __v);
}
- _LIBCPP_HIDE_FROM_ABI iterator __insert_unique(const_iterator __p, const __container_value_type& __v) {
- return __emplace_hint_unique_key_args(__p, _NodeTypes::__get_key(__v), __v).first;
+ _LIBCPP_HIDE_FROM_ABI iterator __insert_unique(const_iterator __p, const value_type& __v) {
+ return __emplace_hint_unique_key_args(__p, __v, __v).first;
}
- _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __insert_unique(__container_value_type&& __v) {
- return __emplace_unique_key_args(_NodeTypes::__get_key(__v), std::move(__v));
+ _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __insert_unique(value_type&& __v) {
+ return __emplace_unique_key_args(__v, std::move(__v));
}
- _LIBCPP_HIDE_FROM_ABI iterator __insert_unique(const_iterator __p, __container_value_type&& __v) {
- return __emplace_hint_unique_key_args(__p, _NodeTypes::__get_key(__v), std::move(__v)).first;
+ _LIBCPP_HIDE_FROM_ABI iterator __insert_unique(const_iterator __p, value_type&& __v) {
+ return __emplace_hint_unique_key_args(__p, __v, std::move(__v)).first;
}
- template <class _Vp, __enable_if_t<!is_same<__remove_const_ref_t<_Vp>, __container_value_type>::value, int> = 0>
+ template <class _Vp, __enable_if_t<!is_same<__remove_const_ref_t<_Vp>, value_type>::value, int> = 0>
_LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __insert_unique(_Vp&& __v) {
return __emplace_unique(std::forward<_Vp>(__v));
}
- template <class _Vp, __enable_if_t<!is_same<__remove_const_ref_t<_Vp>, __container_value_type>::value, int> = 0>
+ template <class _Vp, __enable_if_t<!is_same<__remove_const_ref_t<_Vp>, value_type>::value, int> = 0>
_LIBCPP_HIDE_FROM_ABI iterator __insert_unique(const_iterator __p, _Vp&& __v) {
return __emplace_hint_unique(__p, std::forward<_Vp>(__v));
}
@@ -1101,8 +1077,7 @@ public:
template <class _ValueT = _Tp, __enable_if_t<__is_tree_value_type<_ValueT>::value, int> = 0>
_LIBCPP_HIDE_FROM_ABI void
__insert_unique_from_orphaned_node(const_iterator __p, __get_node_value_type_t<_Tp>&& __value) {
- using __key_type = typename _NodeTypes::key_type;
- __emplace_hint_unique(__p, const_cast<__key_type&&>(__value.first), std::move(__value.second));
+ __emplace_hint_unique(__p, const_cast<key_type&&>(__value.first), std::move(__value.second));
}
template <class _ValueT = _Tp, __enable_if_t<!__is_tree_value_type<_ValueT>::value, int> = 0>
@@ -1110,11 +1085,9 @@ public:
__emplace_hint_unique(__p, std::move(__value));
}
- _LIBCPP_HIDE_FROM_ABI iterator __insert_multi(__container_value_type&& __v) {
- return __emplace_multi(std::move(__v));
- }
+ _LIBCPP_HIDE_FROM_ABI iterator __insert_multi(value_type&& __v) { return __emplace_multi(std::move(__v)); }
- _LIBCPP_HIDE_FROM_ABI iterator __insert_multi(const_iterator __p, __container_value_type&& __v) {
+ _LIBCPP_HIDE_FROM_ABI iterator __insert_multi(const_iterator __p, value_type&& __v) {
return __emplace_hint_multi(__p, std::move(__v));
}
@@ -1129,10 +1102,8 @@ public:
}
template <class _ValueT = _Tp, __enable_if_t<__is_tree_value_type<_ValueT>::value, int> = 0>
- _LIBCPP_HIDE_FROM_ABI void
- __insert_multi_from_orphaned_node(const_iterator __p, __get_node_value_type_t<_Tp>&& __value) {
- using __key_type = typename _NodeTypes::key_type;
- __emplace_hint_multi(__p, const_cast<__key_type&&>(__value.first), std::move(__value.second));
+ _LIBCPP_HIDE_FROM_ABI void __insert_multi_from_orphaned_node(const_iterator __p, value_type&& __value) {
+ __emplace_hint_multi(__p, const_cast<key_type&&>(__value.first), std::move(__value.second));
}
template <class _ValueT = _Tp, __enable_if_t<!__is_tree_value_type<_ValueT>::value, int> = 0>
@@ -1140,8 +1111,7 @@ public:
__emplace_hint_multi(__p, std::move(__value));
}
- _LIBCPP_HIDE_FROM_ABI pair<iterator, bool>
- __node_assign_unique(const __container_value_type& __v, __node_pointer __dest);
+ _LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __node_assign_unique(const value_type& __v, __node_pointer __dest);
_LIBCPP_HIDE_FROM_ABI iterator __node_insert_multi(__node_pointer __nd);
_LIBCPP_HIDE_FROM_ABI iterator __node_insert_multi(const_iterator __p, __node_pointer __nd);
@@ -1254,10 +1224,11 @@ public:
_LIBCPP_HIDE_FROM_ABI void __copy_assign_alloc(const __tree&, false_type) {}
private:
- _LIBCPP_HIDE_FROM_ABI __node_base_pointer& __find_leaf_low(__parent_pointer& __parent, const key_type& __v);
- _LIBCPP_HIDE_FROM_ABI __node_base_pointer& __find_leaf_high(__parent_pointer& __parent, const key_type& __v);
+ _LIBCPP_HIDE_FROM_ABI __node_base_pointer& __find_leaf_low(__parent_pointer& __parent, const value_type& __v);
+ _LIBCPP_HIDE_FROM_ABI __node_base_pointer& __find_leaf_high(__parent_pointer& __parent, const value_type& __v);
+
_LIBCPP_HIDE_FROM_ABI __node_base_pointer&
- __find_leaf(const_iterator __hint, __parent_pointer& __parent, const key_type& __v);
+ __find_leaf(const_iterator __hint, __parent_pointer& __parent, const value_type& __v);
template <class... _Args>
_LIBCPP_HIDE_FROM_ABI __node_holder __construct_node(_Args&&... __args);
@@ -1283,7 +1254,7 @@ private:
template <class _From, class _ValueT = _Tp, __enable_if_t<__is_tree_value_type<_ValueT>::value, int> = 0>
_LIBCPP_HIDE_FROM_ABI static void __assign_value(__get_node_value_type_t<value_type>& __lhs, _From&& __rhs) {
- using __key_type = typename _NodeTypes::key_type;
+ using __key_type = __remove_const_t<typename value_type::first_type>;
// This is technically UB, since the object was constructed as `const`.
// Clang doesn't optimize on this currently though.
@@ -1409,8 +1380,8 @@ template <class _ForwardIterator>
void __tree<_Tp, _Compare, _Allocator>::__assign_unique(_ForwardIterator __first, _ForwardIterator __last) {
typedef iterator_traits<_ForwardIterator> _ITraits;
typedef typename _ITraits::value_type _ItValueType;
- static_assert(is_same<_ItValueType, __container_value_type>::value,
- "__assign_unique may only be called with the containers value type");
+ static_assert(
+ is_same<_ItValueType, value_type>::value, "__assign_unique may only be called with the containers value type");
static_assert(
__has_forward_iterator_category<_ForwardIterator>::value, "__assign_unique requires a forward iterator");
if (size() != 0) {
@@ -1429,10 +1400,8 @@ template <class _InputIterator>
void __tree<_Tp, _Compare, _Allocator>::__assign_multi(_InputIterator __first, _InputIterator __last) {
typedef iterator_traits<_InputIterator> _ITraits;
typedef typename _ITraits::value_type _ItValueType;
- static_assert(
- (is_same<_ItValueType, __container_value_type>::value || is_same<_ItValueType, __node_value_type>::value),
- "__assign_multi may only be called with the containers value type"
- " or the nodes value type");
+ static_assert((is_same<_ItValueType, value_type>::value || is_same<_ItValueType, __node_value_type>::value),
+ "__assign_multi may only be called with the containers value type or the nodes value type");
if (size() != 0) {
_DetachedTreeCache __cache(this);
for (; __cache.__get() && __first != __last; ++__first) {
@@ -1598,7 +1567,7 @@ void __tree<_Tp, _Compare, _Allocator>::clear() _NOEXCEPT {
// Return reference to null leaf
template <class _Tp, class _Compare, class _Allocator>
typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer&
-__tree<_Tp, _Compare, _Allocator>::__find_leaf_low(__parent_pointer& __parent, const key_type& __v) {
+__tree<_Tp, _Compare, _Allocator>::__find_leaf_low(__parent_pointer& __parent, const value_type& __v) {
__node_pointer __nd = __root();
if (__nd != nullptr) {
while (true) {
@@ -1628,7 +1597,7 @@ __tree<_Tp, _Compare, _Allocator>::__find_leaf_low(__parent_pointer& __parent, c
// Return reference to null leaf
template <class _Tp, class _Compare, class _Allocator>
typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer&
-__tree<_Tp, _Compare, _Allocator>::__find_leaf_high(__parent_pointer& __parent, const key_type& __v) {
+__tree<_Tp, _Compare, _Allocator>::__find_leaf_high(__parent_pointer& __parent, const value_type& __v) {
__node_pointer __nd = __root();
if (__nd != nullptr) {
while (true) {
@@ -1660,8 +1629,8 @@ __tree<_Tp, _Compare, _Allocator>::__find_leaf_high(__parent_pointer& __parent,
// Set __parent to parent of null leaf
// Return reference to null leaf
template <class _Tp, class _Compare, class _Allocator>
-typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer&
-__tree<_Tp, _Compare, _Allocator>::__find_leaf(const_iterator __hint, __parent_pointer& __parent, const key_type& __v) {
+typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer& __tree<_Tp, _Compare, _Allocator>::__find_leaf(
+ const_iterator __hint, __parent_pointer& __parent, const value_type& __v) {
if (__hint == end() || !value_comp()(*__hint, __v)) // check before
{
// __v <= *__hint
@@ -1871,7 +1840,7 @@ typename __tree<_Tp, _Compare, _Allocator>::iterator
__tree<_Tp, _Compare, _Allocator>::__emplace_multi(_Args&&... __args) {
__node_holder __h = __construct_node(std::forward<_Args>(__args)...);
__parent_pointer __parent;
- __node_base_pointer& __child = __find_leaf_high(__parent, _NodeTypes::__get_key(__h->__value_));
+ __node_base_pointer& __child = __find_leaf_high(__parent, __h->__value_);
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
return iterator(static_cast<__node_pointer>(__h.release()));
}
@@ -1882,16 +1851,16 @@ typename __tree<_Tp, _Compare, _Allocator>::iterator
__tree<_Tp, _Compare, _Allocator>::__emplace_hint_multi(const_iterator __p, _Args&&... __args) {
__node_holder __h = __construct_node(std::forward<_Args>(__args)...);
__parent_pointer __parent;
- __node_base_pointer& __child = __find_leaf(__p, __parent, _NodeTypes::__get_key(__h->__value_));
+ __node_base_pointer& __child = __find_leaf(__p, __parent, __h->__value_);
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
return iterator(static_cast<__node_pointer>(__h.release()));
}
template <class _Tp, class _Compare, class _Allocator>
pair<typename __tree<_Tp, _Compare, _Allocator>::iterator, bool>
-__tree<_Tp, _Compare, _Allocator>::__node_assign_unique(const __container_value_type& __v, __node_pointer __nd) {
+__tree<_Tp, _Compare, _Allocator>::__node_assign_unique(const value_type& __v, __node_pointer __nd) {
__parent_pointer __parent;
- __node_base_pointer& __child = __find_equal(__parent, _NodeTypes::__get_key(__v));
+ __node_base_pointer& __child = __find_equal(__parent, __v);
__node_pointer __r = static_cast<__node_pointer>(__child);
bool __inserted = false;
if (__child == nullptr) {
@@ -1907,7 +1876,7 @@ template <class _Tp, class _Compare, class _Allocator>
typename __tree<_Tp, _Compare, _Allocator>::iterator
__tree<_Tp, _Compare, _Allocator>::__node_insert_multi(__node_pointer __nd) {
__parent_pointer __parent;
- __node_base_pointer& __child = __find_leaf_high(__parent, _NodeTypes::__get_key(__nd->__value_));
+ __node_base_pointer& __child = __find_leaf_high(__parent, __nd->__value_);
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__nd));
return iterator(__nd);
}
@@ -1916,7 +1885,7 @@ template <class _Tp, class _Compare, class _Allocator>
typename __tree<_Tp, _Compare, _Allocator>::iterator
__tree<_Tp, _Compare, _Allocator>::__node_insert_multi(const_iterator __p, __node_pointer __nd) {
__parent_pointer __parent;
- __node_base_pointer& __child = __find_leaf(__p, __parent, _NodeTypes::__get_key(__nd->__value_));
+ __node_base_pointer& __child = __find_leaf(__p, __parent, __nd->__value_);
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__nd));
return iterator(__nd);
}
@@ -1997,7 +1966,7 @@ _LIBCPP_HIDE_FROM_ABI void __tree<_Tp, _Compare, _Allocator>::__node_handle_m...
[truncated]
|
ldionne
approved these changes
Jun 10, 2025
| typedef value_type& reference; | ||
| typedef typename _NodeTypes::__node_value_type_pointer pointer; | ||
| using iterator_category = bidirectional_iterator_tag; | ||
| using value_type = __get_node_value_type_t<_Tp>; |
There was a problem hiding this comment.
Request for a follow-up patch: could we rename __get_node_value_type to __get_tree_value_type for consistency?
| @@ -945,8 +921,8 @@ public: | |||
| return std::addressof(__end_node()->__left_); | |||
| } | |||
|
|
|||
| typedef __tree_iterator<value_type, __node_pointer, difference_type> iterator; | |||
| typedef __tree_const_iterator<value_type, __node_pointer, difference_type> const_iterator; | |||
| typedef __tree_iterator<_Tp, __node_pointer, difference_type> iterator; | |||
There was a problem hiding this comment.
Since this is ABI sensitive and you're visibly tip-toeing around an ABI break here, we should have a test on that. Otherwise, a future refactoring could definitely break this without ever noticing.
For watchers, the issue is that __tree_iterator is ABI facing, and _Tp is sometimes __value_type, whereas value_type is an actual pair<K const, V>. Changing from _Tp to value_type here would change the mangling of the iterator typedef.
miguelcsx
pushed a commit
to miguelcsx/llvm-project
that referenced
this pull request
Jun 23, 2025
…vm#142397) Most notably, this removes the notion of a distinct `value_type` and `__container_value_type` from `__tree`, since these are now always the same type. There are a few places we need to keep `__value_type` around, since they are ABI visibile. In these cases `_Tp` is used directly. The second simplification here is that we use `const value_type&` instead of `const key_type&` in a few places and make use of the fact that the comparator is capable of comparing any combination of `key_type` and `value_type`. This is a follow-up to llvm#134819.
Jaddyen
pushed a commit
to Jaddyen/llvm-project
that referenced
this pull request
Jun 23, 2025
…vm#142397) Most notably, this removes the notion of a distinct `value_type` and `__container_value_type` from `__tree`, since these are now always the same type. There are a few places we need to keep `__value_type` around, since they are ABI visibile. In these cases `_Tp` is used directly. The second simplification here is that we use `const value_type&` instead of `const key_type&` in a few places and make use of the fact that the comparator is capable of comparing any combination of `key_type` and `value_type`. This is a follow-up to llvm#134819.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Most notably, this removes the notion of a distinct
value_typeand__container_value_typefrom__tree, since these are now always the same type. There are a few places we need to keep__value_typearound, since they are ABI visibile. In these cases_Tpis used directly. The second simplification here is that we useconst value_type&instead ofconst key_type&in a few places and make use of the fact that the comparator is capable of comparing any combination ofkey_typeandvalue_type.This is a follow-up to #134819.