[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/memory:1658
+                || !__has_construct<allocator_type, _DestTp*, const _SourceTp&>::value) &&
+             is_trivially_move_constructible<_DestTp>::value,
             void
----------------
Quuxplusone wrote:
> Shouldn't this be something like `is_trivially_constructible<_DestTp, _SourceTp&>`?  I mean, we're never doing move-construction here. We're constructing `_DestTp` from `_SourceTp&`, which is either copy-construction or non-const-copy-construction, depending on the constness of `_SourceTp`.
> 
> `is_trivially_constructible` has pitfalls in general — https://quuxplusone.github.io/blog/2018/07/03/trivially-constructible-from/ — but I think we won't fall into any of those pits as long as we're testing that `_SourceTp` is cv-qualified `_DestTp`.
> Shouldn't this be something like `is_trivially_constructible<_DestTp, _SourceTp&>`?

I think you're right. We should also fix `__construct_forward` and `__construct_backward`, but maybe as part of a separate change since the three are consistently using `is_trivially_move_constructible` (and so there might be a reason for this).


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

https://reviews.llvm.org/D48342