[libc++] Simplify the char_traits specializations

[mclow.lists]

libcxx/include/__string/char_traits.h

226

Does this handle overlapping ranges? The old code does.

https://eel.is/c++draft/char.traits.require says it has to handle overlapping ranges.

[philnik]

libcxx/include/__string/char_traits.h

226

While std::copy_n doesn't officially support overlapping ranges our implementation forwards trivial types to __builtin_memmove, which does handle them properly.

[ldionne]

libcxx/include/__string/char_traits.h

226

Can you please add tests to cover that (or ensure we already do)?

Also, if we keep this, let's add a comment like // our implementation of std::copy_n handles overlapping ranges.

[EricWF]

libcxx/include/__string/char_traits.h

545–548

Don't we externally instantiate char_traits? This change would allow the compiler to instantiate and generate code in each TU for these functions. Maybe that's what we want, maybe that isn't. But we should consider it.

[philnik]

• GCC doesn't support __builtin_memmove during constant evaluation

[philnik]

libcxx/include/__string/char_traits.h

545–548

All the functions are marked _LIBCPP_INLINE_VISIBILITY? Even if we had an extern template they wouldn't be part of it.

[philnik]

Rebase

[ldionne]

LGTM with comments.

libcxx/include/__string/char_traits.h

181–183

226

My comment about testing for overlapping ranges still holds. It should be simple to add.

[philnik]

• Address comments
• Rename __move_constexpr to __char_traits_move

[philnik]

libcxx/include/__string/char_traits.h

181–183

This would break the implementation. This part of the code is still used if __move_constexpr isn't constant-evaluated. I probably should rename __move_constexpr to something better.

226

Sorry, I should have left a comment. That's already tested.

libcxx/include/__string/char_traits.h

 char_traits<_CharT>::assign(char_type* __s, size_t __n, char_type __a)
 {
     char_type* __r = __s;
     for (; __n; --__n, ++__s)
         assign(*__s, __a);
     return __r;
 }
 
-// constexpr versions of move/copy/assign.
-
 template <class _CharT>
 static inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-_CharT* __copy_constexpr(_CharT* __dest, const _CharT* __source, size_t __n) _NOEXCEPT
+_CharT* __char_traits_move(_CharT* __dest, const _CharT* __source, size_t __n) _NOEXCEPT
 {
-  _LIBCPP_ASSERT(__libcpp_is_constant_evaluated(), "__copy_constexpr() should always be constant evaluated");
-  _VSTD::copy_n(__source, __n, __dest);
-  return __dest;
-}
-
-template <class _CharT>
-static inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-_CharT* __move_constexpr(_CharT* __dest, const _CharT* __source, size_t __n) _NOEXCEPT
-{
-  _LIBCPP_ASSERT(__libcpp_is_constant_evaluated(), "__move_constexpr() should always be constant evaluated");
+#ifdef _LIBCPP_COMPILER_GCC
+  if (__libcpp_is_constant_evaluated()) {
     if (__n == 0)
       return __dest;
     _CharT* __allocation = new _CharT[__n];
-  _VSTD::__copy_constexpr(__allocation, __source, __n);
-  _VSTD::__copy_constexpr(__dest, static_cast<const _CharT*>(__allocation), __n);
+    std::copy_n(__source, __n, __allocation);
+    std::copy_n(static_cast<const _CharT*>(__allocation), __n, __dest);
     delete[] __allocation;
     return __dest;
   }
-
-template <class _CharT>
-static inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-_CharT* __assign_constexpr(_CharT* __s, size_t __n, _CharT __a) _NOEXCEPT
-{
-  _LIBCPP_ASSERT(__libcpp_is_constant_evaluated(), "__assign_constexpr() should always be constant evaluated");
-  _VSTD::fill_n(__s, __n, __a);
-  return __s;
+#endif
+  ::__builtin_memmove(__dest, __source, __n * sizeof(_CharT));
+  return __dest;
 }
 
 // char_traits<char>
 
 template <>
 struct _LIBCPP_TEMPLATE_VIS char_traits<char>
 {
     typedef char      char_type;
#ifdef _LIBCPP_COMPILER_GCC
         return __i;
       }
 #endif
       return __builtin_strlen(__s);
     }
 
     static _LIBCPP_CONSTEXPR_AFTER_CXX14
     const char_type* find(const char_type* __s, size_t __n, const char_type& __a) _NOEXCEPT;
