[libc++] Remove a bunch of unnecessary type indirections from __tree #145295

philnik777 · 2025-06-23T09:04:12Z

Most of the diff is replacing __parent_pointer with __end_node_pointer. The most interesting diff is that the pointer aliases are now defined directly inside __tree instead of a separate traits class.

llvmbot · 2025-06-25T15:41:51Z

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

Most of the diff is replacing __parent_pointer with __end_node_pointer. The most interesting diff is that the pointer aliases are now defined directly inside __tree instead of a separate traits class.

Patch is 29.56 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/145295.diff

1 Files Affected:

(modified) libcxx/include/__tree (+87-120)

diff --git a/libcxx/include/__tree b/libcxx/include/__tree
index 25d098351d572..567d6ccf216e8 100644
--- a/libcxx/include/__tree
+++ b/libcxx/include/__tree
@@ -505,34 +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 _VoidPtr>
-struct __tree_node_base_types {
-  typedef _VoidPtr __void_pointer;
-
-  typedef __tree_node_base<__void_pointer> __node_base_type;
-  typedef __rebind_pointer_t<_VoidPtr, __node_base_type> __node_base_pointer;
-
-  typedef __tree_end_node<__node_base_pointer> __end_node_type;
-  typedef __rebind_pointer_t<_VoidPtr, __end_node_type> __end_node_pointer;
-  typedef __end_node_pointer __parent_pointer;
-
-// TODO(LLVM 22): Remove this check
-#ifndef _LIBCPP_ABI_TREE_REMOVE_NODE_POINTER_UB
-  static_assert(sizeof(__node_base_pointer) == sizeof(__end_node_pointer) && _LIBCPP_ALIGNOF(__node_base_pointer) ==
-                    _LIBCPP_ALIGNOF(__end_node_pointer),
-                "It looks like you are using std::__tree (an implementation detail for (multi)map/set) with a fancy "
-                "pointer type that thas a different representation depending on whether it points to a __tree base "
-                "pointer or a __tree node pointer (both of which are implementation details of the standard library). "
-                "This means that your ABI is being broken between LLVM 19 and LLVM 20. If you don't care about your "
-                "ABI being broken, define the _LIBCPP_ABI_TREE_REMOVE_NODE_POINTER_UB macro to silence this "
-                "diagnostic.");
-#endif
-
-private:
-  static_assert(is_same<typename pointer_traits<_VoidPtr>::element_type, void>::value,
-                "_VoidPtr does not point to unqualified void type");
-};
-
 template <class _Tp>
 struct __get_tree_key_type {
   using type _LIBCPP_NODEBUG = _Tp;
@@ -563,26 +535,13 @@ 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> {
-  typedef __tree_node_base_types<_VoidPtr> __base;
-
-public:
-  typedef typename pointer_traits<_NodePtr>::element_type __node_type;
-  typedef _NodePtr __node_pointer;
-
-  using __node_value_type _LIBCPP_NODEBUG = __get_node_value_type_t<_Tp>;
-  typedef __rebind_pointer_t<_VoidPtr, __node_value_type> __node_value_type_pointer;
+struct __tree_node_types<_NodePtr, __tree_node<_Tp, _VoidPtr> > {
+  using __node_base_pointer _LIBCPP_NODEBUG = __rebind_pointer_t<_VoidPtr, __tree_node_base<_VoidPtr> >;
+  using __end_node_pointer _LIBCPP_NODEBUG  = __rebind_pointer_t<_VoidPtr, __tree_end_node<__node_base_pointer> >;
 
 private:
-  static_assert(!is_const<__node_type>::value, "_NodePtr should never be a pointer to const");
-  static_assert(is_same<__rebind_pointer_t<_VoidPtr, __node_type>, _NodePtr>::value,
-                "_VoidPtr does not rebind to _NodePtr.");
-};
-
-template <class _ValueTp, class _VoidPtr>
-struct __make_tree_node_types {
-  typedef __rebind_pointer_t<_VoidPtr, __tree_node<_ValueTp, _VoidPtr> > _NodePtr;
-  typedef __tree_node_types<_NodePtr> type;
+  static_assert(is_same<typename pointer_traits<_VoidPtr>::element_type, void>::value,
+                "_VoidPtr does not point to unqualified void type");
 };
 
 // node
@@ -597,20 +556,19 @@ public:
 };
 
 template <class _VoidPtr>
-class _LIBCPP_STANDALONE_DEBUG __tree_node_base : public __tree_node_base_types<_VoidPtr>::__end_node_type {
-  typedef __tree_node_base_types<_VoidPtr> _NodeBaseTypes;
-
+class _LIBCPP_STANDALONE_DEBUG
+__tree_node_base : public __tree_end_node<__rebind_pointer_t<_VoidPtr, __tree_node_base<_VoidPtr> > > {
 public:
-  typedef typename _NodeBaseTypes::__node_base_pointer pointer;
-  typedef typename _NodeBaseTypes::__parent_pointer __parent_pointer;
+  using pointer                            = __rebind_pointer_t<_VoidPtr, __tree_node_base>;
+  using __end_node_pointer _LIBCPP_NODEBUG = __rebind_pointer_t<_VoidPtr, __tree_end_node<pointer> >;
 
   pointer __right_;
-  __parent_pointer __parent_;
+  __end_node_pointer __parent_;
   bool __is_black_;
 
   _LIBCPP_HIDE_FROM_ABI pointer __parent_unsafe() const { return static_cast<pointer>(__parent_); }
 
-  _LIBCPP_HIDE_FROM_ABI void __set_parent(pointer __p) { __parent_ = static_cast<__parent_pointer>(__p); }
+  _LIBCPP_HIDE_FROM_ABI void __set_parent(pointer __p) { __parent_ = static_cast<__end_node_pointer>(__p); }
 
   ~__tree_node_base()                                  = delete;
   __tree_node_base(__tree_node_base const&)            = delete;
@@ -640,7 +598,6 @@ public:
   typedef typename __alloc_traits::pointer pointer;
 
 private:
-  typedef __tree_node_types<pointer> _NodeTypes;
   allocator_type& __na_;
 
 public:
@@ -749,7 +706,7 @@ private:
 template <class _Tp, class _NodePtr, class _DiffType>
 class __tree_const_iterator {
   typedef __tree_node_types<_NodePtr> _NodeTypes;
-  typedef typename _NodeTypes::__node_pointer __node_pointer;
+  using __node_pointer _LIBCPP_NODEBUG = _NodePtr;
   typedef typename _NodeTypes::__node_base_pointer __node_base_pointer;
   typedef typename _NodeTypes::__end_node_pointer __end_node_pointer;
 
@@ -842,35 +799,44 @@ public:
 
 private:
   typedef allocator_traits<allocator_type> __alloc_traits;
-  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 __alloc_traits::pointer pointer;
   typedef typename __alloc_traits::const_pointer const_pointer;
   typedef typename __alloc_traits::size_type size_type;
   typedef typename __alloc_traits::difference_type difference_type;
 
 public:
-  typedef typename _NodeTypes::__void_pointer __void_pointer;
+  using __void_pointer _LIBCPP_NODEBUG = typename __alloc_traits::void_pointer;
 
-  typedef typename _NodeTypes::__node_type __node;
-  typedef typename _NodeTypes::__node_pointer __node_pointer;
+  using __node _LIBCPP_NODEBUG = __tree_node<_Tp, __void_pointer>;
+  // NOLINTNEXTLINE(libcpp-nodebug-on-aliases) lldb relies on this alias for pretty printing
+  using __node_pointer = __rebind_pointer_t<__void_pointer, __node>;
 
-  typedef typename _NodeTypes::__node_base_type __node_base;
-  typedef typename _NodeTypes::__node_base_pointer __node_base_pointer;
+  using __node_base _LIBCPP_NODEBUG         = __tree_node_base<__void_pointer>;
+  using __node_base_pointer _LIBCPP_NODEBUG = __rebind_pointer_t<__void_pointer, __node_base>;
 
-  typedef typename _NodeTypes::__end_node_type __end_node_t;
-  typedef typename _NodeTypes::__end_node_pointer __end_node_ptr;
-  using __end_node_pointer _LIBCPP_NODEBUG = typename _NodeTypes::__end_node_pointer;
+  using __end_node_t _LIBCPP_NODEBUG       = __tree_end_node<__node_base_pointer>;
+  using __end_node_pointer _LIBCPP_NODEBUG = __rebind_pointer_t<__void_pointer, __end_node_t>;
 
-  typedef typename _NodeTypes::__parent_pointer __parent_pointer;
+  using __parent_pointer _LIBCPP_NODEBUG = __end_node_pointer; // TODO: Remove this once the uses in <map> are removed
 
   typedef __rebind_alloc<__alloc_traits, __node> __node_allocator;
   typedef allocator_traits<__node_allocator> __node_traits;
 
+// TODO(LLVM 22): Remove this check
+#ifndef _LIBCPP_ABI_TREE_REMOVE_NODE_POINTER_UB
+  static_assert(sizeof(__node_base_pointer) == sizeof(__end_node_pointer) && _LIBCPP_ALIGNOF(__node_base_pointer) ==
+                    _LIBCPP_ALIGNOF(__end_node_pointer),
+                "It looks like you are using std::__tree (an implementation detail for (multi)map/set) with a fancy "
+                "pointer type that thas a different representation depending on whether it points to a __tree base "
+                "pointer or a __tree node pointer (both of which are implementation details of the standard library). "
+                "This means that your ABI is being broken between LLVM 19 and LLVM 20. If you don't care about your "
+                "ABI being broken, define the _LIBCPP_ABI_TREE_REMOVE_NODE_POINTER_UB macro to silence this "
+                "diagnostic.");
+#endif
+
 private:
   // check for sane allocator pointer rebinding semantics. Rebinding the
   // allocator for a new pointer type should be exactly the same as rebinding
@@ -1147,7 +1113,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI size_type __erase_multi(const _Key& __k);
 
   _LIBCPP_HIDE_FROM_ABI void
-  __insert_node_at(__parent_pointer __parent, __node_base_pointer& __child, __node_base_pointer __new_node) _NOEXCEPT;
+  __insert_node_at(__end_node_pointer __parent, __node_base_pointer& __child, __node_base_pointer __new_node) _NOEXCEPT;
 
   template <class _Key>
   _LIBCPP_HIDE_FROM_ABI iterator find(const _Key& __v);
@@ -1203,14 +1169,14 @@ public:
   // FIXME: Make this function const qualified. Unfortunately doing so
   // breaks existing code which uses non-const callable comparators.
   template <class _Key>
-  _LIBCPP_HIDE_FROM_ABI __node_base_pointer& __find_equal(__parent_pointer& __parent, const _Key& __v);
+  _LIBCPP_HIDE_FROM_ABI __node_base_pointer& __find_equal(__end_node_pointer& __parent, const _Key& __v);
   template <class _Key>
-  _LIBCPP_HIDE_FROM_ABI __node_base_pointer& __find_equal(__parent_pointer& __parent, const _Key& __v) const {
+  _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);
   }
   template <class _Key>
   _LIBCPP_HIDE_FROM_ABI __node_base_pointer&
-  __find_equal(const_iterator __hint, __parent_pointer& __parent, __node_base_pointer& __dummy, const _Key& __v);
+  __find_equal(const_iterator __hint, __end_node_pointer& __parent, __node_base_pointer& __dummy, const _Key& __v);
 
   _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>());
@@ -1224,11 +1190,12 @@ 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 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_low(__end_node_pointer& __parent, const value_type& __v);
+
+  _LIBCPP_HIDE_FROM_ABI __node_base_pointer& __find_leaf_high(__end_node_pointer& __parent, const value_type& __v);
 
   _LIBCPP_HIDE_FROM_ABI __node_base_pointer&
-  __find_leaf(const_iterator __hint, __parent_pointer& __parent, const value_type& __v);
+  __find_leaf(const_iterator __hint, __end_node_pointer& __parent, const value_type& __v);
 
   template <class... _Args>
   _LIBCPP_HIDE_FROM_ABI __node_holder __construct_node(_Args&&... __args);
@@ -1400,8 +1367,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, 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, "__assign_multi may only be called with the containers value_type");
   if (size() != 0) {
     _DetachedTreeCache __cache(this);
     for (; __cache.__get() && __first != __last; ++__first) {
@@ -1435,7 +1402,7 @@ __tree<_Tp, _Compare, _Allocator>::__tree(__tree&& __t) _NOEXCEPT_(
   if (size() == 0)
     __begin_node() = __end_node();
   else {
-    __end_node()->__left_->__parent_ = static_cast<__parent_pointer>(__end_node());
+    __end_node()->__left_->__parent_ = static_cast<__end_node_pointer>(__end_node());
     __t.__begin_node()               = __t.__end_node();
     __t.__end_node()->__left_        = nullptr;
     __t.size()                       = 0;
@@ -1451,7 +1418,7 @@ __tree<_Tp, _Compare, _Allocator>::__tree(__tree&& __t, const allocator_type& __
     else {
       __begin_node()                   = __t.__begin_node();
       __end_node()->__left_            = __t.__end_node()->__left_;
-      __end_node()->__left_->__parent_ = static_cast<__parent_pointer>(__end_node());
+      __end_node()->__left_->__parent_ = static_cast<__end_node_pointer>(__end_node());
       size()                           = __t.size();
       __t.__begin_node()               = __t.__end_node();
       __t.__end_node()->__left_        = nullptr;
@@ -1474,7 +1441,7 @@ void __tree<_Tp, _Compare, _Allocator>::__move_assign(__tree& __t, true_type)
   if (size() == 0)
     __begin_node() = __end_node();
   else {
-    __end_node()->__left_->__parent_ = static_cast<__parent_pointer>(__end_node());
+    __end_node()->__left_->__parent_ = static_cast<__end_node_pointer>(__end_node());
     __t.__begin_node()               = __t.__end_node();
     __t.__end_node()->__left_        = nullptr;
     __t.size()                       = 0;
@@ -1547,11 +1514,11 @@ void __tree<_Tp, _Compare, _Allocator>::swap(__tree& __t)
   if (size() == 0)
     __begin_node() = __end_node();
   else
-    __end_node()->__left_->__parent_ = static_cast<__parent_pointer>(__end_node());
+    __end_node()->__left_->__parent_ = __end_node();
   if (__t.size() == 0)
     __t.__begin_node() = __t.__end_node();
   else
-    __t.__end_node()->__left_->__parent_ = static_cast<__parent_pointer>(__t.__end_node());
+    __t.__end_node()->__left_->__parent_ = __t.__end_node();
 }
 
 template <class _Tp, class _Compare, class _Allocator>
@@ -1567,7 +1534,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 value_type& __v) {
+__tree<_Tp, _Compare, _Allocator>::__find_leaf_low(__end_node_pointer& __parent, const value_type& __v) {
   __node_pointer __nd = __root();
   if (__nd != nullptr) {
     while (true) {
@@ -1575,20 +1542,20 @@ __tree<_Tp, _Compare, _Allocator>::__find_leaf_low(__parent_pointer& __parent, c
         if (__nd->__right_ != nullptr)
           __nd = static_cast<__node_pointer>(__nd->__right_);
         else {
-          __parent = static_cast<__parent_pointer>(__nd);
+          __parent = static_cast<__end_node_pointer>(__nd);
           return __nd->__right_;
         }
       } else {
         if (__nd->__left_ != nullptr)
           __nd = static_cast<__node_pointer>(__nd->__left_);
         else {
-          __parent = static_cast<__parent_pointer>(__nd);
+          __parent = static_cast<__end_node_pointer>(__nd);
           return __parent->__left_;
         }
       }
     }
   }
-  __parent = static_cast<__parent_pointer>(__end_node());
+  __parent = __end_node();
   return __parent->__left_;
 }
 
@@ -1597,7 +1564,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 value_type& __v) {
+__tree<_Tp, _Compare, _Allocator>::__find_leaf_high(__end_node_pointer& __parent, const value_type& __v) {
   __node_pointer __nd = __root();
   if (__nd != nullptr) {
     while (true) {
@@ -1605,20 +1572,20 @@ __tree<_Tp, _Compare, _Allocator>::__find_leaf_high(__parent_pointer& __parent,
         if (__nd->__left_ != nullptr)
           __nd = static_cast<__node_pointer>(__nd->__left_);
         else {
-          __parent = static_cast<__parent_pointer>(__nd);
+          __parent = static_cast<__end_node_pointer>(__nd);
           return __parent->__left_;
         }
       } else {
         if (__nd->__right_ != nullptr)
           __nd = static_cast<__node_pointer>(__nd->__right_);
         else {
-          __parent = static_cast<__parent_pointer>(__nd);
+          __parent = static_cast<__end_node_pointer>(__nd);
           return __nd->__right_;
         }
       }
     }
   }
-  __parent = static_cast<__parent_pointer>(__end_node());
+  __parent = __end_node();
   return __parent->__left_;
 }
 
@@ -1630,7 +1597,7 @@ __tree<_Tp, _Compare, _Allocator>::__find_leaf_high(__parent_pointer& __parent,
 // 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 value_type& __v) {
+    const_iterator __hint, __end_node_pointer& __parent, const value_type& __v) {
   if (__hint == end() || !value_comp()(*__hint, __v)) // check before
   {
     // __v <= *__hint
@@ -1638,10 +1605,10 @@ typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer& __tree<_Tp, _Co
     if (__prior == begin() || !value_comp()(__v, *--__prior)) {
       // *prev(__hint) <= __v <= *__hint
       if (__hint.__ptr_->__left_ == nullptr) {
-        __parent = static_cast<__parent_pointer>(__hint.__ptr_);
+        __parent = static_cast<__end_node_pointer>(__hint.__ptr_);
         return __parent->__left_;
       } else {
-        __parent = static_cast<__parent_pointer>(__prior.__ptr_);
+        __parent = static_cast<__end_node_pointer>(__prior.__ptr_);
         return static_cast<__node_base_pointer>(__prior.__ptr_)->__right_;
       }
     }
@@ -1659,7 +1626,7 @@ typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer& __tree<_Tp, _Co
 template <class _Tp, class _Compare, class _Allocator>
 template <class _Key>
 typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer&
-__tree<_Tp, _Compare, _Allocator>::__find_equal(__parent_pointer& __parent, const _Key& __v) {
+__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) {
@@ -1669,7 +1636,7 @@ __tree<_Tp, _Compare, _Allocator>::__find_equal(__parent_pointer& __parent, cons
           __nd_ptr = std::addressof(__nd->__left_);
           __nd     = static_cast<__node_pointer>(__nd->__left_);
         } else {
-          __parent = static_cast<__parent_pointer>(__nd);
+          __parent = static_cast<__end_node_pointer>(__nd);
           return __parent->__left_;
         }
       } else if (value_comp()(__nd->__value_, __v)) {
@@ -1677,16 +1644,16 @@ __tree<_Tp, _Compare, _Allocator>::__find_equal(__parent_pointer& __parent, cons
           __nd_ptr = std::addressof(__nd->__right_);
           __nd     = static_cast<__node_pointer>(__nd->__right_);
         } else {
-          __parent = static_cast<__parent_pointer>(__nd);
+          __parent = static_cast<__end_node_pointer>(__nd);
           return __nd->__right_;
         }
       } else {
-        __parent = static_cast<__parent_pointer>(__nd);
+        __parent = static_cast<__end_node_pointer>(__nd);
         return *__nd_ptr;
       }
     }
   }
-  __parent = static_cast<__parent_pointer>(__end_node());
+  __parent = __end_node();
   return __parent->__left_;
 }
 
@@ -1700,7 +1667,7 @@ __tree<_Tp, _Compare, _Allocator>::__find_equal(__parent_pointer& __parent, cons
 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, __parent_pointer& __parent, __node_base_pointer& __dummy, const _Key& __v) {
+    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
@@ -1708,10 +1675,10 @@ typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer& __tree<_Tp, _Co
     if (_...
[truncated]

philnik777 · 2025-06-25T19:06:10Z

@Michael137 Looks like I'm breaking the formatters again. Sorry. I feel like I'll have to figure our how to debug the formatters at the rate I'm breaking them currently. This time I'm really not sure what the problem is. I've specifically allowed generating debug info for __node_pointer, but that doesn't seem to actually solve the problem.

Michael137 · 2025-06-26T09:02:42Z

@Michael137 Looks like I'm breaking the formatters again. Sorry. I feel like I'll have to figure our how to debug the formatters at the rate I'm breaking them currently. This time I'm really not sure what the problem is. I've specifically allowed generating debug info for __node_pointer, but that doesn't seem to actually solve the problem.

Hehe no worries. I'll have a look. Glad we're catching these pre-merge now

Michael137 · 2025-06-26T16:47:52Z

Looks like this __node_pointer retrieval here is failing:

   441    auto node_pointer_type =
   442        tree_iter_sp->GetCompilerType().GetDirectNestedTypeWithName(
   443            "__node_pointer");
-> 444    if (!node_pointer_type.IsValid())
   445      return lldb::ChildCacheState::eRefetch;
   446

Michael137 · 2025-06-27T07:48:14Z

Yea LLDB tries to grab the __node_pointer typedef for formatting the std::map::iterator. Any chance we can avoid adding the _LIBCPP_NODEBUG attribute to it? That would be currently the easiest path forward. Otherwise we'd have to think about how to rewrite the formatter

Michael137 · 2025-06-27T07:49:13Z

There is this comment just above the code:

  // Type is __tree_iterator::__node_pointer
  // (We could alternatively also get this from the template argument)
  auto node_pointer_type =
      tree_iter_sp->GetCompilerType().GetDirectNestedTypeWithName(
          "__node_pointer");

Will try to see if that could work

philnik777 · 2025-06-27T08:07:00Z

Yea LLDB tries to grab the __node_pointer typedef for formatting the std::map::iterator. Any chance we can avoid adding the _LIBCPP_NODEBUG attribute to it? That would be currently the easiest path forward. Otherwise we'd have to think about how to rewrite the formatter

I've just done that and the tests pass again. Thanks! I'Ve also added a comment that the pretty printer relies on this, so people know that it's important.

…lvm#145295) Most of the diff is replacing `__parent_pointer` with `__end_node_pointer`. The most interesting diff is that the pointer aliases are now defined directly inside `__tree` instead of a separate traits class.

philnik777 force-pushed the simplify_tree2 branch 2 times, most recently from eb97cce to db83c52 Compare June 24, 2025 08:28

philnik777 marked this pull request as ready for review June 25, 2025 15:41

philnik777 requested a review from a team as a code owner June 25, 2025 15:41

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jun 25, 2025

ldionne approved these changes Jun 25, 2025

View reviewed changes

[libc++] Remove a bunch of unnecessary type indirections from __tree

9c51197

philnik777 force-pushed the simplify_tree2 branch from db83c52 to 9c51197 Compare June 26, 2025 17:52

philnik777 merged commit d163ab3 into llvm:main Jun 27, 2025
106 of 117 checks passed

philnik777 deleted the simplify_tree2 branch June 27, 2025 08:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++] Remove a bunch of unnecessary type indirections from __tree #145295

[libc++] Remove a bunch of unnecessary type indirections from __tree #145295

Uh oh!

philnik777 commented Jun 23, 2025

Uh oh!

llvmbot commented Jun 25, 2025

Uh oh!

philnik777 commented Jun 25, 2025

Uh oh!

Michael137 commented Jun 26, 2025

Uh oh!

Michael137 commented Jun 26, 2025

Uh oh!

Michael137 commented Jun 27, 2025

Uh oh!

Michael137 commented Jun 27, 2025

Uh oh!

philnik777 commented Jun 27, 2025

Uh oh!

Uh oh!

Uh oh!

[libc++] Remove a bunch of unnecessary type indirections from __tree #145295

[libc++] Remove a bunch of unnecessary type indirections from __tree #145295

Uh oh!

Conversation

philnik777 commented Jun 23, 2025

Uh oh!

llvmbot commented Jun 25, 2025

Uh oh!

philnik777 commented Jun 25, 2025

Uh oh!

Michael137 commented Jun 26, 2025

Uh oh!

Michael137 commented Jun 26, 2025

Uh oh!

Michael137 commented Jun 27, 2025

Uh oh!

Michael137 commented Jun 27, 2025

Uh oh!

philnik777 commented Jun 27, 2025

Uh oh!

Uh oh!

Uh oh!