[4/6] libstdc++: port the ranges::uninitialized_* algorithms away from is_trivial

Hello,

The range-based uninitialized_* algorithm have codepaths that turn 
constructions into assignments if the output type is trivial. Change 
this detection to be more accurate. There's a couple of cases which are 
suspicious and deserve accurate reasoning.

Thanks,

On 09/12/24 13:22 +0100, Giuseppe D'Angelo wrote:
>Hello,
>
>The range-based uninitialized_* algorithm have codepaths that turn 
>constructions into assignments if the output type is trivial. Change 
>this detection to be more accurate. There's a couple of cases which 
>are suspicious and deserve accurate reasoning.

I think those algos should just be updated in the same way (and for
the same reasons) that I did in r15-4473-g3abe751ea86e34

The current optimizations are suboptimal because they are restricted
to cases where the std::copy/std::fill algo can be used, which means
we fail to optimize some cases where memcpy/memset could be used.

But that can be done later, the patch looks OK as-is.


>Thanks,
>-- 
>Giuseppe D'Angelo

>From 41e39c72edba5ab0559881d3b4136d551649e4cc Mon Sep 17 00:00:00 2001
>From: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
>Date: Mon, 9 Dec 2024 02:22:23 +0100
>Subject: [PATCH 4/6] libstdc++: port the ranges::uninitialized_* algorithms
> away from is_trivial
>
>In preparation for the deprecation of is_trivial (P3247R2).
>The rangified uninitialized_* specialized memory algorithms have code
>paths where they call the non-uninitialized versions, because the latter
>are usually optimized. The detection in these code paths uses is_trivial;
>port it away from it towards more specific checks.
>
>The detection for the copy/move algorithms was suspicious: it checked
>that the output type was trivial, and that assignment from the input
>range reference type was nothrow. If so, the algorithm would copy/move
>assign (by calling the ranges::copy/move algorithms) instead of
>constructing elements. I think this is off because:
>
>1) the constructor that would be called by the algorithm (which may be
>   neither a copy or a move constructor) wasn't checked. If that
>   constructor isn't trivial the caller might detect that we're not
>   calling it, and that goes against the algorithms' specifications.
>2) a nothrow assignment is necessary but not sufficient, as again we
>   need to check for triviality, or the caller can detect we're calling
>   an assignment operator we were never meant to be calling from these
>   algorithms.
>
>Therefore I've amended the respective detections.
>
>libstdc++-v3/ChangeLog:
>
>	* include/bits/ranges_uninitialized.h: port some if
>	  constexpr away from is_trivial, and towards more specific
>	  detections instead.
>
>Signed-off-by: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
>---
> .../include/bits/ranges_uninitialized.h       | 36 ++++++++++---------
> 1 file changed, 20 insertions(+), 16 deletions(-)
>
>diff --git a/libstdc++-v3/include/bits/ranges_uninitialized.h b/libstdc++-v3/include/bits/ranges_uninitialized.h
>index f16f2ef39f5..ff673d58be7 100644
>--- a/libstdc++-v3/include/bits/ranges_uninitialized.h
>+++ b/libstdc++-v3/include/bits/ranges_uninitialized.h
>@@ -202,8 +202,8 @@ namespace ranges
>       operator()(_Iter __first, _Sent __last) const
>       {
> 	using _ValueType = remove_reference_t<iter_reference_t<_Iter>>;
>-	if constexpr (is_trivial_v<_ValueType>
>-		      && is_copy_assignable_v<_ValueType>)
>+	if constexpr (is_trivially_default_constructible_v<_ValueType>
>+		      && is_trivially_copy_assignable_v<_ValueType>)
> 	  return ranges::fill(__first, __last, _ValueType());
> 	else
> 	  {
>@@ -235,8 +235,8 @@ namespace ranges
>       operator()(_Iter __first, iter_difference_t<_Iter> __n) const
>       {
> 	using _ValueType = remove_reference_t<iter_reference_t<_Iter>>;
>-	if constexpr (is_trivial_v<_ValueType>
>-		      && is_copy_assignable_v<_ValueType>)
>+	if constexpr (is_trivially_default_constructible_v<_ValueType>
>+		      && is_trivially_copy_assignable_v<_ValueType>)
> 	  return ranges::fill_n(__first, __n, _ValueType());
> 	else
> 	  {
>@@ -268,8 +268,9 @@ namespace ranges
> 	using _OutType = remove_reference_t<iter_reference_t<_Out>>;
> 	if constexpr (sized_sentinel_for<_ISent, _Iter>
> 		      && sized_sentinel_for<_OSent, _Out>
>-		      && is_trivial_v<_OutType>
>-		      && is_nothrow_assignable_v<_OutType&,
>+		      && is_trivially_constructible_v<_OutType, iter_reference_t<_Iter>>
>+		      && is_trivially_default_constructible_v<_OutType>
>+		      && is_trivially_assignable_v<_OutType&,
> 						 iter_reference_t<_Iter>>)
> 	  {
> 	    auto __d1 = __ilast - __ifirst;
>@@ -316,8 +317,9 @@ namespace ranges
>       {
> 	using _OutType = remove_reference_t<iter_reference_t<_Out>>;
> 	if constexpr (sized_sentinel_for<_Sent, _Out>
>-		      && is_trivial_v<_OutType>
>-		      && is_nothrow_assignable_v<_OutType&,
>+		      && is_trivially_constructible_v<_OutType, iter_reference_t<_Iter>>
>+		      && is_trivially_default_constructible_v<_OutType>
>+		      && is_trivially_assignable_v<_OutType&,
> 						 iter_reference_t<_Iter>>)
> 	  {
> 	    auto __d = __olast - __ofirst;
>@@ -355,8 +357,9 @@ namespace ranges
> 	using _OutType = remove_reference_t<iter_reference_t<_Out>>;
> 	if constexpr (sized_sentinel_for<_ISent, _Iter>
> 		      && sized_sentinel_for<_OSent, _Out>
>-		      && is_trivial_v<_OutType>
>-		      && is_nothrow_assignable_v<_OutType&,
>+		      && is_trivially_constructible_v<_OutType, iter_rvalue_reference_t<_Iter>>
>+		      && is_trivially_default_constructible_v<_OutType>
>+		      && is_trivially_assignable_v<_OutType&,
> 						 iter_rvalue_reference_t<_Iter>>)
> 	  {
> 	    auto __d1 = __ilast - __ifirst;
>@@ -407,8 +410,9 @@ namespace ranges
>       {
> 	using _OutType = remove_reference_t<iter_reference_t<_Out>>;
> 	if constexpr (sized_sentinel_for<_Sent, _Out>
>-		      && is_trivial_v<_OutType>
>-		      && is_nothrow_assignable_v<_OutType&,
>+		      && is_trivially_constructible_v<_OutType, iter_rvalue_reference_t<_Iter>>
>+		      && is_trivially_default_constructible_v<_OutType>
>+		      && is_trivially_assignable_v<_OutType&,
> 						 iter_rvalue_reference_t<_Iter>>)
> 	  {
> 	    auto __d = __olast - __ofirst;
>@@ -441,8 +445,8 @@ namespace ranges
>       operator()(_Iter __first, _Sent __last, const _Tp& __x) const
>       {
> 	using _ValueType = remove_reference_t<iter_reference_t<_Iter>>;
>-	if constexpr (is_trivial_v<_ValueType>
>-		      && is_nothrow_assignable_v<_ValueType&, const _Tp&>)
>+	if constexpr (is_trivially_constructible_v<_ValueType, const _Tp&>
>+		      && is_trivially_assignable_v<_ValueType&, const _Tp&>)
> 	  return ranges::fill(__first, __last, __x);
> 	else
> 	  {
>@@ -474,8 +478,8 @@ namespace ranges
> 		 const _Tp& __x) const
>       {
> 	using _ValueType = remove_reference_t<iter_reference_t<_Iter>>;
>-	if constexpr (is_trivial_v<_ValueType>
>-		      && is_nothrow_assignable_v<_ValueType&, const _Tp&>)
>+	if constexpr (is_trivially_constructible_v<_ValueType, const _Tp&>
>+		      && is_trivially_assignable_v<_ValueType&, const _Tp&>)
> 	  return ranges::fill_n(__first, __n, __x);
> 	else
> 	  {
>-- 
>2.34.1
>