Re: [PATCH] Reimplement __gnu_cxx::__ops operators

[Jonathan Wakely <jwakely@redhat.com>]

On Wed, 6 Dec 2023 at 20:55, François Dumont <frs.dumont@gmail.com> wrote:
>
> I think I still got no feedback about this cleanup proposal.

Can you remind me why we have all those different functions in
predefined_ops.h in the first place? I think it was to avoid having
two versions of every algorithm, one that does *l < *r and one that
does pred(*l, *r), right?

One property of the current code is that _Iter_less_iter will compare
exactly *lhs < *rhs and so works even with this type, where its
operator< only accepts non-const arguments:

struct X { bool operator<(X&); };

Doesn't your simplification break that, because the _Less function
only accepts const references now?

Maybe another way to remove the number of types in predefined_ops.h
would be to compose functions from projections. So instead of
_Iter_less_iter and _Iter_less_val etc. we have:

template<typename LProj, typename RProj>
struct _Less
{
  template<typename T, typename U>
    bool operator()(T& l, U& r) const
    { return LProj()(l) < RProj()(r); }
};

And a set of projections:

struct _Proj_deref {
  template<typename T>
    decltype(auto) operator()(T& t) const
    { return *t; }
};
struct _Proj_identity {
  template<typename T>
    T& operator()(T& t) const
    { return t; }
};

Then:

using _Iter_less_iter =_Less<_Proj_deref, _Proj_deref>;
using _Iter_less_val = _Less<_Proj_deref, _Proj_identity>;

The problem here is the use of decltype(auto) which isn't valid in
C++98. If we didn't have to support C++98 then we could just use
forwarding refs in_Less::operator()(T&&, U&&) and use perfect
forwarding everywhere. But we can't.


>
> Here is a new version.
>
> François
>
> On 15/06/2023 07:07, François Dumont wrote:
> > I think we all agree that __gnu_cxx::__ops needed to be reimplemented,
> > here it is.
> >
> > Note that I kept the usage of std::ref in <string>, <vector> and <deque>.
> >
> >     libstdc++: Reimplement __gnu_cxx::__ops operators
> >
> >     Replace functors using iterators as input to adopt functors that
> >     are matching the same Standard expectations as the ones imposed on
> >     predicates used in predicates-aware algos. Doing so we need far less
> >     functors. It impose that iterators are dereference at algo level and
> >     not in the functors anymore.
> >
> >     libstdc++-v3/ChangeLog:
> >
> >             * include/std/functional (_Not_fn): Move to...
> >             * include/bits/predefined_ops.h: ...here, and expose a
> > version
> >             in pre-C++14 mode.
> >             (__not_fn): New, use latter.
> >             (_Iter_less_iter, _Iter_less_val, _Val_less_iter,
> > _Iter_equal_to_iter)
> >             (_Iter_equal_to_val, _Iter_comp_iter, _Iter_comp_val,
> > _Val_comp_iter)
> >             (_Iter_equals_val, _Iter_equals_iter, _Iter_pred,
> > _Iter_comp_val)
> >             (_Iter_comp_to_val, _Iter_comp_to_iter, _Iter_negate):
> > Remove.
> >             (__iter_less_iter, __iter_less_val, __iter_comp_val,
> > __val_less_iter)
> >             (__val_comp_iter, __iter_equal_to_iter,
> > __iter_equal_to_val, __iter_comp_iter)
> >             (__val_comp_iter, __iter_equals_val, __iter_comp_iter,
> > __pred_iter): Remove.
> >             (_Less, _Equal_to, _Equal_to_val, _Comp_val): New.
> >             (__less, __equal_to, __comp_val): New.
> >             * include/bits/stl_algo.h: Adapt all algos to use new
> > __gnu_cxx::__ops operators.
> >             When possible use std::move to pass predicates between
> > routines.
> >             * include/bits/stl_algobase.h: Likewise.
> >             * include/bits/stl_heap.h: Likewise.
> >             * include/std/deque: Cleanup usage of __gnu_cxx::__ops
> > operators.
> >             * include/std/string: Likewise.
> >             * include/std/vector: Likewise.
> >
> > Tested under Linux x86_64 normal and _GLIBCXX_DEBUG modes.
> >
> > Ok to commit ?
> >
> > François
> >