[libc++][NFC] Use _LIBCPP_DEBUG_ASSERT in <list>

[philnik]

Summary

Use _LIBCPP_DEBUG_ASSERT in <list>

[• Quuxplusone]

LGTM once my comment on lines 2007–2008 is addressed. Thanks, this is good cleanup!

libcxx/include/list

1435–1436

I just wanted to call out that personally I dislike the old code's style of breaking a string literal randomly in the middle, because it means that if I get that error message and grep to find where it was generated — git grep 'iterator not referring to this list' — I would have gotten no results. Putting the message all on one line like you just did is the Right Way because it is greppable. :)

1973

Consider leaving this string message (all in one literal but) on the next line. I.e. don't change lines 2007–2008 at all, I think is what I'm saying, right?

2063

Consider s/_LIBCPP_ASSERT/_LIBCPP_DEBUG_ASSERT/ on this line. IIUC, the change should have no effect, because this entire loop is guarded under _LIBCPP_DEBUG_LEVEL == 2 already; but it would be more self-documenting — this is still a "debug-mode assertion", with all the connotations of _LIBCPP_DEBUG_ASSERT rather than whatever is connoted by a plain _LIBCPP_ASSERT.

[philnik]

• Addressed comments

libcxx/include/list

__list_iterator& operator=(const __list_iterator& __p)
         return *this;
     }
 
 #endif // _LIBCPP_DEBUG_LEVEL == 2
 
     _LIBCPP_INLINE_VISIBILITY
     reference operator*() const
     {
-#if _LIBCPP_DEBUG_LEVEL == 2
-        _LIBCPP_ASSERT(__get_const_db()->__dereferenceable(this),
+        _LIBCPP_DEBUG_ASSERT(__get_const_db()->__dereferenceable(this),
                              "Attempted to dereference a non-dereferenceable list::iterator");
-#endif
         return __ptr_->__as_node()->__value_;
     }
     _LIBCPP_INLINE_VISIBILITY
     pointer operator->() const
     {
-#if _LIBCPP_DEBUG_LEVEL == 2
-        _LIBCPP_ASSERT(__get_const_db()->__dereferenceable(this),
+        _LIBCPP_DEBUG_ASSERT(__get_const_db()->__dereferenceable(this),
                              "Attempted to dereference a non-dereferenceable list::iterator");
-#endif
         return pointer_traits<pointer>::pointer_to(__ptr_->__as_node()->__value_);
     }
 
     _LIBCPP_INLINE_VISIBILITY
     __list_iterator& operator++()
     {
-#if _LIBCPP_DEBUG_LEVEL == 2
-        _LIBCPP_ASSERT(__get_const_db()->__dereferenceable(this),
+        _LIBCPP_DEBUG_ASSERT(__get_const_db()->__dereferenceable(this),
                              "Attempted to increment a non-incrementable list::iterator");
-#endif
         __ptr_ = __ptr_->__next_;
         return *this;
     }
     _LIBCPP_INLINE_VISIBILITY
     __list_iterator operator++(int) {__list_iterator __t(*this); ++(*this); return __t;}
 
     _LIBCPP_INLINE_VISIBILITY
     __list_iterator& operator--()
     {
-#if _LIBCPP_DEBUG_LEVEL == 2
-        _LIBCPP_ASSERT(__get_const_db()->__decrementable(this),
+        _LIBCPP_DEBUG_ASSERT(__get_const_db()->__decrementable(this),
                              "Attempted to decrement a non-decrementable list::iterator");
-#endif
         __ptr_ = __ptr_->__prev_;
         return *this;
     }
     _LIBCPP_INLINE_VISIBILITY
     __list_iterator operator--(int) {__list_iterator __t(*this); --(*this); return __t;}
 
     friend _LIBCPP_INLINE_VISIBILITY
     bool operator==(const __list_iterator& __x, const __list_iterator& __y)
__list_const_iterator& operator=(const __list_const_iterator& __p)
         }
         return *this;
     }
 
 #endif // _LIBCPP_DEBUG_LEVEL == 2
     _LIBCPP_INLINE_VISIBILITY
     reference operator*() const
     {
-#if _LIBCPP_DEBUG_LEVEL == 2
-        _LIBCPP_ASSERT(__get_const_db()->__dereferenceable(this),
+        _LIBCPP_DEBUG_ASSERT(__get_const_db()->__dereferenceable(this),
                              "Attempted to dereference a non-dereferenceable list::const_iterator");
-#endif
         return __ptr_->__as_node()->__value_;
     }
     _LIBCPP_INLINE_VISIBILITY
     pointer operator->() const
     {
-#if _LIBCPP_DEBUG_LEVEL == 2
-        _LIBCPP_ASSERT(__get_const_db()->__dereferenceable(this),
+        _LIBCPP_DEBUG_ASSERT(__get_const_db()->__dereferenceable(this),
                              "Attempted to dereference a non-dereferenceable list::const_iterator");
-#endif
         return pointer_traits<pointer>::pointer_to(__ptr_->__as_node()->__value_);
     }
 
     _LIBCPP_INLINE_VISIBILITY
     __list_const_iterator& operator++()
     {
-#if _LIBCPP_DEBUG_LEVEL == 2
-        _LIBCPP_ASSERT(__get_const_db()->__dereferenceable(this),
+        _LIBCPP_DEBUG_ASSERT(__get_const_db()->__dereferenceable(this),
                              "Attempted to increment a non-incrementable list::const_iterator");
-#endif
         __ptr_ = __ptr_->__next_;
         return *this;
     }
     _LIBCPP_INLINE_VISIBILITY
     __list_const_iterator operator++(int) {__list_const_iterator __t(*this); ++(*this); return __t;}
 
     _LIBCPP_INLINE_VISIBILITY
     __list_const_iterator& operator--()
     {
-#if _LIBCPP_DEBUG_LEVEL == 2
-        _LIBCPP_ASSERT(__get_const_db()->__decrementable(this),
+        _LIBCPP_DEBUG_ASSERT(__get_const_db()->__decrementable(this),
                              "Attempted to decrement a non-decrementable list::const_iterator");
-#endif
         __ptr_ = __ptr_->__prev_;
         return *this;
     }
     _LIBCPP_INLINE_VISIBILITY
     __list_const_iterator operator--(int) {__list_const_iterator __t(*this); --(*this); return __t;}
 
     friend _LIBCPP_INLINE_VISIBILITY
     bool operator==(const __list_const_iterator& __x, const __list_const_iterator& __y)

 {
     return allocator_type(base::__node_alloc());
 }
 
 template <class _Tp, class _Alloc>
 typename list<_Tp, _Alloc>::iterator
 list<_Tp, _Alloc>::insert(const_iterator __p, const value_type& __x)
 {
-#if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
-        "list::insert(iterator, x) called with an iterator not"
-        " referring to this list");
-#endif
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
+                         "list::insert(iterator, x) called with an iterator not referring to this list");
     __node_allocator& __na = base::__node_alloc();
     __hold_pointer __hold = __allocate_node(__na);
     __node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), __x);
     __link_nodes(__p.__ptr_, __hold->__as_link(), __hold->__as_link());
     ++base::__sz();
 #if _LIBCPP_DEBUG_LEVEL == 2
     return iterator(__hold.release()->__as_link(), this);
 #else
     return iterator(__hold.release()->__as_link());
 #endif
 }
 
 template <class _Tp, class _Alloc>
 typename list<_Tp, _Alloc>::iterator
 list<_Tp, _Alloc>::insert(const_iterator __p, size_type __n, const value_type& __x)
 {
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
+                         "list::insert(iterator, n, x) called with an iterator not referring to this list");
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
-        "list::insert(iterator, n, x) called with an iterator not"
-        " referring to this list");
     iterator __r(__p.__ptr_, this);
 #else
     iterator __r(__p.__ptr_);
 #endif
     if (__n > 0)
     {
         size_type __ds = 0;
         __node_allocator& __na = base::__node_alloc();

 }
 
 template <class _Tp, class _Alloc>
 template <class _InpIter>
 typename list<_Tp, _Alloc>::iterator
 list<_Tp, _Alloc>::insert(const_iterator __p, _InpIter __f, _InpIter __l,
              typename enable_if<__is_cpp17_input_iterator<_InpIter>::value>::type*)
 {
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
+                         "list::insert(iterator, range) called with an iterator not referring to this list");
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
-        "list::insert(iterator, range) called with an iterator not"
-        " referring to this list");
     iterator __r(__p.__ptr_, this);
 #else
     iterator __r(__p.__ptr_);
 #endif
     if (__f != __l)
     {
         size_type __ds = 0;
         __node_allocator& __na = base::__node_alloc();

 #endif
 }
 
 template <class _Tp, class _Alloc>
 template <class... _Args>
 typename list<_Tp, _Alloc>::iterator
 list<_Tp, _Alloc>::emplace(const_iterator __p, _Args&&... __args)
 {
-#if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
-        "list::emplace(iterator, args...) called with an iterator not"
-        " referring to this list");
-#endif
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
+                         "list::emplace(iterator, args...) called with an iterator not referring to this list");
     __node_allocator& __na = base::__node_alloc();
     __hold_pointer __hold = __allocate_node(__na);
     __node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), _VSTD::forward<_Args>(__args)...);
     __link_pointer __nl = __hold.get()->__as_link();
     __link_nodes(__p.__ptr_, __nl, __nl);
     ++base::__sz();
     __hold.release();
 #if _LIBCPP_DEBUG_LEVEL == 2
     return iterator(__nl, this);
 #else
     return iterator(__nl);
 #endif
 }
 
 template <class _Tp, class _Alloc>
 typename list<_Tp, _Alloc>::iterator
 list<_Tp, _Alloc>::insert(const_iterator __p, value_type&& __x)
 {
-#if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
-        "list::insert(iterator, x) called with an iterator not"
-        " referring to this list");
-#endif
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
+                         "list::insert(iterator, x) called with an iterator not referring to this list");
     __node_allocator& __na = base::__node_alloc();
     __hold_pointer __hold = __allocate_node(__na);
     __node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), _VSTD::move(__x));
     __link_pointer __nl = __hold->__as_link();
     __link_nodes(__p.__ptr_, __nl, __nl);
     ++base::__sz();
     __hold.release();
 #if _LIBCPP_DEBUG_LEVEL == 2
