[libcxx-commits] [libcxx] 990ea39 - [libc++] Add a few _LIBCPP_ASSERTs in __tree

[Louis Dionne via libcxx-commits]

Author: Louis Dionne
Date: 2022-05-05T11:29:48-04:00
New Revision: 990ea3925b7ad07fa4f7bd24398c4338769fdf05

URL: https://github.com/llvm/llvm-project/commit/990ea3925b7ad07fa4f7bd24398c4338769fdf05
DIFF: https://github.com/llvm/llvm-project/commit/990ea3925b7ad07fa4f7bd24398c4338769fdf05.diff

LOG: [libc++] Add a few _LIBCPP_ASSERTs in __tree

Several helper functions specify preconditions as comments, but we never
check them. I ran across a bug report (without a reproducer) in this code,
and I thought that having these assertions in place would make it easier
to troubleshoot.

Differential Revision: https://reviews.llvm.org/D124477

Added: 
    

Modified: 
    libcxx/include/__tree

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__tree b/libcxx/include/__tree
index f8d2226e66fbb..c493ff9be03e1 100644
--- a/libcxx/include/__tree
+++ b/libcxx/include/__tree
@@ -141,35 +141,35 @@ __tree_invariant(_NodePtr __root)
 }
 
 // Returns:  pointer to the left-most node under __x.
-// Precondition:  __x != nullptr.
 template <class _NodePtr>
 inline _LIBCPP_INLINE_VISIBILITY
 _NodePtr
 __tree_min(_NodePtr __x) _NOEXCEPT
 {
+    _LIBCPP_ASSERT(__x != nullptr, "Root node shouldn't be null");
     while (__x->__left_ != nullptr)
         __x = __x->__left_;
     return __x;
 }
 
 // Returns:  pointer to the right-most node under __x.
-// Precondition:  __x != nullptr.
 template <class _NodePtr>
 inline _LIBCPP_INLINE_VISIBILITY
 _NodePtr
 __tree_max(_NodePtr __x) _NOEXCEPT
 {
+    _LIBCPP_ASSERT(__x != nullptr, "Root node shouldn't be null");
     while (__x->__right_ != nullptr)
         __x = __x->__right_;
     return __x;
 }
 
 // Returns:  pointer to the next in-order node after __x.
