[libc++] Audit all uses of _LIBCPP_ASSERT and _LIBCPP_DEBUG_ASSERT

[ldionne]

Summary

I audited all uses of _LIBCPP_ASSERT to make sure that we only used it
for "basic assertions", i.e. assertions with constant-time conditions.
I also audited all uses of _LIBCPP_DEBUG_ASSERT to make sure we used it
only for debug-mode assertions, and in one case had to change for
_LIBCPP_ASSERT instead.

As a fly-by, I also changed a couple of tests against nullptr or 0 to
be more explicit.

After this patch, all uses of _LIBCPP_ASSERT should be with constant-time
conditions, and all uses of _LIBCPP_DEBUG_ASSERT should be with conditions
that we only want to check when the debug mode is enabled.

[philnik]

libcxx/include/list

2044

Since you are touching these lines anyways, could you replace the _VSTD uses?

[ldionne]

I'll update _VSTD upon committing to avoid hitting the CI again!

libcxx/include/__hash_table

 template <class _Tp, class _Hash, class _Equal, class _Alloc>
 typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::iterator
 __hash_table<_Tp, _Hash, _Equal, _Alloc>::erase(const_iterator __p)
 {
     __next_pointer __np = __p.__node_;
     _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
                          "unordered container erase(iterator) called with an iterator not"
                          " referring to this container");
-    _LIBCPP_DEBUG_ASSERT(__p != end(),
-                         "unordered container erase(iterator) called with a non-dereferenceable iterator");
+    _LIBCPP_ASSERT(__p != end(),
 #if _LIBCPP_DEBUG_LEVEL == 2
     iterator __r(__np, this);
 #else
     iterator __r(__np);
 #endif
     ++__r;
     remove(__p);
     return __r;

libcxx/include/__memory/construct_at.h

 
 #if _LIBCPP_STD_VER > 17
 
 template<class _Tp, class ..._Args, class = decltype(
     ::new (declval<void*>()) _Tp(declval<_Args>()...)
 )>
 _LIBCPP_HIDE_FROM_ABI
 constexpr _Tp* construct_at(_Tp* __location, _Args&& ...__args) {
-    _LIBCPP_ASSERT(__location, "null pointer given to construct_at");
+    _LIBCPP_ASSERT(__location != nullptr, "null pointer given to construct_at");
     return ::new (_VSTD::__voidify(*__location)) _Tp(_VSTD::forward<_Args>(__args)...);
 }
 
 #endif
 
 // destroy_at
 
 // The internal functions are available regardless of the language version (with the exception of the `__destroy_at`
 // taking an array).
 
 template <class _ForwardIterator>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
 _ForwardIterator __destroy(_ForwardIterator, _ForwardIterator);
 
 template <class _Tp, typename enable_if<!is_array<_Tp>::value, int>::type = 0>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
 void __destroy_at(_Tp* __loc) {
-    _LIBCPP_ASSERT(__loc, "null pointer given to destroy_at");
+    _LIBCPP_ASSERT(__loc != nullptr, "null pointer given to destroy_at");
     __loc->~_Tp();
 }
 
 #if _LIBCPP_STD_VER > 17
 template <class _Tp, typename enable_if<is_array<_Tp>::value, int>::type = 0>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
 void __destroy_at(_Tp* __loc) {
-    _LIBCPP_ASSERT(__loc, "null pointer given to destroy_at");
+    _LIBCPP_ASSERT(__loc != nullptr, "null pointer given to destroy_at");
     _VSTD::__destroy(_VSTD::begin(*__loc), _VSTD::end(*__loc));
 }
 #endif
 
 template <class _ForwardIterator>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
 _ForwardIterator __destroy(_ForwardIterator __first, _ForwardIterator __last) {
     for (; __first != __last; ++__first)

libcxx/include/list

#if _LIBCPP_DEBUG_LEVEL == 2
                 }
             }
             __db->unlock();
         }
 #endif
     }
 }
 
+template <class _Iterator>
+_LIBCPP_HIDE_FROM_ABI
+bool __iterator_in_range(_Iterator __first, _Iterator __last, _Iterator __it) {
+    for (_Iterator __p = __first; __p != __last; ++__p) {
+        if (__p == __it) {
+            return true;
+        }
+    }
+    return false;
+}
+
 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
