[libcxx-commits] [PATCH] D124477: [libc++] Add a few _LIBCPP_ASSERTs in __tree

[Louis Dionne via Phabricator via libcxx-commits]

ldionne created this revision.
Herald added a project: All.
ldionne requested review of this revision.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124477

Files:
  libcxx/include/__tree


Index: libcxx/include/__tree
===================================================================
--- libcxx/include/__tree
+++ libcxx/include/__tree
@@ -147,6 +147,7 @@
 _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;
@@ -159,6 +160,7 @@
 _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;
@@ -170,6 +172,7 @@
 _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 +185,7 @@
 _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))
@@ -197,6 +201,7 @@
 _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);
@@ -211,6 +216,7 @@
 _NodePtr
 __tree_leaf(_NodePtr __x) _NOEXCEPT
 {
+    _LIBCPP_ASSERT(__x != nullptr, "node shouldn't be null");
     while (true)
     {
         if (__x->__left_ != nullptr)
@@ -235,6 +241,8 @@
 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)
@@ -255,6 +263,8 @@
 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,7 +279,7 @@
 }
 
 // Effects:  Rebalances __root after attaching __x to a leaf.
-// Precondition:  __root != nulptr && __x != nullptr.
+// Precondition:  __root != nullptr && __x != nullptr.
 //                __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
@@ -280,6 +290,8 @@
 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_)
     {
@@ -350,6 +362,8 @@
 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");
     // __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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124477.425300.patch
Type: text/x-patch
Size: 3459 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20220426/e58f3510/attachment.bin>