From 990ea3925b7ad07fa4f7bd24398c4338769fdf05 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Tue, 26 Apr 2022 15:02:40 -0600 Subject: [PATCH] [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 --- libcxx/include/__tree | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/libcxx/include/__tree b/libcxx/include/__tree index f8d2226e66fb..c493ff9be03e 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 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 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 _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 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 _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 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 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 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 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.