On 18/10/24 10:41 +0100, Jonathan Wakely wrote:
On 16/10/24 21:39 -0400, Patrick Palka wrote:
On Tue, 15 Oct 2024, Jonathan Wakely wrote:
+#if __cplusplus < 201103L
+
+ // True if we can unwrap _Iter to get a pointer by using std::__niter_base.
+ template<typename _Iter>
+ struct __unwrappable_niter
+ {
+ template<typename> struct __is_ptr { enum { __value = 0 }; };
+ template<typename _Tp> struct __is_ptr<_Tp*> { enum { __value = 1 }; };
+
+ typedef __decltype(std::__niter_base(*(_Iter*)0)) _Base;
+
+ enum { __value = __is_ptr<_Base>::__value };
+ };
It might be slightly cheaper to define this without the nested class
template as:
template<typename _Iter, typename _Base =
__decltype(std::__niter_base(*(_Iter*)0))>
struct __unwrappable_niter
{ enum { __value = false }; };
template<typename _Iter, typename _Tp>
struct __unwrappable_niter<_Iter, _Tp*>
{ enum { __value = true }; };
Nice. I think after spending a while failing to make any C++98
metaprogramming work for __memcpyable in cpp_type_traits.h I was not
in the mood for fighting C++98 any more :-) But this works well.
+
+ // Use template specialization for C++98 when 'if constexpr' can't be used.
+ template<bool _CanMemcpy>
struct __uninitialized_copy
{
template<typename _InputIterator, typename _ForwardIterator>
@@ -186,53 +172,150 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<>
struct __uninitialized_copy<true>
{
+ // Overload for generic iterators.
template<typename _InputIterator, typename _ForwardIterator>
static _ForwardIterator
__uninit_copy(_InputIterator __first, _InputIterator __last,
_ForwardIterator __result)
- { return std::copy(__first, __last, __result); }
- };
+ {
+ if (__unwrappable_niter<_InputIterator>::__value
+ && __unwrappable_niter<_ForwardIterator>::__value)
+ {
+ __uninit_copy(std::__niter_base(__first),
+ std::__niter_base(__last),
+ std::__niter_base(__result));
+ std::advance(__result, std::distance(__first, __last));
+ return __result;
+ }
+ else
+ return std::__do_uninit_copy(__first, __last, __result);
+ }
+ // Overload for pointers.
+ template<typename _Tp, typename _Up>
+ static _Up*
+ __uninit_copy(_Tp* __first, _Tp* __last, _Up* __result)
+ {
+ // Ensure that we don't successfully memcpy in cases that should be
+ // ill-formed because is_constructible<_Up, _Tp&> is false.
+ typedef __typeof__(static_cast<_Up>(*__first)) __check
+ __attribute__((__unused__));
+
+ if (const ptrdiff_t __n = __last - __first)
Do we have to worry about the __n == 1 case here like in the C++11 code path?
Actually I think we don't need to worry about it in either case.
C++20 had a note in [specialized.algorithms.general]/3 that said:
[Note 1: When invoked on ranges of potentially-overlapping subobjects
([intro.object]), the algorithms specified in [specialized.algorithms]
result in undefined behavior. — end note]
The reason is that the uninitialized algos create new objects at the
specified storage locations, and creating new objects reuses storage,
which ends the lifetime of any other objects in that storage. That
includes any objects that were in tail padding within that storage.
See Casey's Feb 2023 comment at
https://github.com/cplusplus/draft/issues/6143
That note was removed for C++23 (which is unfortunate IMHO), but the
algos still reuse storage by creating new objects, and so still end
the lifetime of potentially-overlapping subobjects within that
storage.
For std::copy there are no new objects created, and the effects are
specified in terms of assignment, which does not reuse storage. A
compiler-generated trivial copy assignment operator is careful to not
overwrite tail padding, so we can't use memmove if it would produce
different effects.
tl;dr I think I can remove the __n == 1 handling from the C++11 paths.
I actually had `if (__n > 0)` in an earlier version, and then looked
at it before posting the patch and "fixed" it to check __n > 1 and
then handle the __n == 1 case. But that's not nexcessary here, I'd
just been spending too much time looking at std::copy and got
confused.