-    if (this == _VSTD::addressof(__c))
-    {
-        for (const_iterator __i = __f; __i != __l; ++__i)
-            _LIBCPP_DEBUG_ASSERT(__i != __p,
-                "list::splice(iterator, list, iterator, iterator)"
-                " called with the first iterator within the range of the second and third iterators");
-    }
-#endif
+    _LIBCPP_DEBUG_ASSERT(this != _VSTD::addressof(__c) || !std::__iterator_in_range(__f, __l, __p),
+
     if (__f != __l)
     {
         __link_pointer __first = __f.__ptr_;
         --__l;
         __link_pointer __last = __l.__ptr_;
         if (this != _VSTD::addressof(__c))
         {
             size_type __s = _VSTD::distance(__f, __l) + 1;

libcxx/include/unordered_set

#endif // _LIBCPP_CXX03_LANG
     const_iterator cend()   const _NOEXCEPT {return __table_.end();}
 
 #ifndef _LIBCPP_CXX03_LANG
     template <class... _Args>
         _LIBCPP_INLINE_VISIBILITY
         pair<iterator, bool> emplace(_Args&&... __args)
             {return __table_.__emplace_unique(_VSTD::forward<_Args>(__args)...);}
     template <class... _Args>
-        _LIBCPP_INLINE_VISIBILITY
-#if _LIBCPP_DEBUG_LEVEL == 2
-        iterator emplace_hint(const_iterator __p, _Args&&... __args)
-        {
-            _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
-                "unordered_set::emplace_hint(const_iterator, args...) called with an iterator not"
-                " referring to this unordered_set");
-            return __table_.__emplace_unique(_VSTD::forward<_Args>(__args)...).first;
-        }
-#else
-        iterator emplace_hint(const_iterator, _Args&&... __args)
-            {return __table_.__emplace_unique(_VSTD::forward<_Args>(__args)...).first;}
-#endif
+    iterator emplace_hint(const_iterator __p, _Args&&... __args) {
+        _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
+        (void)__p;
 
     _LIBCPP_INLINE_VISIBILITY
     pair<iterator, bool> insert(value_type&& __x)
         {return __table_.__insert_unique(_VSTD::move(__x));}
     _LIBCPP_INLINE_VISIBILITY
-#if _LIBCPP_DEBUG_LEVEL == 2
-    iterator insert(const_iterator __p, value_type&& __x)
-        {
-            _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
-                "unordered_set::insert(const_iterator, value_type&&) called with an iterator not"
-                " referring to this unordered_set");
-            return insert(_VSTD::move(__x)).first;
-        }
-#else
-    iterator insert(const_iterator, value_type&& __x)
-        {return insert(_VSTD::move(__x)).first;}
-#endif
+    iterator insert(const_iterator __p, value_type&& __x) {
+        _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
+        (void)__p;
+
     _LIBCPP_INLINE_VISIBILITY
     void insert(initializer_list<value_type> __il)
         {insert(__il.begin(), __il.end());}
 #endif // _LIBCPP_CXX03_LANG
     _LIBCPP_INLINE_VISIBILITY
     pair<iterator, bool> insert(const value_type& __x)
         {return __table_.__insert_unique(__x);}
 
     _LIBCPP_INLINE_VISIBILITY
-#if _LIBCPP_DEBUG_LEVEL == 2
-    iterator insert(const_iterator __p, const value_type& __x)
-        {
-            _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
-                "unordered_set::insert(const_iterator, const value_type&) called with an iterator not"
-                " referring to this unordered_set");
-            return insert(__x).first;
-        }
-#else
-    iterator insert(const_iterator, const value_type& __x)
-        {return insert(__x).first;}
-#endif
+    iterator insert(const_iterator __p, const value_type& __x) {
+        _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__p)) == this,
+        (void)__p;
     template <class _InputIterator>
         _LIBCPP_INLINE_VISIBILITY
         void insert(_InputIterator __first, _InputIterator __last);
 
     _LIBCPP_INLINE_VISIBILITY
     iterator erase(const_iterator __p) {return __table_.erase(__p);}
     _LIBCPP_INLINE_VISIBILITY
     size_type erase(const key_type& __k) {return __table_.__erase_unique(__k);}

libcxx/src/filesystem/filesystem_common.h

#ifndef _LIBCPP_NO_EXCEPTIONS
     throw;
   }
 #endif // _LIBCPP_NO_EXCEPTIONS
   va_end(ap);
   return ret;
 }
 
 error_code capture_errno() {
-  _LIBCPP_ASSERT(errno, "Expected errno to be non-zero");
+  _LIBCPP_ASSERT(errno != 0, "Expected errno to be non-zero");
   return error_code(errno, generic_category());
 }
 
 #if defined(_LIBCPP_WIN32API)
 error_code make_windows_error(int err) {
   return make_error_code(__win_err_to_errc(err));
 }
 #endif