[libcxx-commits] [PATCH] D61761: P1144 "Trivially relocatable" (1/3): is_trivially_relocatable, relocate_at, and uninitialized_relocate

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Sun May 12 19:05:40 PDT 2019


Quuxplusone added inline comments.


================
Comment at: include/memory:3464
+_Tp *__relocate_at_impl(_Tp *__source, _Tp *__dest, false_type) {
+    ::new (static_cast<void*>(__dest)) _Tp(_VSTD::move(*__dest));
+    __source->~_Tp();
----------------
zoecarver wrote:
> Are you intentionally not doing anything with the result of `new`? Also, I am not sure you need to `static_cast` to `void*`.
> 
> Also, also, I think you want to move `__source` not `__dest` (because I am pretty sure `__dest` is where you want to relocate the new thing and `__source` is where you want to get it).
Yikes! That was a pretty bad bug. Thanks.


================
Comment at: include/memory:3518
+_ForwardIt2
+__uninitialized_relocate_impl(_ForwardIt1 __first, _ForwardIt1 __last,
+                              _ForwardIt2 __result,
----------------
zoecarver wrote:
> I don't think this overload is necessary in addition to the below one.
I think this overload would never really be useful in practice, but since it does something different from the below one (element-wise memcpy in a loop, instead of one big memcpy), I figured I should implement it.

However, since P1144R3 adds `relocate_at`, I can refactor this overload and the one //above// into a single overload that does `relocate_at`-in-a-loop! Will do.

In the process of rewriting this code, I realized that whereas `uninitialized_relocate` copies `uninitialized_move` in not requiring the two iterator types to have the same `value_type`, `relocate_at` requires the exact same type `_Tp *` for both `__source` and `__dest`. I have added an internal version `__relocate_at2(_Sp *__source, _Dp *__dest)`, and am ambivalently considering adding it to the proposal P1144R4. That would require some kind of change to the word "homogeneous" in P1144R3, though. Feel free to comment on this idea here and/or by email.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D61761





More information about the libcxx-commits mailing list