Re: [PATCH] libstdc++: Avoid double-deref of __first in ranges::minmax [PR104858]

[Patrick Palka <ppalka@redhat.com>]

On Thu, 14 Apr 2022, Jonathan Wakely wrote:

> On Thu, 14 Apr 2022 at 16:21, Patrick Palka via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > Tested on x86_64-pc-linux-gnu, does this look OK for trunk and 11/10
> > once the branch is unfrozen?
> >
> >         PR libstdc++/104858
> >
> > libstdc++-v3/ChangeLog:
> >
> >         * include/bits/ranges_algo.h (__minmax_fn): Avoid dereferencing
> >         __first twice at the start.
> >         * testsuite/25_algorithms/minmax/constrained.cc (test06): New test.
> > ---
> >  libstdc++-v3/include/bits/ranges_algo.h       |  2 +-
> >  .../25_algorithms/minmax/constrained.cc       | 23 +++++++++++++++++++
> >  2 files changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
> > index 62dc605080a..3d30fb1428c 100644
> > --- a/libstdc++-v3/include/bits/ranges_algo.h
> > +++ b/libstdc++-v3/include/bits/ranges_algo.h
> > @@ -3084,7 +3084,7 @@ namespace ranges
> >         auto __last = ranges::end(__r);
> >         __glibcxx_assert(__first != __last);
> >         auto __comp_proj = __detail::__make_comp_proj(__comp, __proj);
> > -       minmax_result<range_value_t<_Range>> __result = {*__first, *__first};
> > +       minmax_result<range_value_t<_Range>> __result = {*__first, __result.min};
> 
> Clever ... I'm surprised this even works. I would have expected it to
> evaluate both initializers before actually initializing the members.
> TIL.

Indeed, it seems to do the right thing, practically speaking at least :)
FWIW the alternative approach

-       minmax_result<range_value_t<_Range>> __result = {*__first, *__first};
+       minmax_result<range_value_t<_Range>> __result;
+       __result.max = __result.min = *__first;

wouldn't be right because the value type is not necessarily default
constructible.  I beefed up the new testcase to verify we don't demand
default constructibility here.