libstdc++/ranges: Fix more wrong value type init from reference type [PR111861]

  Tested on x86_64-pc-linux-gnu, does this look OK for trunk/15/14?

-- >8 --

As in r16-3912-g412a1f78b53709, this fixes some other spots where we
wrongly use a deduced type and non-direct-initialization when intending
to initialize a value type from an iterator's reference type.

	PR libstdc++/111861

libstdc++-v3/ChangeLog:

	* include/bits/ranges_algo.h (ranges::unique_copy): When
	initializing a value type object from *iter, use
	direct-initialization and don't use a deduced type.
	(ranges::push_heap): Use direct-initialization when initializing
	a value type object from ranges::iter_move.
	(ranges::max): As in ranges::unique_copy.
	* include/bits/ranges_util.h (ranges::min): Likewise.
---
 libstdc++-v3/include/bits/ranges_algo.h | 8 ++++----
 libstdc++-v3/include/bits/ranges_util.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)
  

On Wed, 17 Sept 2025 at 15:19, Patrick Palka <ppalka@redhat.com> wrote:
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk/15/14?
>
> -- >8 --
>
> As in r16-3912-g412a1f78b53709, this fixes some other spots where we
> wrongly use a deduced type and non-direct-initialization when intending
> to initialize a value type from an iterator's reference type.
>
>         PR libstdc++/111861
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/ranges_algo.h (ranges::unique_copy): When
>         initializing a value type object from *iter, use
>         direct-initialization and don't use a deduced type.
>         (ranges::push_heap): Use direct-initialization when initializing
>         a value type object from ranges::iter_move.
>         (ranges::max): As in ranges::unique_copy.
>         * include/bits/ranges_util.h (ranges::min): Likewise.
> ---
>  libstdc++-v3/include/bits/ranges_algo.h | 8 ++++----
>  libstdc++-v3/include/bits/ranges_util.h | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
> index 4025bba9f204..eebad9e1621c 100644
> --- a/libstdc++-v3/include/bits/ranges_algo.h
> +++ b/libstdc++-v3/include/bits/ranges_algo.h
> @@ -1529,7 +1529,7 @@ namespace ranges
>           }
>         else // indirectly_copyable_storable<_Iter, _Out>
>           {
> -           auto __value = *__first;
> +           iter_value_t<_Iter> __value(*__first);
>             *__result = __value;
>             while (++__first != __last)
>               {
> @@ -2075,9 +2075,9 @@ namespace ranges
>         else
>           {
>             auto __comp_proj = __detail::__make_comp_proj(__comp, __proj);
> +           iter_value_t<_Iter> __value(ranges::iter_move(ranges::prev(__last)));
>             __detail::__push_heap(__first, (__last - __first) - 1,
> -                                 0, ranges::iter_move(ranges::prev(__last)),
> -                                 __comp_proj);
> +                                 0, __value, __comp_proj);

Should this be std::move(__value)?

I find it quite painful that the standard allows iterators to return a
proxy that doesn't implicitly convert to the value type. Or that
ranges::iter_move doesn't do the conversion and guarantee to return
value_type or a real reference to value_type, instead of returning the
proxy reference. It makes it quite difficult to write correct code.

>             return __last;
>           }
>        }
> @@ -4219,7 +4219,7 @@ namespace ranges
>         auto __first = ranges::begin(__r);
>         auto __last = ranges::end(__r);
>         __glibcxx_assert(__first != __last);
> -       auto __result = *__first;
> +       range_value_t<_Range> __result(*__first);
>         while (++__first != __last)
>           {
>             auto&& __tmp = *__first;
> diff --git a/libstdc++-v3/include/bits/ranges_util.h b/libstdc++-v3/include/bits/ranges_util.h
> index 84de258908ea..2aa8938edf25 100644
> --- a/libstdc++-v3/include/bits/ranges_util.h
> +++ b/libstdc++-v3/include/bits/ranges_util.h
> @@ -761,7 +761,7 @@ namespace ranges
>         auto __first = ranges::begin(__r);
>         auto __last = ranges::end(__r);
>         __glibcxx_assert(__first != __last);
> -       auto __result = *__first;
> +       range_value_t<_Range> __result(*__first);
>         while (++__first != __last)
>           {
>             auto&& __tmp = *__first;
> --
> 2.51.0.268.ga483264b01
>
  

On Wed, 17 Sep 2025, Jonathan Wakely wrote:

> On Wed, 17 Sept 2025 at 15:19, Patrick Palka <ppalka@redhat.com> wrote:
> >
> > Tested on x86_64-pc-linux-gnu, does this look OK for trunk/15/14?
> >
> > -- >8 --
> >
> > As in r16-3912-g412a1f78b53709, this fixes some other spots where we
> > wrongly use a deduced type and non-direct-initialization when intending
> > to initialize a value type from an iterator's reference type.
> >
> >         PR libstdc++/111861
> >
> > libstdc++-v3/ChangeLog:
> >
> >         * include/bits/ranges_algo.h (ranges::unique_copy): When
> >         initializing a value type object from *iter, use
> >         direct-initialization and don't use a deduced type.
> >         (ranges::push_heap): Use direct-initialization when initializing
> >         a value type object from ranges::iter_move.
> >         (ranges::max): As in ranges::unique_copy.
> >         * include/bits/ranges_util.h (ranges::min): Likewise.
> > ---
> >  libstdc++-v3/include/bits/ranges_algo.h | 8 ++++----
> >  libstdc++-v3/include/bits/ranges_util.h | 2 +-
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
> > index 4025bba9f204..eebad9e1621c 100644
> > --- a/libstdc++-v3/include/bits/ranges_algo.h
> > +++ b/libstdc++-v3/include/bits/ranges_algo.h
> > @@ -1529,7 +1529,7 @@ namespace ranges
> >           }
> >         else // indirectly_copyable_storable<_Iter, _Out>
> >           {
> > -           auto __value = *__first;
> > +           iter_value_t<_Iter> __value(*__first);
> >             *__result = __value;
> >             while (++__first != __last)
> >               {
> > @@ -2075,9 +2075,9 @@ namespace ranges
> >         else
> >           {
> >             auto __comp_proj = __detail::__make_comp_proj(__comp, __proj);
> > +           iter_value_t<_Iter> __value(ranges::iter_move(ranges::prev(__last)));
> >             __detail::__push_heap(__first, (__last - __first) - 1,
> > -                                 0, ranges::iter_move(ranges::prev(__last)),
> > -                                 __comp_proj);
> > +                                 0, __value, __comp_proj);
> 
> Should this be std::move(__value)?

Oops, fixed.

> 
> I find it quite painful that the standard allows iterators to return a
> proxy that doesn't implicitly convert to the value type. Or that
> ranges::iter_move doesn't do the conversion and guarantee to return
> value_type or a real reference to value_type, instead of returning the
> proxy reference. It makes it quite difficult to write correct code.

Yeah :/

How does the below look?  This is probably not worth backporting
actually.  I doubt this causes problems in practice.

-- >8 --

Subject: [PATCH] libstdc++/ranges: Fix more wrong value type init from
 reference type [PR111861]

	PR libstdc++/111861

libstdc++-v3/ChangeLog:

	* include/bits/ranges_algo.h (ranges::unique_copy): When
	initializing a value type object from *iter, use
	direct-initialization and don't use a deduced type.
	(ranges::push_heap): Use direct-initialization when initializing
	a value type object from ranges::iter_move.
	(ranges::max): As in ranges::unique_copy.
	* include/bits/ranges_util.h (ranges::min): Likewise.
---
 libstdc++-v3/include/bits/ranges_algo.h | 8 ++++----
 libstdc++-v3/include/bits/ranges_util.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
index 4025bba9f204..5c9fe627aee0 100644
--- a/libstdc++-v3/include/bits/ranges_algo.h
+++ b/libstdc++-v3/include/bits/ranges_algo.h
@@ -1529,7 +1529,7 @@ namespace ranges
 	  }
 	else // indirectly_copyable_storable<_Iter, _Out>
 	  {
-	    auto __value = *__first;
+	    iter_value_t<_Iter> __value(*__first);
 	    *__result = __value;
 	    while (++__first != __last)
 	      {
@@ -2075,9 +2075,9 @@ namespace ranges
 	else
 	  {
 	    auto __comp_proj = __detail::__make_comp_proj(__comp, __proj);
+	    iter_value_t<_Iter> __value(ranges::iter_move(ranges::prev(__last)));
 	    __detail::__push_heap(__first, (__last - __first) - 1,
-				  0, ranges::iter_move(ranges::prev(__last)),
-				  __comp_proj);
+				  0, std::move(__value), __comp_proj);
 	    return __last;
 	  }
       }
@@ -4219,7 +4219,7 @@ namespace ranges
 	auto __first = ranges::begin(__r);
 	auto __last = ranges::end(__r);
 	__glibcxx_assert(__first != __last);
-	auto __result = *__first;
+	range_value_t<_Range> __result(*__first);
 	while (++__first != __last)
 	  {
 	    auto&& __tmp = *__first;
diff --git a/libstdc++-v3/include/bits/ranges_util.h b/libstdc++-v3/include/bits/ranges_util.h
index 84de258908ea..2aa8938edf25 100644
--- a/libstdc++-v3/include/bits/ranges_util.h
+++ b/libstdc++-v3/include/bits/ranges_util.h
@@ -761,7 +761,7 @@ namespace ranges
 	auto __first = ranges::begin(__r);
 	auto __last = ranges::end(__r);
 	__glibcxx_assert(__first != __last);
-	auto __result = *__first;
+	range_value_t<_Range> __result(*__first);
 	while (++__first != __last)
 	  {
 	    auto&& __tmp = *__first;
  

On Wed, 17 Sept 2025 at 16:08, Patrick Palka <ppalka@redhat.com> wrote:
>
> On Wed, 17 Sep 2025, Jonathan Wakely wrote:
>
> > On Wed, 17 Sept 2025 at 15:19, Patrick Palka <ppalka@redhat.com> wrote:
> > >
> > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk/15/14?
> > >
> > > -- >8 --
> > >
> > > As in r16-3912-g412a1f78b53709, this fixes some other spots where we
> > > wrongly use a deduced type and non-direct-initialization when intending
> > > to initialize a value type from an iterator's reference type.
> > >
> > >         PR libstdc++/111861
> > >
> > > libstdc++-v3/ChangeLog:
> > >
> > >         * include/bits/ranges_algo.h (ranges::unique_copy): When
> > >         initializing a value type object from *iter, use
> > >         direct-initialization and don't use a deduced type.
> > >         (ranges::push_heap): Use direct-initialization when initializing
> > >         a value type object from ranges::iter_move.
> > >         (ranges::max): As in ranges::unique_copy.
> > >         * include/bits/ranges_util.h (ranges::min): Likewise.
> > > ---
> > >  libstdc++-v3/include/bits/ranges_algo.h | 8 ++++----
> > >  libstdc++-v3/include/bits/ranges_util.h | 2 +-
> > >  2 files changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
> > > index 4025bba9f204..eebad9e1621c 100644
> > > --- a/libstdc++-v3/include/bits/ranges_algo.h
> > > +++ b/libstdc++-v3/include/bits/ranges_algo.h
> > > @@ -1529,7 +1529,7 @@ namespace ranges
> > >           }
> > >         else // indirectly_copyable_storable<_Iter, _Out>
> > >           {
> > > -           auto __value = *__first;
> > > +           iter_value_t<_Iter> __value(*__first);
> > >             *__result = __value;
> > >             while (++__first != __last)
> > >               {
> > > @@ -2075,9 +2075,9 @@ namespace ranges
> > >         else
> > >           {
> > >             auto __comp_proj = __detail::__make_comp_proj(__comp, __proj);
> > > +           iter_value_t<_Iter> __value(ranges::iter_move(ranges::prev(__last)));
> > >             __detail::__push_heap(__first, (__last - __first) - 1,
> > > -                                 0, ranges::iter_move(ranges::prev(__last)),
> > > -                                 __comp_proj);
> > > +                                 0, __value, __comp_proj);
> >
> > Should this be std::move(__value)?
>
> Oops, fixed.
>
> >
> > I find it quite painful that the standard allows iterators to return a
> > proxy that doesn't implicitly convert to the value type. Or that
> > ranges::iter_move doesn't do the conversion and guarantee to return
> > value_type or a real reference to value_type, instead of returning the
> > proxy reference. It makes it quite difficult to write correct code.
>
> Yeah :/
>
> How does the below look?  This is probably not worth backporting
> actually.  I doubt this causes problems in practice.

OK for trunk, and I agree that it's unlikely to matter to anybody, so
not important to backport.

>
> -- >8 --
>
> Subject: [PATCH] libstdc++/ranges: Fix more wrong value type init from
>  reference type [PR111861]
>
>         PR libstdc++/111861
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/ranges_algo.h (ranges::unique_copy): When
>         initializing a value type object from *iter, use
>         direct-initialization and don't use a deduced type.
>         (ranges::push_heap): Use direct-initialization when initializing
>         a value type object from ranges::iter_move.
>         (ranges::max): As in ranges::unique_copy.
>         * include/bits/ranges_util.h (ranges::min): Likewise.
> ---
>  libstdc++-v3/include/bits/ranges_algo.h | 8 ++++----
>  libstdc++-v3/include/bits/ranges_util.h | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
> index 4025bba9f204..5c9fe627aee0 100644
> --- a/libstdc++-v3/include/bits/ranges_algo.h
> +++ b/libstdc++-v3/include/bits/ranges_algo.h
> @@ -1529,7 +1529,7 @@ namespace ranges
>           }
>         else // indirectly_copyable_storable<_Iter, _Out>
>           {
> -           auto __value = *__first;
> +           iter_value_t<_Iter> __value(*__first);
>             *__result = __value;
>             while (++__first != __last)
>               {
> @@ -2075,9 +2075,9 @@ namespace ranges
>         else
>           {
>             auto __comp_proj = __detail::__make_comp_proj(__comp, __proj);
> +           iter_value_t<_Iter> __value(ranges::iter_move(ranges::prev(__last)));
>             __detail::__push_heap(__first, (__last - __first) - 1,
> -                                 0, ranges::iter_move(ranges::prev(__last)),
> -                                 __comp_proj);
> +                                 0, std::move(__value), __comp_proj);
>             return __last;
>           }
>        }
> @@ -4219,7 +4219,7 @@ namespace ranges
>         auto __first = ranges::begin(__r);
>         auto __last = ranges::end(__r);
>         __glibcxx_assert(__first != __last);
> -       auto __result = *__first;
> +       range_value_t<_Range> __result(*__first);
>         while (++__first != __last)
>           {
>             auto&& __tmp = *__first;
> diff --git a/libstdc++-v3/include/bits/ranges_util.h b/libstdc++-v3/include/bits/ranges_util.h
> index 84de258908ea..2aa8938edf25 100644
> --- a/libstdc++-v3/include/bits/ranges_util.h
> +++ b/libstdc++-v3/include/bits/ranges_util.h
> @@ -761,7 +761,7 @@ namespace ranges
>         auto __first = ranges::begin(__r);
>         auto __last = ranges::end(__r);
>         __glibcxx_assert(__first != __last);
> -       auto __result = *__first;
> +       range_value_t<_Range> __result(*__first);
>         while (++__first != __last)
>           {
>             auto&& __tmp = *__first;
> --
> 2.51.0.268.ga483264b01
>