[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.
Volodymyr Sapsai via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 17 11:18:38 PDT 2018
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 C++03 mode? The tests are passing but I expected a compiler warning and didn't investigate further.
> I talked with Glen Fernandes about this on Slack the other day. I think the deal is that `make check-cxx` runs only the `-std=c++2a` tests, and if you want `-std=c++03` you have to run them manually with `llvm-lit --param=-std=c++03 -sv path/to/tests`. Which of course I didn't do. :)
> If there's a more foolproof way of automatically testing libc++ in *all* compiler modes, I'd like to know about it.
>
> Fixed!
The test suite didn't detect anything even in C++03 mode because of [`-Wno-c++11-extensions`](https://github.com/llvm-mirror/libcxx/blob/ffbb91bb640b1b0425a91aa70e2a6a2e0f7244e0/utils/libcxx/test/config.py#L922). Thanks for using typedef instead.
================
Comment at: include/vector:542
+template<class _Tp, class _Allocator>
+struct __vector_copy_via_memcpy : integral_constant<bool,
+ (is_same<_Allocator, allocator<_Tp> >::value || !__has_construct<_Allocator, _Tp*, _Tp>::value) &&
----------------
Quuxplusone wrote:
> vsapsai wrote:
> > I think the name `__vector_constructable_via_memcpy` better reflects the meaning. It detects cases when individual element construction can be safely replaced with memcpy, so it feels more about construct than about copy. And `copy_via_memcpy` is too imperative as for me, not really conveying it has boolean semantic.
> > `copy_via_memcpy` is too imperative for me
>
> I see your point. However, for background... in my other branch, this trait is joined by two companions:
> ```
> struct __vector_relocate_via_memcpy
> struct __vector_destroy_via_noop
> ```
> So I'd like a naming scheme that fits all three use-cases comfortably.
>
> How about just adding the word "should"? `__vector_should_construct_via_memcpy`, `__vector_should_destroy_via_noop`, etc? Would that sufficiently address the "too imperative" issue?
Yes, "should" is fine as it implies yes/no answer.
Repository:
rCXX libc++
https://reviews.llvm.org/D49317
More information about the cfe-commits
mailing list