[libcxx-commits] [PATCH] D127159: [libc++] Simplify the char_traits specializations

Nikolas Klauser via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jun 13 10:38:16 PDT 2022

philnik added inline comments.

Comment at: libcxx/include/__string/char_traits.h:181-183
+  ::__builtin_memmove(__dest, __source, __n * sizeof(_CharT));
   return __dest;
ldionne wrote:
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.

Comment at: libcxx/include/__string/char_traits.h:207
+    char_type* move(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
+        std::copy_n(__s2, __n, __s1);
+        return __s1;
ldionne wrote:
> ldionne wrote:
> > philnik wrote:
> > > mclow.lists wrote:
> > > > Does this handle overlapping ranges? The old code does.
> > > > 
> > > > https://eel.is/c++draft/char.traits.require says it has to handle overlapping ranges.
> > > While `std::copy_n` doesn't officially support overlapping ranges our implementation forwards trivial types to `__builtin_memmove`, which does handle them properly.
> > 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`.
> My comment about testing for overlapping ranges still holds. It should be simple to add.
Sorry, I should have left a comment. That's already tested.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list