+
     static inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-    char_type* move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT
-        {
-            return __libcpp_is_constant_evaluated()
-                       ? _VSTD::__move_constexpr(__s1, __s2, __n)
-                       : __n == 0 ? __s1 : (char_type*)_VSTD::memmove(__s1, __s2, __n);
-        }
+    char_type* move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
+        return std::__char_traits_move(__s1, __s2, __n);
+
     static inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-    char_type* copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT
-        {
-            if (!__libcpp_is_constant_evaluated()) {
-                _LIBCPP_ASSERT(__s2 < __s1 || __s2 >= __s1+__n, "char_traits::copy overlapped range");
-            }
-            return __libcpp_is_constant_evaluated()
-                       ? _VSTD::__copy_constexpr(__s1, __s2, __n)
-                       : __n == 0 ? __s1 : (char_type*)_VSTD::memcpy(__s1, __s2, __n);
-        }
+    char_type* copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
+        if (!__libcpp_is_constant_evaluated())
+        std::copy_n(__s2, __n, __s1);
+        return __s1;
+
     static inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-    char_type* assign(char_type* __s, size_t __n, char_type __a) _NOEXCEPT
-        {
-            return __libcpp_is_constant_evaluated()
-                       ? _VSTD::__assign_constexpr(__s, __n, __a)
-                       : __n == 0 ? __s : (char_type*)_VSTD::memset(__s, to_int_type(__a), __n);
-        }
+    char_type* assign(char_type* __s, size_t __n, char_type __a) _NOEXCEPT {
+        std::fill_n(__s, __n, __a);
+        return __s;
 
     static inline _LIBCPP_CONSTEXPR int_type  not_eof(int_type __c) _NOEXCEPT
         {return eq_int_type(__c, eof()) ? ~eof() : __c;}
     static inline _LIBCPP_CONSTEXPR char_type to_char_type(int_type __c) _NOEXCEPT
         {return char_type(__c);}
     static inline _LIBCPP_CONSTEXPR int_type to_int_type(char_type __c) _NOEXCEPT
         {return int_type((unsigned char)__c);}
     static inline _LIBCPP_CONSTEXPR bool eq_int_type(int_type __c1, int_type __c2) _NOEXCEPT
static inline _LIBCPP_CONSTEXPR bool lt(char_type __c1, char_type __c2) _NOEXCEPT
         {return __c1 < __c2;}
 
     static _LIBCPP_CONSTEXPR_AFTER_CXX14
     int compare(const char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT;
     static _LIBCPP_CONSTEXPR_AFTER_CXX14
     size_t length(const char_type* __s) _NOEXCEPT;
     static _LIBCPP_CONSTEXPR_AFTER_CXX14
     const char_type* find(const char_type* __s, size_t __n, const char_type& __a) _NOEXCEPT;
+
     static inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-    char_type* move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT
-        {
-            return __libcpp_is_constant_evaluated()
-                       ? _VSTD::__move_constexpr(__s1, __s2, __n)
-                       : __n == 0 ? __s1 : _VSTD::wmemmove(__s1, __s2, __n);
-        }
+    char_type* move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
+        return std::__char_traits_move(__s1, __s2, __n);
+
     static inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-    char_type* copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT
-        {
-            if (!__libcpp_is_constant_evaluated()) {
-                _LIBCPP_ASSERT(__s2 < __s1 || __s2 >= __s1+__n, "char_traits::copy overlapped range");
-            }
-            return __libcpp_is_constant_evaluated()
-                       ? _VSTD::__copy_constexpr(__s1, __s2, __n)
-                       : __n == 0 ? __s1 : _VSTD::wmemcpy(__s1, __s2, __n);
-        }
+    char_type* copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
+        if (!__libcpp_is_constant_evaluated())
+        std::copy_n(__s2, __n, __s1);
+        return __s1;
+
     static inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-    char_type* assign(char_type* __s, size_t __n, char_type __a) _NOEXCEPT
-        {
-            return __libcpp_is_constant_evaluated()
-                       ? _VSTD::__assign_constexpr(__s, __n, __a)
-                       : __n == 0 ? __s : _VSTD::wmemset(__s, __a, __n);
-        }
+    char_type* assign(char_type* __s, size_t __n, char_type __a) _NOEXCEPT {
+        std::fill_n(__s, __n, __a);
+        return __s;
+
     static inline _LIBCPP_CONSTEXPR int_type  not_eof(int_type __c) _NOEXCEPT
         {return eq_int_type(__c, eof()) ? ~eof() : __c;}
     static inline _LIBCPP_CONSTEXPR char_type to_char_type(int_type __c) _NOEXCEPT
         {return char_type(__c);}
     static inline _LIBCPP_CONSTEXPR int_type to_int_type(char_type __c) _NOEXCEPT
         {return int_type(__c);}
     static inline _LIBCPP_CONSTEXPR bool eq_int_type(int_type __c1, int_type __c2) _NOEXCEPT
         {return __c1 == __c2;}
struct _LIBCPP_TEMPLATE_VIS char_traits<char8_t>
 
     static constexpr
     size_t           length(const char_type* __s) _NOEXCEPT;
 
     _LIBCPP_INLINE_VISIBILITY static constexpr
     const char_type* find(const char_type* __s, size_t __n, const char_type& __a) _NOEXCEPT;
 
     static _LIBCPP_CONSTEXPR_AFTER_CXX17
-    char_type*       move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT
-        {
-            return __libcpp_is_constant_evaluated()
-                       ? _VSTD::__move_constexpr(__s1, __s2, __n)
-                       : __n == 0 ? __s1 : (char_type*)_VSTD::memmove(__s1, __s2, __n);
-        }
+    char_type*       move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
+        return std::__char_traits_move(__s1, __s2, __n);
 
     static _LIBCPP_CONSTEXPR_AFTER_CXX17
-    char_type*       copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT
-        {
-            if (!__libcpp_is_constant_evaluated()) {
-                _LIBCPP_ASSERT(__s2 < __s1 || __s2 >= __s1+__n, "char_traits::copy overlapped range");
-            }
-            return __libcpp_is_constant_evaluated()
-                       ? _VSTD::__copy_constexpr(__s1, __s2, __n)
-                       : __n == 0 ? __s1 : (char_type*)_VSTD::memcpy(__s1, __s2, __n);
-        }
+    char_type*       copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
+        if (!__libcpp_is_constant_evaluated())
+        std::copy_n(__s2, __n, __s1);
+        return __s1;
 
     static _LIBCPP_CONSTEXPR_AFTER_CXX17
-    char_type*       assign(char_type* __s, size_t __n, char_type __a) _NOEXCEPT
-        {
-            return __libcpp_is_constant_evaluated()
-                       ? _VSTD::__assign_constexpr(__s, __n, __a)
-                       : __n == 0 ? __s : (char_type*)_VSTD::memset(__s, to_int_type(__a), __n);
-        }
+    char_type*       assign(char_type* __s, size_t __n, char_type __a) _NOEXCEPT {
+        std::fill_n(__s, __n, __a);
+        return __s;
 
     static inline constexpr int_type  not_eof(int_type __c) noexcept
         {return eq_int_type(__c, eof()) ? ~eof() : __c;}
     static inline constexpr char_type to_char_type(int_type __c) noexcept
         {return char_type(__c);}
     static inline constexpr int_type to_int_type(char_type __c) noexcept
         {return int_type(__c);}
     static inline constexpr bool eq_int_type(int_type __c1, int_type __c2) noexcept
static inline _LIBCPP_CONSTEXPR bool lt(char_type __c1, char_type __c2) _NOEXCEPT
         {return __c1 < __c2;}
 
     _LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR_AFTER_CXX14
     int              compare(const char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT;
     _LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR_AFTER_CXX14
     size_t           length(const char_type* __s) _NOEXCEPT;
     _LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR_AFTER_CXX14
     const char_type* find(const char_type* __s, size_t __n, const char_type& __a) _NOEXCEPT;
+
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
-    static char_type*       move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT;
+    static char_type*       move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
+        return std::__char_traits_move(__s1, __s2, __n);
+    }
+
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
-    static char_type*       copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT;
+    static char_type*       copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
+        if (!__libcpp_is_constant_evaluated())
+            _LIBCPP_ASSERT(__s2 < __s1 || __s2 >= __s1+__n, "char_traits::copy overlapped range");
+        std::copy_n(__s2, __n, __s1);
+        return __s1;
+    }
+
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
-    static char_type*       assign(char_type* __s, size_t __n, char_type __a) _NOEXCEPT;
+    static char_type*       assign(char_type* __s, size_t __n, char_type __a) _NOEXCEPT {
+        std::fill_n(__s, __n, __a);
+        return __s;
+    }
 
     static inline _LIBCPP_CONSTEXPR int_type  not_eof(int_type __c) _NOEXCEPT
         {return eq_int_type(__c, eof()) ? ~eof() : __c;}
     static inline _LIBCPP_CONSTEXPR char_type to_char_type(int_type __c) _NOEXCEPT
         {return char_type(__c);}
     static inline _LIBCPP_CONSTEXPR int_type to_int_type(char_type __c) _NOEXCEPT
         {return int_type(__c);}
     static inline _LIBCPP_CONSTEXPR bool eq_int_type(int_type __c1, int_type __c2) _NOEXCEPT
char_traits<char16_t>::find(const char_type* __s, size_t __n, const char_type& __a) _NOEXCEPT
     {
         if (eq(*__s, __a))
             return __s;
         ++__s;
     }
     return nullptr;
 }
 
-inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-char16_t*
-char_traits<char16_t>::move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT
-{
-    if (__n == 0) return __s1;
-    char_type* __r = __s1;
-    if (__s1 < __s2)
-    {
-        for (; __n; --__n, ++__s1, ++__s2)
-            assign(*__s1, *__s2);
-    }
-    else if (__s2 < __s1)
-    {
-        __s1 += __n;
-        __s2 += __n;
-        for (; __n; --__n)
-            assign(*--__s1, *--__s2);
-    }
-    return __r;
-}
-
-inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-char16_t*
-char_traits<char16_t>::copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT
-{
-    if (!__libcpp_is_constant_evaluated()) {
-        _LIBCPP_ASSERT(__s2 < __s1 || __s2 >= __s1+__n, "char_traits::copy overlapped range");
-    }
-    char_type* __r = __s1;
-    for (; __n; --__n, ++__s1, ++__s2)
-        assign(*__s1, *__s2);
-    return __r;
-}
-
-inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-char16_t*
-char_traits<char16_t>::assign(char_type* __s, size_t __n, char_type __a) _NOEXCEPT
-{
-    char_type* __r = __s;
-    for (; __n; --__n, ++__s)
-        assign(*__s, __a);
-    return __r;
-}
-
 template <>
 struct _LIBCPP_TEMPLATE_VIS char_traits<char32_t>
 {
     typedef char32_t       char_type;
     typedef uint_least32_t int_type;
     typedef streamoff      off_type;
     typedef u32streampos   pos_type;
     typedef mbstate_t      state_type;
 
     static inline _LIBCPP_CONSTEXPR_AFTER_CXX14
     void assign(char_type& __c1, const char_type& __c2) _NOEXCEPT {__c1 = __c2;}
     static inline _LIBCPP_CONSTEXPR bool eq(char_type __c1, char_type __c2) _NOEXCEPT
         {return __c1 == __c2;}
     static inline _LIBCPP_CONSTEXPR bool lt(char_type __c1, char_type __c2) _NOEXCEPT
         {return __c1 < __c2;}
 
     _LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR_AFTER_CXX14
     int              compare(const char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT;
     _LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR_AFTER_CXX14
     size_t           length(const char_type* __s) _NOEXCEPT;
     _LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR_AFTER_CXX14
     const char_type* find(const char_type* __s, size_t __n, const char_type& __a) _NOEXCEPT;
+
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
-    static char_type*       move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT;
+    static char_type*       move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
+        return std::__char_traits_move(__s1, __s2, __n);
+    }
+
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
-    static char_type*       copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT;
+    static char_type*       copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
+        std::copy_n(__s2, __n, __s1);
+        return __s1;
+    }
+
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
-    static char_type*       assign(char_type* __s, size_t __n, char_type __a) _NOEXCEPT;
+    static char_type*       assign(char_type* __s, size_t __n, char_type __a) _NOEXCEPT {
+        std::fill_n(__s, __n, __a);
+        return __s;
+    }
 
     static inline _LIBCPP_CONSTEXPR int_type  not_eof(int_type __c) _NOEXCEPT
         {return eq_int_type(__c, eof()) ? ~eof() : __c;}
     static inline _LIBCPP_CONSTEXPR char_type to_char_type(int_type __c) _NOEXCEPT
         {return char_type(__c);}
     static inline _LIBCPP_CONSTEXPR int_type to_int_type(char_type __c) _NOEXCEPT
         {return int_type(__c);}
     static inline _LIBCPP_CONSTEXPR bool eq_int_type(int_type __c1, int_type __c2) _NOEXCEPT
char_traits<char32_t>::find(const char_type* __s, size_t __n, const char_type& __a) _NOEXCEPT
     {
         if (eq(*__s, __a))
             return __s;
         ++__s;
     }
     return nullptr;
 }
 
-inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-char32_t*
-char_traits<char32_t>::move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT
-{
-    if (__n == 0) return __s1;
-    char_type* __r = __s1;
-    if (__s1 < __s2)
-    {
-        for (; __n; --__n, ++__s1, ++__s2)
-            assign(*__s1, *__s2);
-    }
-    else if (__s2 < __s1)
-    {
-        __s1 += __n;
-        __s2 += __n;
-        for (; __n; --__n)
-            assign(*--__s1, *--__s2);
-    }
-    return __r;
-}
-
-inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-char32_t*
-char_traits<char32_t>::copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT
-{
-    if (!__libcpp_is_constant_evaluated()) {
-        _LIBCPP_ASSERT(__s2 < __s1 || __s2 >= __s1+__n, "char_traits::copy overlapped range");
-    }
-    char_type* __r = __s1;
-    for (; __n; --__n, ++__s1, ++__s2)
-        assign(*__s1, *__s2);
-    return __r;
-}
-
-inline _LIBCPP_CONSTEXPR_AFTER_CXX17
-char32_t*
-char_traits<char32_t>::assign(char_type* __s, size_t __n, char_type __a) _NOEXCEPT
-{
-    char_type* __r = __s;
-    for (; __n; --__n, ++__s)
-        assign(*__s, __a);
-    return __r;
-}
-
 #endif // _LIBCPP_HAS_NO_UNICODE_CHARS
 
 // helper fns for basic_string and string_view
 
 // __str_find
 template<class _CharT, class _SizeT, class _Traits, _SizeT __npos>
 inline _SizeT _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
 __str_find(const _CharT *__p, _SizeT __sz,