[libcxx-commits] [PATCH] D147741: [libc++, std::vector] call the optimized version of __uninitialized_allocator_copy for trivial types

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D147741#4409483 <https://reviews.llvm.org/D147741#4409483>, @hiraditya wrote:

> In D147741#4407956 <https://reviews.llvm.org/D147741#4407956>, @hans wrote:
>
>> We're hitting build failures after this change. Here's a reduced version:
>>
>>   $ cat /tmp/a.cc
>>   #include <vector>
>>   
>>   void f(volatile int *p, int n) {
>>     std::vector<int> v(p, p + n);
>>   }
>>   
>>   /work/llvm-project/build2/bin/../include/c++/v1/__memory/uninitialized_algorithms.h:589:12: error: cannot initialize return object of type 'int *' with an rvalue of type 'volatile int *'
>>       return std::copy(__first1, __last1, const_cast<_RawTypeIn*>(__first2));
>
> This looks like a legit  bug, calling std::copy (which gets optimized to memmove) on `volatile` source type seems incorrect. cc: @philnik for clarification.

I don't think `std::copy` gets optimized to a `memmove` when it gets a volatile pointer. The problem is that our return type is `OutT*`, but `std::copy` gets `remove_const_t<InT>*`. I think the simplest solution would be to check for `is_same<__remove_const_t<_In>, __remove_const_t<_Out>` instead of `is_same<__remove_cv_t<_In>, __remove_cv_t<_Out>`. @hiraditya would you fix this in a quick follow-up patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147741