[libc++] Disable _LIBCPP_DEBUG_ASSERT during constant evaluation

[philnik]

Summary

Disable _LIBCPP_DEBUG_ASSERT and debug iterators in <string> during constant evaluation

[philnik]

Added <__debug> changes

[philnik]

• Forgot an !

[• Quuxplusone]

libcxx/include/__debug

37

This feels like a change in meaning, from "Assert in debug mode" to "Assert specifically concerned with debug iterators." It turns out that this macro has pretty much always been used with the latter meaning, though, so even though a better name might be _LIBCPP_DEBUG_ITERATOR_ASSERT, I think this is fine.

However, the one place in libc++ that doesn't use the macro with the latter meaning, needs to be updated to use _LIBCPP_ASSERT instead.

../libcxx/include/bit:    _LIBCPP_DEBUG_ASSERT(__libcpp_is_constant_evaluated() || __n != numeric_limits<_Tp>::digits, "Bad input to bit_ceil");

I think this should just be _LIBCPP_ASSERT(__n != numeric_limits<_Tp>::digits, "Bad input to bit_ceil"); and if that's not right then investigation is needed as to why that's not right.

Then, please update the summary/commit-message to talk about the semantic change here, which I think is more important than the <string> part. Something like, "_LIBCPP_DEBUG_ASSERT is now used strictly for debug iterators, and disabled during constant evaluation." Observe that this is a change for both <__hash_table> (since ages ago) and <string> (since D115765).

libcxx/include/string

828–832

Somewhere in the past year I'd suggested something like

_LIBCPP_IF_DEBUG_ITERATORS(
  __get_db()->__insert_c(this);
)

which could simplify this repeated pattern. Thoughts on reviving that idea?

2990

Stray blank line here

[philnik]

libcxx/include/__debug

37

There are not that many usages of _LIBCPP_DEBUG_ASSERT. Maybe it would be good to change the meaning and rename it to _LIBCPP_DEBUG_ITERATOR_ASSERT in another patch?

libcxx/include/string

828–832

I do quite like the idea of _LIBCPP_IF_DEBUG_ITERATORS. Should I make this a more general patch to simplify the debug iterators across the repo, or would that be too much?

[ldionne]

libcxx/include/__debug

37

IMO this is fine, but needs to be called out in the commit message. Basically, the important thing this patch is doing is that we're not going to check debug-only assertions inside constant expressions anymore. I think that's acceptable, since those assertions are meant to catch things like UB, and the compiler will do that for us during constexpr evaluation.

libcxx/include/string

828–832

I am leery of adding a bunch of macros as "shortcuts" for these things. I think the current approach is fine.

3497

After your patch, this is a mix of 4 space and 2 space indentation. Please consistently use 4 space in this file, since that's what is done for the other functions in that file.

This applies to all the other functions you touched AFAICT.

[philnik]

• Changed indentation
• Replaced _LIBCPP_DEBUG_ASSET with _LIBCPP_ASSERT in <bit>

[philnik]

• More indentation changes

[philnik]

• And even more

[philnik]

Why have non-libc++ builds been triggered? Can I land with them failing? They don't seem to fail because of this patch.

[ldionne]

In D115788#3197640, @philnik wrote:

Why have non-libc++ builds been triggered?

I don't know. @goncharov ?

Can I land with them failing? They don't seem to fail because of this patch.

Yes, you can land if the libc++ CI is green, but thanks for being diligent and asking.

[philnik]

Rebased

libcxx/include/__debug

 
 #if _LIBCPP_DEBUG_LEVEL == 0
 #   define _LIBCPP_DEBUG_ASSERT(x, m) ((void)0)
 #   define _LIBCPP_ASSERT_IMPL(x, m) ((void)0)
 #elif _LIBCPP_DEBUG_LEVEL == 1
 #   define _LIBCPP_DEBUG_ASSERT(x, m) ((void)0)
 #   define _LIBCPP_ASSERT_IMPL(x, m) ((x) ? (void)0 : _VSTD::__libcpp_debug_function(_VSTD::__libcpp_debug_info(__FILE__, __LINE__, #x, m)))
 #elif _LIBCPP_DEBUG_LEVEL == 2
