On Tue, Jun 10, 2025 at 6:17 PM Jonathan Wakely <[email protected]> wrote:
> With improved memset optimizations in std::uninitialized_fill and
> std::uninitialized_fill_n (see r15-4473-g3abe751ea86e34), we can make
> the non-standard internal helpers __uninitialized_default and
> __uninitialized_default_n use those directly instead of using std::fill
> and std::fill_n respectively. And if we do that, we no longer need to
> check whether the type is assignable, because avoiding std::fill means
> no assignment happens.
>
> If the type being constructed is trivially default constructible and
> trivially copy constructible, then it's unobservable if we construct one
> object and copy it N-1 times, rather than constructing N objects. For
> byte-sized integer types this allows the loop to be replaced with
> memset.
>
> Because these functions are not defined for C++98 at all, we can use
> if-constexpr to simplify them and remove the dispatching to members of
> class template specializations.
>
> By removing the uses of std::fill and std::fill_n we no longer need to
> include stl_algobase.h in stl_uninitialized.h which might improve
> compilation time for some other parts of the library.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/stl_uninitialized.h: Do not include
> bits/stl_algobase.h.
> (__uninitialized_default_1, __uninitialized_default_n_1):
> Remove.
> (__uninitialized_default, __uninitialized_default_n): Use
> 'if constexpr' and only consider triviality constructibility
> not assignability when deciding on the algorithm to use.
> ---
>
> Tested x86_64-linux.
>
Only one stylitic comment.
>
> libstdc++-v3/include/bits/stl_uninitialized.h | 136 ++++++------------
> 1 file changed, 42 insertions(+), 94 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/stl_uninitialized.h
> b/libstdc++-v3/include/bits/stl_uninitialized.h
> index bde787c2beaa..d5a399e16c90 100644
> --- a/libstdc++-v3/include/bits/stl_uninitialized.h
> +++ b/libstdc++-v3/include/bits/stl_uninitialized.h
> @@ -60,7 +60,6 @@
> # include <type_traits>
> # include <bits/ptr_traits.h> // to_address
> # include <bits/stl_pair.h> // pair
> -# include <bits/stl_algobase.h> // fill, fill_n
> #endif
>
> #include <bits/cpp_type_traits.h> // __is_pointer
> @@ -829,92 +828,36 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> // Extensions: __uninitialized_default, __uninitialized_default_n,
> // __uninitialized_default_a, __uninitialized_default_n_a.
>
> - template<bool _TrivialValueType>
> - struct __uninitialized_default_1
> - {
> - template<typename _ForwardIterator>
> - _GLIBCXX26_CONSTEXPR
> - static void
> - __uninit_default(_ForwardIterator __first, _ForwardIterator
> __last)
> - {
> - _UninitDestroyGuard<_ForwardIterator> __guard(__first);
> - for (; __first != __last; ++__first)
> - std::_Construct(std::addressof(*__first));
> - __guard.release();
> - }
> - };
> -
> - template<>
> - struct __uninitialized_default_1<true>
> - {
> - template<typename _ForwardIterator>
> - _GLIBCXX26_CONSTEXPR
> - static void
> - __uninit_default(_ForwardIterator __first, _ForwardIterator
> __last)
> - {
> - if (__first == __last)
> - return;
> -
> - typename iterator_traits<_ForwardIterator>::value_type* __val
> - = std::addressof(*__first);
> - std::_Construct(__val);
> - if (++__first != __last)
> - std::fill(__first, __last, *__val);
> - }
> - };
> -
> - template<bool _TrivialValueType>
> - struct __uninitialized_default_n_1
> - {
> - template<typename _ForwardIterator, typename _Size>
> - _GLIBCXX20_CONSTEXPR
> - static _ForwardIterator
> - __uninit_default_n(_ForwardIterator __first, _Size __n)
> - {
> - _UninitDestroyGuard<_ForwardIterator> __guard(__first);
> - for (; __n > 0; --__n, (void) ++__first)
> - std::_Construct(std::addressof(*__first));
> - __guard.release();
> - return __first;
> - }
> - };
> -
> - template<>
> - struct __uninitialized_default_n_1<true>
> - {
> - template<typename _ForwardIterator, typename _Size>
> - _GLIBCXX20_CONSTEXPR
> - static _ForwardIterator
> - __uninit_default_n(_ForwardIterator __first, _Size __n)
> - {
> - if (__n > 0)
> - {
> - typename iterator_traits<_ForwardIterator>::value_type* __val
> - = std::addressof(*__first);
> - std::_Construct(__val);
> - ++__first;
> - __first = std::fill_n(__first, __n - 1, *__val);
> - }
> - return __first;
> - }
> - };
> -
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wc++17-extensions"
> // __uninitialized_default
> // Fills [first, last) with value-initialized value_types.
> template<typename _ForwardIterator>
> _GLIBCXX26_CONSTEXPR
> inline void
> - __uninitialized_default(_ForwardIterator __first,
> - _ForwardIterator __last)
> + __uninitialized_default(_ForwardIterator __first, _ForwardIterator
> __last)
> {
> - typedef typename iterator_traits<_ForwardIterator>::value_type
> - _ValueType;
> - // trivial types can have deleted assignment
> - const bool __assignable = is_copy_assignable<_ValueType>::value;
> + using _ValueType = typename
> iterator_traits<_ForwardIterator>::value_type;
>
> - std::__uninitialized_default_1<__is_trivial(_ValueType)
> - && __assignable>::
> - __uninit_default(__first, __last);
> + if constexpr (__and_<is_trivially_default_constructible<_ValueType>,
> +
> is_trivially_copy_constructible<_ValueType>>::value)
> + {
> + if (__first == __last)
> + return;
>
This is different from _n, I would do them consistently and have:
if (!std::__is_constant_evaluated() && __first != __last)
> + if (!std::__is_constant_evaluated())
> + {
> + auto* __addr = std::addressof(*__first);
> + std::_Construct(__addr);
> + const auto& __val = *__addr;
> + if (++__first != __last)
> + std::uninitialized_fill(__first, __last, __val);
> + }
> + }
> +
> + _UninitDestroyGuard<_ForwardIterator> __guard(__first);
> + for (; __first != __last; ++__first)
> + std::_Construct(std::addressof(*__first));
> + __guard.release();
> }
>
> // __uninitialized_default_n
> @@ -924,23 +867,28 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> inline _ForwardIterator
> __uninitialized_default_n(_ForwardIterator __first, _Size __n)
> {
> -#ifdef __cpp_lib_is_constant_evaluated
> - if (std::is_constant_evaluated())
> - return __uninitialized_default_n_1<false>::
> - __uninit_default_n(__first, __n);
> -#endif
> + using _ValueType = typename
> iterator_traits<_ForwardIterator>::value_type;
>
> - typedef typename iterator_traits<_ForwardIterator>::value_type
> - _ValueType;
> - // See uninitialized_fill_n for the conditions for using
> std::fill_n.
> - constexpr bool __can_fill
> - = __and_<is_integral<_Size>,
> is_copy_assignable<_ValueType>>::value;
> + if constexpr (__and_<is_trivially_default_constructible<_ValueType>,
> + is_trivially_copy_constructible<_ValueType>,
> + is_integral<_Size>>::value)
> + {
>
We do not need nested braces hre.
> + if (!std::__is_constant_evaluated() && __n > 0)
> + {
+ auto* __addr = std::addressof(*__first);
> + std::_Construct(__addr);
> + const auto& __val = *__addr;
> + return std::uninitialized_fill_n(++__first, --__n, __val);
> + }
> + }
>
> - return __uninitialized_default_n_1<__is_trivial(_ValueType)
> - && __can_fill>::
> - __uninit_default_n(__first, __n);
> + _UninitDestroyGuard<_ForwardIterator> __guard(__first);
> + for (; __n > 0; --__n, (void) ++__first)
> + std::_Construct(std::addressof(*__first));
> + __guard.release();
> + return __first;
> }
> -
> +#pragma GCC diagnostic pop
>
> // __uninitialized_default_a
> // Fills [first, last) with value_types constructed by the allocator
> --
> 2.49.0
>
>
