Re: [PATCH] Reimplement __gnu_cxx::__ops operators
On 07/12/2023 14:41, Jonathan Wakely wrote:
On Wed, 6 Dec 2023 at 20:55, François Dumont 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?
Yes, that was the purpose.
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?
Indeed, I thought more about the problem of const qualification on the
operator(). This is why _Comp_val is mimicking what _Not_fn does.
To be honnest I even thought that this kind of operator above was more a
user code issue than a real use case we need to handle. But it looks
like you, I guess the C++ Standard then, impose to support it.
I'll rework it then, thanks for the proposal below and in your other email.
Re: [PATCH] Reimplement __gnu_cxx::__ops operators
On Thu, 7 Dec 2023 at 13:41, Jonathan Wakely wrote:
>
> On Wed, 6 Dec 2023 at 20:55, François Dumont 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
> struct _Less
> {
> template
> bool operator()(T& l, U& r) const
> { return LProj()(l) < RProj()(r); }
> };
>
> And a set of projections:
>
> struct _Proj_deref {
> template
> decltype(auto) operator()(T& t) const
> { return *t; }
> };
> struct _Proj_identity {
> template
> 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::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 , and .
> > >
> > > 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
> > >
Re: [PATCH] Reimplement __gnu_cxx::__ops operators
On Wed, 6 Dec 2023 at 20:55, François Dumont 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
struct _Less
{
template
bool operator()(T& l, U& r) const
{ return LProj()(l) < RProj()(r); }
};
And a set of projections:
struct _Proj_deref {
template
decltype(auto) operator()(T& t) const
{ return *t; }
};
struct _Proj_identity {
template
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 , and .
> >
> > 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
> >
Re: [PATCH] Reimplement __gnu_cxx::__ops operators
I think I still got no feedback about this cleanup proposal.
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 , and .
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
diff --git a/libstdc++-v3/include/bits/predefined_ops.h
b/libstdc++-v3/include/bits/predefined_ops.h
index e9933373ed9..8753e6f64cd 100644
--- a/libstdc++-v3/include/bits/predefined_ops.h
+++ b/libstdc++-v3/include/bits/predefined_ops.h
@@ -32,376 +32,229 @@
#include
+#if __cplusplus >= 201103L
+# include
+#endif
+
namespace __gnu_cxx
{
namespace __ops
{
- struct _Iter_less_iter
+ struct _Less
{
-template
+template
_GLIBCXX14_CONSTEXPR
bool
- operator()(_Iterator1 __it1, _Iterator2 __it2) const
- { return *__it1 < *__it2; }
+ operator()(const _Lhs& __lhs, const _Rhs& __rhs) const
+ { return __lhs < __rhs; }
};
_GLIBCXX14_CONSTEXPR
- inline _Iter_less_iter
- __iter_less_iter()
- { return _Iter_less_iter(); }
-
- struct _Iter_less_val
- {
-#if __cplusplus >= 201103L
-constexpr _Iter_less_val() = default;
-#else
-_Iter_less_val() { }
-#endif
-
-_GLIBCXX20_CONSTEXPR
-explicit
-_Iter_less_val(_Iter_less_iter) { }
-
-template
- _GLIBCXX20_CONSTEXPR
- bool
- operator()(_Iterator __it, _Value& __val) const
- { return *__it < __val; }
- };
-
- _GLIBCXX20_CONSTEXPR
- inline _Iter_less_val
- __iter_less_val()
- { return _Iter_less_val(); }
-
- _GLIBCXX20_CONSTEXPR
- inline _Iter_less_val
- __iter_comp_val(_Iter_less_iter)
- { return _Iter_less_val(); }
-
- struct _Val_less_iter
- {
-#if __cplusplus >= 201103L
-constexpr _Val_less_iter() = default;
-#else
-_Val_less_iter() { }
-#endif
-
-_GLIBCXX20_CONSTEXPR
-explicit
-_Val_less_iter(_Iter_less_iter) { }
-
-template
- _GLIBCXX20_CONSTEXPR
- bool
- operator()(_Value& __val, _Iterator __it) const
- { return __val < *__it; }
- };
+ inline _Less
+ __less()
+ { return _Less(); }
- _GLIBCXX20_CONSTEXPR
- inline _Val_less_iter
- __val_less_iter()
- { return _Val_less_iter(); }
-
- _GLIBCXX20_CONSTEXPR
- inline _Val_less_iter
- __val_comp_iter(_Iter_less_iter)
- { return _Val_less_iter(); }
-
- struct _Iter_equal_to_iter
+ struct _Equal_to
{
-template
+template
_GLIBCXX20_CONSTEXPR
bool
- operator()(_Iterator1 __it1, _Iterator2 __it2) const
- { return *__it1 == *__it2; }
+ operator()(const _Lhs& __lhs, const _Rhs& __rhs) const
+ { return __lhs == __rhs; }
};
_GLIBCXX20_CONSTEXPR
- inline _Iter_equal_to_iter
- __iter_equal_to_iter()
- { return _Iter_equal_to_iter(); }
-
- struct _Iter_equal_to_val
- {
-template
- _GLIBCXX20_CONSTEXPR
- bool
- operator()(_Iterator __it, _Value& __val) const
- { return *__it == __val; }
- };
+ inline _Equal_to
+ __equal_to()
+ { return _Equal_to(); }
- _GLIBCXX20_CONSTEXPR
- inline _Iter_equal_to_val
- __iter_equal_to_val()
- { return _Iter_equal_to_val(); }
-
- _GLIBCXX20_CONSTEXPR
-
[PATCH] Reimplement __gnu_cxx::__ops operators
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 , and .
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
diff --git a/libstdc++-v3/include/bits/predefined_ops.h b/libstdc++-v3/include/bits/predefined_ops.h
index e9933373ed9..dc8920ed5f8 100644
--- a/libstdc++-v3/include/bits/predefined_ops.h
+++ b/libstdc++-v3/include/bits/predefined_ops.h
@@ -32,376 +32,170 @@
#include
+#if __cplusplus >= 201103L
+# include
+#endif
+
namespace __gnu_cxx
{
namespace __ops
{
- struct _Iter_less_iter
+ struct _Less
{
-template
+template
_GLIBCXX14_CONSTEXPR
bool
- operator()(_Iterator1 __it1, _Iterator2 __it2) const
- { return *__it1 < *__it2; }
+ operator()(const _Lhs& __lhs, const _Rhs& __rhs) const
+ { return __lhs < __rhs; }
};
_GLIBCXX14_CONSTEXPR
- inline _Iter_less_iter
- __iter_less_iter()
- { return _Iter_less_iter(); }
-
- struct _Iter_less_val
- {
-#if __cplusplus >= 201103L
-constexpr _Iter_less_val() = default;
-#else
-_Iter_less_val() { }
-#endif
-
-_GLIBCXX20_CONSTEXPR
-explicit
-_Iter_less_val(_Iter_less_iter) { }
-
-template
- _GLIBCXX20_CONSTEXPR
- bool
- operator()(_Iterator __it, _Value& __val) const
- { return *__it < __val; }
- };
-
- _GLIBCXX20_CONSTEXPR
- inline _Iter_less_val
- __iter_less_val()
- { return _Iter_less_val(); }
-
- _GLIBCXX20_CONSTEXPR
- inline _Iter_less_val
- __iter_comp_val(_Iter_less_iter)
- { return _Iter_less_val(); }
-
- struct _Val_less_iter
- {
-#if __cplusplus >= 201103L
-constexpr _Val_less_iter() = default;
-#else
-_Val_less_iter() { }
-#endif
-
-_GLIBCXX20_CONSTEXPR
-explicit
-_Val_less_iter(_Iter_less_iter) { }
-
-template
- _GLIBCXX20_CONSTEXPR
- bool
- operator()(_Value& __val, _Iterator __it) const
- { return __val < *__it; }
- };
-
- _GLIBCXX20_CONSTEXPR
- inline _Val_less_iter
- __val_less_iter()
- { return _Val_less_iter(); }
-
- _GLIBCXX20_CONSTEXPR
- inline _Val_less_iter
- __val_comp_iter(_Iter_less_iter)
- { return _Val_less_iter(); }
-
- struct _Iter_equal_to_iter
- {
-template
- _GLIBCXX20_CONSTEXPR
- bool
- operator()(_Iterator1 __it1, _Iterator2 __it2) const
- { return *__it1 == *__it2; }
- };
-
- _GLIBCXX20_CONSTEXPR
- inline _Iter_equal_to_iter
- __iter_equal_to_iter()
- { return _Iter_equal_to_iter(); }
+ inline _Less
+ __less()
+ { return _Less(); }
- struct _Iter_equal_to_val
+ struct _Equal_to
{
-template
+template
_GLIBCXX20_CONSTEXPR
bool
- operator()(_Iterator __it, _Value& __val) const
- { return *__it == __val; }
+ operator()(const _Lhs& __lhs, const _Rhs& __rhs) const
+ { return __lhs == __rhs; }
};
_GLIBCXX20_CONSTEXPR
- inline _Iter_equal_to_val
- __iter_equal_to_val()
- { return _Iter_equal_to_val(); }
-
- _GLIBCXX20_CONSTEXPR
- inline _Iter_equal_to_val
- __iter_comp_val(_Iter_equal_to_iter)
- { return _Iter_equal_to_val(); }
-
- template
-struct _Iter_comp_iter
-{
- _Compare _M_comp;
-
- explicit