-#   define _LIBCPP_DEBUG_ASSERT(x, m) _LIBCPP_ASSERT(x, m)
+#   define _LIBCPP_DEBUG_ASSERT(x, m) _LIBCPP_ASSERT(__libcpp_is_constant_evaluated() || (x), m)
 #   define _LIBCPP_ASSERT_IMPL(x, m) ((x) ? (void)0 : _VSTD::__libcpp_debug_function(_VSTD::__libcpp_debug_info(__FILE__, __LINE__, #x, m)))
 #else
 #   error _LIBCPP_DEBUG_LEVEL must be one of 0, 1, 2
 #endif
 
 #if !defined(_LIBCPP_ASSERT)
 #   define _LIBCPP_ASSERT(x, m) _LIBCPP_ASSERT_IMPL(x, m)
 #endif

libcxx/include/bit

 
 template <class _Tp>
 _LIBCPP_INLINE_VISIBILITY constexpr
 enable_if_t<__libcpp_is_unsigned_integer<_Tp>::value, _Tp>
 bit_ceil(_Tp __t) noexcept
 {
     if (__t < 2) return 1;
     const unsigned __n = numeric_limits<_Tp>::digits - countl_zero((_Tp)(__t - 1u));
-    _LIBCPP_DEBUG_ASSERT(__libcpp_is_constant_evaluated() || __n != numeric_limits<_Tp>::digits, "Bad input to bit_ceil");
+    _LIBCPP_ASSERT(__n != numeric_limits<_Tp>::digits, "Bad input to bit_ceil");
 
     if constexpr (sizeof(_Tp) >= sizeof(unsigned))
         return _Tp{1} << __n;
     else
     {
         const unsigned __extra = numeric_limits<unsigned>::digits  - numeric_limits<_Tp>::digits;
         const unsigned __retVal = 1u << (__n + __extra);
         return (_Tp) (__retVal >> __extra);

libcxx/include/string

#endif
     _LIBCPP_INLINE_VISIBILITY
     basic_string(basic_string&& __str, const allocator_type& __a);
 #endif // _LIBCPP_CXX03_LANG
 
     template <class = __enable_if_t<__is_allocator<_Allocator>::value, nullptr_t> >
     _LIBCPP_INLINE_VISIBILITY
     basic_string(const _CharT* __s) : __r_(__default_init_tag(), __default_init_tag()) {
       _LIBCPP_ASSERT(__s != nullptr, "basic_string(const char*) detected nullptr");
       __init(__s, traits_type::length(__s));
-#   if _LIBCPP_DEBUG_LEVEL == 2
+#if _LIBCPP_DEBUG_LEVEL == 2
+        if (!__libcpp_is_constant_evaluated())
             __get_db()->__insert_c(this);
-#   endif
+#endif
     }
 
     template <class = __enable_if_t<__is_allocator<_Allocator>::value, nullptr_t> >
         _LIBCPP_INLINE_VISIBILITY
         basic_string(const _CharT* __s, const _Allocator& __a);
 
 #if _LIBCPP_STD_VER > 20
     basic_string(nullptr_t) = delete;

 #endif
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 void
 basic_string<_CharT, _Traits, _Allocator>::__invalidate_all_iterators()
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__invalidate_all(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 void
 basic_string<_CharT, _Traits, _Allocator>::__invalidate_iterators_past(size_type __pos)
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated()) {
         __c_node* __c = __get_db()->__find_c_and_lock(this);
         if (__c)
         {
             const_pointer __new_last = __get_pointer() + __pos;
             for (__i_node** __p = __c->end_; __p != __c->beg_; )
             {
                 --__p;
                 const_iterator* __i = static_cast<const_iterator*>((*__p)->__i_);
                 if (__i->base() > __new_last)
                 {
                     (*__p)->__c_ = nullptr;
                     if (--__c->end_ != __p)
                         _VSTD::memmove(__p, __p+1, (__c->end_ - __p)*sizeof(__i_node*));
                 }
             }
             __get_db()->unlock();
         }
+    }
 #else
     (void)__pos;
 #endif // _LIBCPP_DEBUG_LEVEL == 2
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 basic_string<_CharT, _Traits, _Allocator>::basic_string()
     _NOEXCEPT_(is_nothrow_default_constructible<allocator_type>::value)
      : __r_(__default_init_tag(), __default_init_tag())
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
     __zero();
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const allocator_type& __a)
 #if _LIBCPP_STD_VER <= 14
         _NOEXCEPT_(is_nothrow_copy_constructible<allocator_type>::value)
 #else
         _NOEXCEPT
 #endif
 : __r_(__default_init_tag(), __a)
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
     __zero();
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 void basic_string<_CharT, _Traits, _Allocator>::__init(const value_type* __s,
                                                        size_type __sz,
                                                        size_type __reserve)

 template <class _CharT, class _Traits, class _Allocator>
 template <class>
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const _CharT* __s, const _Allocator& __a)
     : __r_(__default_init_tag(), __a)
 {
     _LIBCPP_ASSERT(__s != nullptr, "basic_string(const char*, allocator) detected nullptr");
     __init(__s, traits_type::length(__s));
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const _CharT* __s, size_type __n)
      : __r_(__default_init_tag(), __default_init_tag())
 {
       _LIBCPP_ASSERT(__n == 0 || __s != nullptr, "basic_string(const char*, n) detected nullptr");
       __init(__s, __n);
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const _CharT* __s, size_type __n, const _Allocator& __a)
     : __r_(__default_init_tag(), __a)
 {
     _LIBCPP_ASSERT(__n == 0 || __s != nullptr, "basic_string(const char*, n, allocator) detected nullptr");
     __init(__s, __n);
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const basic_string& __str)
     : __r_(__default_init_tag(), __alloc_traits::select_on_container_copy_construction(__str.__alloc()))
 {
     if (!__str.__is_long())
         __r_.first().__r = __str.__r_.first().__r;
     else
         __init_copy_ctor_external(_VSTD::__to_address(__str.__get_long_pointer()),
                                   __str.__get_long_size());
 
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 basic_string<_CharT, _Traits, _Allocator>::basic_string(
     const basic_string& __str, const allocator_type& __a)
     : __r_(__default_init_tag(), __a)
 {
     if (!__str.__is_long())
         __r_.first().__r = __str.__r_.first().__r;
     else
         __init_copy_ctor_external(_VSTD::__to_address(__str.__get_long_pointer()),
                                   __str.__get_long_size());
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 void basic_string<_CharT, _Traits, _Allocator>::__init_copy_ctor_external(
     const value_type* __s, size_type __sz) {
   pointer __p;
   if (__sz < __min_cap) {
#if _LIBCPP_STD_VER <= 14
         _NOEXCEPT_(is_nothrow_move_constructible<allocator_type>::value)
 #else
         _NOEXCEPT
 #endif
     : __r_(_VSTD::move(__str.__r_))
 {
     __str.__zero();
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated()) {
         __get_db()->__insert_c(this);
         if (__is_long())
             __get_db()->swap(this, &__str);
+    }
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 basic_string<_CharT, _Traits, _Allocator>::basic_string(basic_string&& __str, const allocator_type& __a)
     : __r_(__default_init_tag(), __a)
 {
     if (__str.__is_long() && __a != __str.__alloc()) // copy, not move
         __init(_VSTD::__to_address(__str.__get_long_pointer()), __str.__get_long_size());
     else
     {
         __r_.first().__r = __str.__r_.first().__r;
         __str.__zero();
     }
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated()) {
         __get_db()->__insert_c(this);
         if (__is_long())
             __get_db()->swap(this, &__str);
+    }
 #endif
 }
 
 #endif // _LIBCPP_CXX03_LANG
 
 template <class _CharT, class _Traits, class _Allocator>
 void
 basic_string<_CharT, _Traits, _Allocator>::__init(size_type __n, value_type __c)

 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 basic_string<_CharT, _Traits, _Allocator>::basic_string(size_type __n, _CharT __c)
      : __r_(__default_init_tag(), __default_init_tag())
 {
     __init(__n, __c);
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 template <class>
 basic_string<_CharT, _Traits, _Allocator>::basic_string(size_type __n, _CharT __c, const _Allocator& __a)
     : __r_(__default_init_tag(), __a)
 {
     __init(__n, __c);
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const basic_string& __str,
                                                         size_type __pos, size_type __n,
                                                         const _Allocator& __a)
     : __r_(__default_init_tag(), __a)
 {
     size_type __str_sz = __str.size();
     if (__pos > __str_sz)
         this->__throw_out_of_range();
     __init(__str.data() + __pos, _VSTD::min(__n, __str_sz - __pos));
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const basic_string& __str, size_type __pos,
                                                         const _Allocator& __a)
     : __r_(__default_init_tag(), __a)
 {
     size_type __str_sz = __str.size();
     if (__pos > __str_sz)
         this->__throw_out_of_range();
     __init(__str.data() + __pos, __str_sz - __pos);
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 template <class _Tp, class>
 basic_string<_CharT, _Traits, _Allocator>::basic_string(
              const _Tp& __t, size_type __pos, size_type __n, const allocator_type& __a)
     : __r_(__default_init_tag(), __a)
 {
     __self_view __sv0 = __t;
     __self_view __sv = __sv0.substr(__pos, __n);
     __init(__sv.data(), __sv.size());
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 template <class _Tp, class>
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const _Tp & __t)
      : __r_(__default_init_tag(), __default_init_tag())
 {
     __self_view __sv = __t;
     __init(__sv.data(), __sv.size());
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 template <class _Tp, class>
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const _Tp & __t, const _Allocator& __a)
     : __r_(__default_init_tag(), __a)
 {
     __self_view __sv = __t;
     __init(__sv.data(), __sv.size());
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 template <class _InputIterator>
 __enable_if_t
 <
     __is_exactly_cpp17_input_iterator<_InputIterator>::value

 template <class _CharT, class _Traits, class _Allocator>
 template<class _InputIterator, class>
 inline
 basic_string<_CharT, _Traits, _Allocator>::basic_string(_InputIterator __first, _InputIterator __last)
      : __r_(__default_init_tag(), __default_init_tag())
 {
     __init(__first, __last);
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 template<class _InputIterator, class>
 inline
 basic_string<_CharT, _Traits, _Allocator>::basic_string(_InputIterator __first, _InputIterator __last,
                                                         const allocator_type& __a)
     : __r_(__default_init_tag(), __a)
 {
     __init(__first, __last);
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 #ifndef _LIBCPP_CXX03_LANG
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 basic_string<_CharT, _Traits, _Allocator>::basic_string(
     initializer_list<_CharT> __il)
      : __r_(__default_init_tag(), __default_init_tag())
 {
     __init(__il.begin(), __il.end());
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 
 basic_string<_CharT, _Traits, _Allocator>::basic_string(
     initializer_list<_CharT> __il, const _Allocator& __a)
     : __r_(__default_init_tag(), __a)
 {
     __init(__il.begin(), __il.end());
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__insert_c(this);
 #endif
 }
 
 #endif // _LIBCPP_CXX03_LANG
 
 template <class _CharT, class _Traits, class _Allocator>
 basic_string<_CharT, _Traits, _Allocator>::~basic_string()
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated())
         __get_db()->__erase_c(this);
 #endif
     if (__is_long())
         __alloc_traits::deallocate(__alloc(), __get_long_pointer(), __get_long_cap());
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 void
 basic_string<_CharT, _Traits, _Allocator>::__grow_by_and_replace
basic_string<_CharT, _Traits, _Allocator>::insert(const_iterator __pos, value_type __c)
     return begin() + static_cast<difference_type>(__ip);
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 typename basic_string<_CharT, _Traits, _Allocator>::iterator
 basic_string<_CharT, _Traits, _Allocator>::insert(const_iterator __pos, size_type __n, value_type __c)
 {
   _LIBCPP_DEBUG_ASSERT(__get_const_db()->__find_c_from_i(&__pos) == this,
                        "string::insert(iterator, n, value) called with an iterator not"
                        " referring to this string");
   difference_type __p = __pos - begin();
   insert(static_cast<size_type>(__p), __n, __c);
   return begin() + __p;
 }
 
 // replace

 #if _LIBCPP_STD_VER >= 14
         _NOEXCEPT
 #else
         _NOEXCEPT_(!__alloc_traits::propagate_on_container_swap::value ||
                     __is_nothrow_swappable<allocator_type>::value)
 #endif
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
+    if (!__libcpp_is_constant_evaluated()) {
         if (!__is_long())
             __get_db()->__invalidate_all(this);
         if (!__str.__is_long())
             __get_db()->__invalidate_all(&__str);
         __get_db()->swap(this, &__str);
+    }
 #endif
     _LIBCPP_ASSERT(
         __alloc_traits::propagate_on_container_swap::value ||
         __alloc_traits::is_always_equal::value ||
         __alloc() == __str.__alloc(), "swapping non-equal allocators");
     _VSTD::swap(__r_.first(), __str.__r_.first());
     _VSTD::__swap_allocator(__alloc(), __str.__alloc());
 }