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

[Aditya Kumar via Phabricator via libcxx-commits]

hiraditya added a comment.

In D147741#4409500 <https://reviews.llvm.org/D147741#4409500>, @philnik wrote:

> 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?

Yeah, just did that in https://reviews.llvm.org/D152571 i'll also add a test case soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147741