On Thu, 7 Dec 2023 at 13:41, Jonathan Wakely <jwak...@redhat.com> wrote:
>
> On Wed, 6 Dec 2023 at 20:55, François Dumont <frs.dum...@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.
I suppose we could use typename iterator_traits<T>::reference instead
of decltype(auto), and hope that typedef is actuallyaccurate, and what
dereferencing the iterator gives us.
I do like the idea of simplifying the algos this way, I'm just
concerned that the changes to the ops will break some code that works
today.
>
>
> >
> > 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
> > >
