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 `inline` here?
I'm actually not sure — and also suddenly not sure if the visibility attribute
should be `_LIBCPP_INLINE_VISIBILITY` or `_LIBCPP_TEMPLATE_VIS`. (I *think* the
latter is only for type templates, though, not function templates?) However,
this is exactly parallel to what we do for `operator<`, so I think changing it
would be gratuitous. If someone wants to remove `inline` from a bunch of
templates, I won't complain, but I also don't want this PR to be the one that
initiates it.
```
template <class _Tp, class _Allocator>
inline _LIBCPP_INLINE_VISIBILITY
bool
operator< (const vector<_Tp, _Allocator>& __x, const vector<_Tp, _Allocator>&
__y)
{
return _VSTD::lexicographical_compare(__x.begin(), __x.end(), __y.begin(),
__y.end());
}
```
================
Comment at: include/vector:545
+ is_trivially_move_constructible<_Tp>::value
+> {};
+
----------------
Louis writes:
> 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 themselves.
I know where you're coming from, but I believe that in this case we definitely
*can't* do that, because the whole point of these routines is that the routine
itself can't always tell whether it's supposed to memcpy or not; the *caller*
is the only one with the power to decide that. The decision (in theory, though
not yet in practice, because this particular PR is a pure refactoring) depends
not only on details of `_Tp` and `_Allocator` but also on the specific
call-site: we can memcpy more aggressively at some call-sites than others,
because of information available only to the caller (such as "this is a
relocation operation").
See
https://github.com/Quuxplusone/libcxx/commit/e7e5999b01#diff-07c2b769648850d040dcbb07754e5f2fR1076
, lines 1076 et seq., for how I envision some future caller making the
decisions on a callsite-by-callsite basis.
Repository:
rCXX libc++
https://reviews.llvm.org/D49317
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
