[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone abandoned this revision. Quuxplusone added a comment. Abandoning, as this has been done in d9a4f936d05 . Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49317/new/ https://reviews.

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone added a comment. In https://reviews.llvm.org/D49317#1180852, @ldionne wrote: > After thinking about this for some more, I'm not sure this patch is worth > doing in its current form. The minimal patch for allowing specializations of > `allocator_traits` would be: > > 1. move the `__m

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. After thinking about this for some more, I'm not sure this patch is worth doing in its current form. The minimal patch for allowing specializations of `allocator_traits` would be:

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone added a comment. @mclow.lists: Well, anyway, is this pure refactoring acceptable, or would you prefer to keep these helpers inside `allocator_traits` until a better solution to constexpr-memcpy can be invented? Repository: rCXX libc++ https://reviews.llvm.org/D49317 _

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

mclow.lists added a comment. > I don't think it makes sense to pessimize existing (non-constexpr) users in > C++03-through-C++17 just because someone hypothetically might in > C++2a-or-later want to mutate a std::vector in a constexpr context. That's not the right (implied) question. The corre

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone added inline comments. Comment at: include/vector:318 +} +} + mclow.lists wrote: > Quuxplusone wrote: > > Marshall writes: > > > Instead, we should be writing simple loops that the compiler can optimize > > > into a call to memcpy if it chooses.

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

mclow.lists added inline comments. Comment at: include/vector:318 +} +} + Quuxplusone wrote: > Marshall writes: > > Instead, we should be writing simple loops that the compiler can optimize > > into a call to memcpy if it chooses. Having calls to memcpy in t

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone added inline comments. Comment at: include/vector:318 +} +} + Marshall writes: > Instead, we should be writing simple loops that the compiler can optimize > into a call to memcpy if it chooses. Having calls to memcpy in the code paths > makes it

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

ldionne added a comment. In https://reviews.llvm.org/D49317#1180200, @mclow.lists wrote: > I am not in favor of this patch. > > I'm in favor of fixing the problem that Arthur has described, but not like > this, for the following reasons: > > - Conceptually, these are (similar to) "Allocator-base

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

mclow.lists added a comment. I am not in favor of this patch. I'm in favor of fixing the problem that Arthur has described, but not like this, for the following reasons: - Conceptually, these are (similar to) "Allocator-based versions of the algorithms proposed in P0040

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

ldionne added a comment. In https://reviews.llvm.org/D49317#1178767, @Quuxplusone wrote: > @ldionne: I don't know if your "LGTM" is necessarily sufficient to commit > this or not; but either way, I don't have commit privs, so could I ask you > (or someone else) to commit this on my behalf? Than

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone added a comment. @ldionne: I don't know if your "LGTM" is necessarily sufficient to commit this or not; but either way, I don't have commit privs, so could I ask you (or someone else) to commit this on my behalf? Thanks! Repository: rCXX libc++ https://reviews.llvm.org/D49317

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM Comment at: include/vector:296 +_LIBCPP_INLINE_VISIBILITY +inline void +__copy_construct_forward(_Alloc& __a, _Iter __begin1, _Iter __end1, Quuxpluson

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone marked an inline comment as done. Quuxplusone added inline comments. Comment at: include/vector:296 +_LIBCPP_INLINE_VISIBILITY +inline void +__copy_construct_forward(_Alloc& __a, _Iter __begin1, _Iter __end1, ldionne wrote: > Do you really need `inlin

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

ldionne added a comment. It would be nice if all the TMP required to determine whether to call `__move_construct_forward(..., true_type)` or `__move_construct_forward(..., false_type)` was done in `__move_construct_forward` itself (or a helper). This way, callers wouldn't have to do it themselv

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

vsapsai added inline comments. Comment at: include/vector:300 +{ +using _Alloc_traits = allocator_traits<_Alloc>; +for (; __begin1 != __end1; ++__begin1, (void)++__begin2) Quuxplusone wrote: > vsapsai wrote: > > Have you checked why `using` is accepted in

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone added inline comments. Comment at: include/vector:298 +__copy_construct_forward(_Alloc& __a, _Iter __begin1, _Iter __end1, + _Ptr& __begin2, _CopyViaMemcpy) +{ vsapsai wrote: > Why does this function use `_CopyViaMemcpy` and no

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone updated this revision to Diff 155796. Quuxplusone marked 4 inline comments as done. Quuxplusone added a comment. Address @vsapsai's review comments. Repository: rCXX libc++ https://reviews.llvm.org/D49317 Files: include/memory include/vector test/libcxx/containers/sequences

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

vsapsai added a comment. My review is incomplete, especially I cannot say with confidence if the proposed change is entirely free from unintended consequences that might break code not covered by the test suite. So other reviewers are welcome to chime in. Comment at: include/

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone updated this revision to Diff 155473. Quuxplusone added a comment. Move the functions from `` to ``, since that's their only caller. Uniform treatment of the pointer/iterator parameters; discover that the difference between "copy_forward" and "copy_range_forward" was that the former

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

Quuxplusone created this revision. Quuxplusone added reviewers: EricWF, mclow.lists, erik.pilkington, vsapsai. Herald added subscribers: cfe-commits, ldionne, christof. Inspired by Volodymyr's work on https://reviews.llvm.org/D48753, I've taken it upon myself to refactor the static member functio