[libcxx-commits] [PATCH] D151953: [libc++] Fix std::copy and std::move for ranges with potentially overlapping tail padding

[Alexander Kornienko via Phabricator via libcxx-commits]

alexfh added inline comments.


================
Comment at: libcxx/include/__string/constexpr_c_functions.h:149
+      for (; __count > 0; --__count)
+        __dest[__count - 1] = __src[__count - 1];
+    } else {
----------------
ldionne wrote:
> alexfh wrote:
> > Some of our (and third-party) code started triggering an error related to this line after this commit. It turns out, std::vector<T>::erase() doesn't compile now, if T is move-only: https://gcc.godbolt.org/z/Txnc66M97
> > 
> > Is this intended?
> Hmm, thanks for the heads up. `vector<T>::erase(...)` should only require that `T` is move assignable. In fact, it looks like this is being triggered in the `std::move` algorithm -- we probably don't have tests with move-only types in `std::move`, which is pretty embarrassing. @philnik it looks like we need to check additional constraints before calling `__constexpr_memmove`. Using `std::move(__src[__count - 1])` here wouldn't be an option since the semantics wouldn't match those of `memmove` IMO.
If this is not intended, I'd ask for a revert, since the impact of this is rather large.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151953/new/

https://reviews.llvm.org/D151953