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 > > >