[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 inline comments.


================
Comment at: libcxx/include/__memory/uninitialized_algorithms.h:602
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Up*
+__uninitialized_allocator_copy_impl(_Alloc&, const _Type* __first1, const _Type* __last1, _Up* __first2) {
+  // TODO: Remove the const_cast once we drop support for std::allocator<T const>
----------------
hiraditya wrote:
> this version gets called for test cases like `vector<const int>`
But the other overload works for that just as well, no?


================
Comment at: libcxx/include/__memory/uninitialized_algorithms.h:607
+  } else {
+    return std::copy(__first1, __last1, const_cast<_RawType*>(__first2));
+  }
----------------
Mordante wrote:
> hiraditya wrote:
> > Mordante wrote:
> > > LLVM style is not `else` after a `return`, here and other places in this patch. Note the `if constexpr` does get the `else` to avoid generating unneeded code by the compiler.
> > This code is directly copied from the function above. Not sure what is the guidance for libcxx is, cc: @ldionne 
> We have more code that does not conform to our policy, but for new code we use our policy.
I'm not aware that we use this particular policy in our new code. Anyways, this code is redundant anyways, and shouldn't have to be added (unless I'm missing something).


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

https://reviews.llvm.org/D147741