#endif
     __node_alloc_traits::destroy(__na, _VSTD::addressof(__np->__value_));
     __node_alloc_traits::deallocate(__na, __np, 1);
 }
 
 template <class _Tp, class _Alloc>
 typename list<_Tp, _Alloc>::iterator
 list<_Tp, _Alloc>::erase(const_iterator __p)
 {
-#if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
-        "list::erase(iterator) called with an iterator not"
-        " referring to this list");
-#endif
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
+                         "list::erase(iterator) called with an iterator not referring to this list");
     _LIBCPP_ASSERT(__p != end(),
         "list::erase(iterator) called with a non-dereferenceable iterator");
     __node_allocator& __na = base::__node_alloc();
     __link_pointer __n = __p.__ptr_;
     __link_pointer __r = __n->__next_;
     base::__unlink_nodes(__n, __n);
     --base::__sz();
 #if _LIBCPP_DEBUG_LEVEL == 2
#else
     return iterator(__r);
 #endif
 }
 
 template <class _Tp, class _Alloc>
 typename list<_Tp, _Alloc>::iterator
 list<_Tp, _Alloc>::erase(const_iterator __f, const_iterator __l)
 {
-#if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__f)) == this,
-        "list::erase(iterator, iterator) called with an iterator not"
-        " referring to this list");
-   _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__l)) == this,
-        "list::erase(iterator, iterator) called with an iterator not"
-        " referring to this list");
-#endif
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__f)) == this,
+                         "list::erase(iterator, iterator) called with an iterator not referring to this list");
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__l)) == this,
+                         "list::erase(iterator, iterator) called with an iterator not referring to this list");
     if (__f != __l)
     {
         __node_allocator& __na = base::__node_alloc();
         base::__unlink_nodes(__f.__ptr_, __l.__ptr_->__prev_);
         while (__f != __l)
         {
             __link_pointer __n = __f.__ptr_;
             ++__f;
#endif // _LIBCPP_NO_EXCEPTIONS
         base::__sz() += __ds;
     }
 }
 
 template <class _Tp, class _Alloc>
 void
 list<_Tp, _Alloc>::splice(const_iterator __p, list& __c)
 {
-    _LIBCPP_ASSERT(this != _VSTD::addressof(__c),
-                   "list::splice(iterator, list) called with this == &list");
-#if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
-        "list::splice(iterator, list) called with an iterator not"
-        " referring to this list");
-#endif
+    _LIBCPP_ASSERT(this != _VSTD::addressof(__c), "list::splice(iterator, list) called with this == &list");
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
+                         "list::splice(iterator, list) called with an iterator not referring to this list");
     if (!__c.empty())
     {
         __link_pointer __f = __c.__end_.__next_;
         __link_pointer __l = __c.__end_.__prev_;
         base::__unlink_nodes(__f, __l);
         __link_nodes(__p.__ptr_, __f, __l);
         base::__sz() += __c.__sz();
         __c.__sz() = 0;

 #endif
     }
 }
 
 template <class _Tp, class _Alloc>
 void
 list<_Tp, _Alloc>::splice(const_iterator __p, list& __c, const_iterator __i)
 {
-#if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
-        "list::splice(iterator, list, iterator) called with the first iterator"
-        " not referring to this list");
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__i)) == _VSTD::addressof(__c),
-        "list::splice(iterator, list, iterator) called with the second iterator"
-        " not referring to the list argument");
-    _LIBCPP_ASSERT(__get_const_db()->__dereferenceable(_VSTD::addressof(__i)),
-        "list::splice(iterator, list, iterator) called with the second iterator"
-        " not dereferenceable");
-#endif
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
+        "list::splice(iterator, list, iterator) called with the first iterator not referring to this list");
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__i)) == _VSTD::addressof(__c),
+        "list::splice(iterator, list, iterator) called with the second iterator not referring to the list argument");
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__dereferenceable(_VSTD::addressof(__i)),
+        "list::splice(iterator, list, iterator) called with the second iterator not dereferenceable");
+
     if (__p.__ptr_ != __i.__ptr_ && __p.__ptr_ != __i.__ptr_->__next_)
     {
         __link_pointer __f = __i.__ptr_;
         base::__unlink_nodes(__f, __f);
         __link_nodes(__p.__ptr_, __f, __f);
         --__c.__sz();
         ++base::__sz();
 #if _LIBCPP_DEBUG_LEVEL == 2

 #endif
     }
 }
 
 template <class _Tp, class _Alloc>
 void
 list<_Tp, _Alloc>::splice(const_iterator __p, list& __c, const_iterator __f, const_iterator __l)
 {
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
+        "list::splice(iterator, list, iterator, iterator) called with first iterator not referring to this list");
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__f)) == _VSTD::addressof(__c),
+        "list::splice(iterator, list, iterator, iterator) called with second iterator not referring to the list argument");
+    _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__l)) == _VSTD::addressof(__c),
+        "list::splice(iterator, list, iterator, iterator) called with third iterator not referring to the list argument");
+
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
-        "list::splice(iterator, list, iterator, iterator) called with first iterator not"
-        " referring to this list");
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__f)) == _VSTD::addressof(__c),
-        "list::splice(iterator, list, iterator, iterator) called with second iterator not"
-        " referring to the list argument");
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__l)) == _VSTD::addressof(__c),
-        "list::splice(iterator, list, iterator, iterator) called with third iterator not"
-        " referring to the list argument");
     if (this == _VSTD::addressof(__c))
     {
         for (const_iterator __i = __f; __i != __l; ++__i)
             _LIBCPP_ASSERT(__i != __p,
                            "list::splice(iterator, list, iterator, iterator)"
                            " called with the first iterator within the range"
                            " of the second and third iterators");
     }
 #endif
     if (__f != __l)
     {
         __link_pointer __first = __f.__ptr_;