-// Precondition:  __x != nullptr.
 template <class _NodePtr>
 _NodePtr
 __tree_next(_NodePtr __x) _NOEXCEPT
 {
+    _LIBCPP_ASSERT(__x != nullptr, "node shouldn't be null");
     if (__x->__right_ != nullptr)
         return _VSTD::__tree_min(__x->__right_);
     while (!_VSTD::__tree_is_left_child(__x))
@@ -182,6 +182,7 @@ inline _LIBCPP_INLINE_VISIBILITY
 _EndNodePtr
 __tree_next_iter(_NodePtr __x) _NOEXCEPT
 {
+    _LIBCPP_ASSERT(__x != nullptr, "node shouldn't be null");
     if (__x->__right_ != nullptr)
         return static_cast<_EndNodePtr>(_VSTD::__tree_min(__x->__right_));
     while (!_VSTD::__tree_is_left_child(__x))
@@ -190,13 +191,13 @@ __tree_next_iter(_NodePtr __x) _NOEXCEPT
 }
 
 // Returns:  pointer to the previous in-order node before __x.
-// Precondition:  __x != nullptr.
 // Note: __x may be the end node.
 template <class _NodePtr, class _EndNodePtr>
 inline _LIBCPP_INLINE_VISIBILITY
 _NodePtr
 __tree_prev_iter(_EndNodePtr __x) _NOEXCEPT
 {
+    _LIBCPP_ASSERT(__x != nullptr, "node shouldn't be null");
     if (__x->__left_ != nullptr)
         return _VSTD::__tree_max(__x->__left_);
     _NodePtr __xx = static_cast<_NodePtr>(__x);
@@ -206,11 +207,11 @@ __tree_prev_iter(_EndNodePtr __x) _NOEXCEPT
 }
 
 // Returns:  pointer to a node which has no children
-// Precondition:  __x != nullptr.
 template <class _NodePtr>
 _NodePtr
 __tree_leaf(_NodePtr __x) _NOEXCEPT
 {
+    _LIBCPP_ASSERT(__x != nullptr, "node shouldn't be null");
     while (true)
     {
         if (__x->__left_ != nullptr)
@@ -230,11 +231,12 @@ __tree_leaf(_NodePtr __x) _NOEXCEPT
 
 // Effects:  Makes __x->__right_ the subtree root with __x as its left child
 //           while preserving in-order order.
-// Precondition:  __x->__right_ != nullptr
 template <class _NodePtr>
 void
 __tree_left_rotate(_NodePtr __x) _NOEXCEPT
 {
+    _LIBCPP_ASSERT(__x != nullptr, "node shouldn't be null");
+    _LIBCPP_ASSERT(__x->__right_ != nullptr, "node should have a right child");
     _NodePtr __y = __x->__right_;
     __x->__right_ = __y->__left_;
     if (__x->__right_ != nullptr)
@@ -250,11 +252,12 @@ __tree_left_rotate(_NodePtr __x) _NOEXCEPT
 
 // Effects:  Makes __x->__left_ the subtree root with __x as its right child
 //           while preserving in-order order.
-// Precondition:  __x->__left_ != nullptr
 template <class _NodePtr>
 void
 __tree_right_rotate(_NodePtr __x) _NOEXCEPT
 {
+    _LIBCPP_ASSERT(__x != nullptr, "node shouldn't be null");
+    _LIBCPP_ASSERT(__x->__left_ != nullptr, "node should have a left child");
     _NodePtr __y = __x->__left_;
     __x->__left_ = __y->__right_;
     if (__x->__left_ != nullptr)
@@ -269,8 +272,7 @@ __tree_right_rotate(_NodePtr __x) _NOEXCEPT
 }
 
 // Effects:  Rebalances __root after attaching __x to a leaf.
-// Precondition:  __root != nulptr && __x != nullptr.
-//                __x has no children.
+// Precondition:  __x has no children.
 //                __x == __root or == a direct or indirect child of __root.
 //                If __x were to be unlinked from __root (setting __root to
 //                  nullptr if __root == __x), __tree_invariant(__root) == true.
@@ -280,6 +282,8 @@ template <class _NodePtr>
 void
 __tree_balance_after_insert(_NodePtr __root, _NodePtr __x) _NOEXCEPT
 {
+    _LIBCPP_ASSERT(__root != nullptr, "Root of the tree shouldn't be null");
+    _LIBCPP_ASSERT(__x != nullptr, "Can't attach null node to a leaf");
     __x->__is_black_ = __x == __root;
     while (__x != __root && !__x->__parent_unsafe()->__is_black_)
     {
@@ -339,9 +343,7 @@ __tree_balance_after_insert(_NodePtr __root, _NodePtr __x) _NOEXCEPT
     }
 }
 
-// Precondition:  __root != nullptr && __z != nullptr.
-//                __tree_invariant(__root) == true.
-//                __z == __root or == a direct or indirect child of __root.
+// Precondition:  __z == __root or == a direct or indirect child of __root.
 // Effects:  unlinks __z from the tree rooted at __root, rebalancing as needed.
 // Postcondition: __tree_invariant(end_node->__left_) == true && end_node->__left_
 //                nor any of its children refer to __z.  end_node->__left_
@@ -350,6 +352,9 @@ template <class _NodePtr>
 void
 __tree_remove(_NodePtr __root, _NodePtr __z) _NOEXCEPT
 {
+    _LIBCPP_ASSERT(__root != nullptr, "Root node should not be null");
+    _LIBCPP_ASSERT(__z != nullptr, "The node to remove should not be null");
+    _LIBCPP_DEBUG_ASSERT(__tree_invariant(__root), "The tree invariants should hold");
     // __z will be removed from the tree.  Client still needs to destruct/deallocate it
     // __y is either __z, or if __z has two children, __tree_next(__z).
     // __y will have at most one child.