* [PATCH] Replace __gnu_cxx::__ops::__negate with std::not_fn
@ 2023-05-22 20:50 François Dumont
2023-05-22 20:55 ` Jonathan Wakely
0 siblings, 1 reply; 3+ messages in thread
From: François Dumont @ 2023-05-22 20:50 UTC (permalink / raw)
To: libstdc++; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]
I was thinking that it might be nice to get rid of predefined_ops.h content.
So here is a start with __negate. Drawback is that stl_algo.h has to
include <functional>. For now I just get rid of stl_algo.h include in
<functional> to rather use stl_algobase.h. But maybe it would be better
to also isolate std::not_fn in a dedicated header file so that
stl_algo.h do not have to include all <functional>.
libstdc++: Replace __gnu_cxx::__ops::__negate with std::not_fn
Replace the internal __gnu_cxx::__ops::__negate function and associated
__gnu_cxx::__ops::_Iter_negate by the C++17 std::not_fn.
libstdc++-v3/ChangeLog:
* include/bits/predefined_ops.h: Include <version>.
[__cpp_lib_not_fn](__gnu_cxx::__ops::_Iter_negate): Remove.
[__cpp_lib_not_fn](__gnu_cxx::__ops::__negate): Remove.
* include/bits/stl_algo.h: Include <functional> for C++17
and later.
[__cpp_lib_not_fn](__find_if_not): Use std::not_fn.
(std::__search, std::search(_FwdIt1, _FwdIt1, _FwdIt2,
_FwdIt2, _BinPred)): Move...
* include/bits/stl_algobase.h: ...here.
* include/std/functional: Replace <stl_algo.h> include by
<stl_algobase.h>.
Tests still running.
François
[-- Attachment #2: predefined_ops.patch --]
[-- Type: text/x-patch, Size: 9636 bytes --]
diff --git a/libstdc++-v3/include/bits/predefined_ops.h b/libstdc++-v3/include/bits/predefined_ops.h
index e9933373ed9..8fdb11ea84b 100644
--- a/libstdc++-v3/include/bits/predefined_ops.h
+++ b/libstdc++-v3/include/bits/predefined_ops.h
@@ -30,6 +30,7 @@
#ifndef _GLIBCXX_PREDEFINED_OPS_H
#define _GLIBCXX_PREDEFINED_OPS_H 1
+#include <version>
#include <bits/move.h>
namespace __gnu_cxx
@@ -377,6 +378,7 @@ namespace __ops
_GLIBCXX_MOVE(__comp._M_comp), __it);
}
+#if !__cpp_lib_not_fn
template<typename _Predicate>
struct _Iter_negate
{
@@ -400,6 +402,7 @@ namespace __ops
inline _Iter_negate<_Predicate>
__negate(_Iter_pred<_Predicate> __pred)
{ return _Iter_negate<_Predicate>(_GLIBCXX_MOVE(__pred._M_pred)); }
+#endif
} // namespace __ops
} // namespace __gnu_cxx
diff --git a/libstdc++-v3/include/bits/stl_algo.h b/libstdc++-v3/include/bits/stl_algo.h
index 54695490166..849d8a59ec2 100644
--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -65,6 +65,10 @@
#include <bits/uniform_int_dist.h>
#endif
+#if __cplusplus >= 201703L
+#include <functional> // for std::not_fn.
+#endif
+
#if _GLIBCXX_HOSTED
# include <bits/stl_tempbuf.h> // for _Temporary_buffer
# if (__cplusplus <= 201103L || _GLIBCXX_USE_DEPRECATED)
@@ -110,7 +114,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Predicate __pred)
{
return std::__find_if(__first, __last,
+#if __cpp_lib_not_fn
+ std::not_fn(std::move(__pred)),
+#else
__gnu_cxx::__ops::__negate(__pred),
+#endif
std::__iterator_category(__first));
}
@@ -140,54 +148,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// count
// count_if
// search
-
- template<typename _ForwardIterator1, typename _ForwardIterator2,
- typename _BinaryPredicate>
- _GLIBCXX20_CONSTEXPR
- _ForwardIterator1
- __search(_ForwardIterator1 __first1, _ForwardIterator1 __last1,
- _ForwardIterator2 __first2, _ForwardIterator2 __last2,
- _BinaryPredicate __predicate)
- {
- // Test for empty ranges
- if (__first1 == __last1 || __first2 == __last2)
- return __first1;
-
- // Test for a pattern of length 1.
- _ForwardIterator2 __p1(__first2);
- if (++__p1 == __last2)
- return std::__find_if(__first1, __last1,
- __gnu_cxx::__ops::__iter_comp_iter(__predicate, __first2));
-
- // General case.
- _ForwardIterator1 __current = __first1;
-
- for (;;)
- {
- __first1 =
- std::__find_if(__first1, __last1,
- __gnu_cxx::__ops::__iter_comp_iter(__predicate, __first2));
-
- if (__first1 == __last1)
- return __last1;
-
- _ForwardIterator2 __p = __p1;
- __current = __first1;
- if (++__current == __last1)
- return __last1;
-
- while (__predicate(__current, __p))
- {
- if (++__p == __last2)
- return __first1;
- if (++__current == __last1)
- return __last1;
- }
- ++__first1;
- }
- return __first1;
- }
-
// search_n
/**
@@ -4147,48 +4107,6 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
__gnu_cxx::__ops::__iter_equal_to_iter());
}
- /**
- * @brief Search a sequence for a matching sub-sequence using a predicate.
- * @ingroup non_mutating_algorithms
- * @param __first1 A forward iterator.
- * @param __last1 A forward iterator.
- * @param __first2 A forward iterator.
- * @param __last2 A forward iterator.
- * @param __predicate A binary predicate.
- * @return The first iterator @c i in the range
- * @p [__first1,__last1-(__last2-__first2)) such that
- * @p __predicate(*(i+N),*(__first2+N)) is true for each @c N in the range
- * @p [0,__last2-__first2), or @p __last1 if no such iterator exists.
- *
- * Searches the range @p [__first1,__last1) for a sub-sequence that
- * compares equal value-by-value with the sequence given by @p
- * [__first2,__last2), using @p __predicate to determine equality,
- * and returns an iterator to the first element of the
- * sub-sequence, or @p __last1 if no such iterator exists.
- *
- * @see search(_ForwardIter1, _ForwardIter1, _ForwardIter2, _ForwardIter2)
- */
- template<typename _ForwardIterator1, typename _ForwardIterator2,
- typename _BinaryPredicate>
- _GLIBCXX20_CONSTEXPR
- inline _ForwardIterator1
- search(_ForwardIterator1 __first1, _ForwardIterator1 __last1,
- _ForwardIterator2 __first2, _ForwardIterator2 __last2,
- _BinaryPredicate __predicate)
- {
- // concept requirements
- __glibcxx_function_requires(_ForwardIteratorConcept<_ForwardIterator1>)
- __glibcxx_function_requires(_ForwardIteratorConcept<_ForwardIterator2>)
- __glibcxx_function_requires(_BinaryPredicateConcept<_BinaryPredicate,
- typename iterator_traits<_ForwardIterator1>::value_type,
- typename iterator_traits<_ForwardIterator2>::value_type>)
- __glibcxx_requires_valid_range(__first1, __last1);
- __glibcxx_requires_valid_range(__first2, __last2);
-
- return std::__search(__first1, __last1, __first2, __last2,
- __gnu_cxx::__ops::__iter_comp_iter(__predicate));
- }
-
/**
* @brief Search a sequence for a number of consecutive values.
* @ingroup non_mutating_algorithms
diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index 4a6f8195d98..dd95e94f7e9 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -2150,6 +2150,53 @@ _GLIBCXX_END_NAMESPACE_ALGO
return __result;
}
+ template<typename _ForwardIterator1, typename _ForwardIterator2,
+ typename _BinaryPredicate>
+ _GLIBCXX20_CONSTEXPR
+ _ForwardIterator1
+ __search(_ForwardIterator1 __first1, _ForwardIterator1 __last1,
+ _ForwardIterator2 __first2, _ForwardIterator2 __last2,
+ _BinaryPredicate __predicate)
+ {
+ // Test for empty ranges
+ if (__first1 == __last1 || __first2 == __last2)
+ return __first1;
+
+ // Test for a pattern of length 1.
+ _ForwardIterator2 __p1(__first2);
+ if (++__p1 == __last2)
+ return std::__find_if(__first1, __last1,
+ __gnu_cxx::__ops::__iter_comp_iter(__predicate, __first2));
+
+ // General case.
+ _ForwardIterator1 __current = __first1;
+
+ for (;;)
+ {
+ __first1 =
+ std::__find_if(__first1, __last1,
+ __gnu_cxx::__ops::__iter_comp_iter(__predicate, __first2));
+
+ if (__first1 == __last1)
+ return __last1;
+
+ _ForwardIterator2 __p = __p1;
+ __current = __first1;
+ if (++__current == __last1)
+ return __last1;
+
+ while (__predicate(__current, __p))
+ {
+ if (++__p == __last2)
+ return __first1;
+ if (++__current == __last1)
+ return __last1;
+ }
+ ++__first1;
+ }
+ return __first1;
+ }
+
#if __cplusplus >= 201103L
template<typename _ForwardIterator1, typename _ForwardIterator2,
typename _BinaryPredicate>
@@ -2220,6 +2267,51 @@ _GLIBCXX_END_NAMESPACE_ALGO
}
#endif // C++11
+_GLIBCXX_BEGIN_NAMESPACE_ALGO
+
+ /**
+ * @brief Search a sequence for a matching sub-sequence using a predicate.
+ * @ingroup non_mutating_algorithms
+ * @param __first1 A forward iterator.
+ * @param __last1 A forward iterator.
+ * @param __first2 A forward iterator.
+ * @param __last2 A forward iterator.
+ * @param __predicate A binary predicate.
+ * @return The first iterator @c i in the range
+ * @p [__first1,__last1-(__last2-__first2)) such that
+ * @p __predicate(*(i+N),*(__first2+N)) is true for each @c N in the range
+ * @p [0,__last2-__first2), or @p __last1 if no such iterator exists.
+ *
+ * Searches the range @p [__first1,__last1) for a sub-sequence that
+ * compares equal value-by-value with the sequence given by @p
+ * [__first2,__last2), using @p __predicate to determine equality,
+ * and returns an iterator to the first element of the
+ * sub-sequence, or @p __last1 if no such iterator exists.
+ *
+ * @see search(_ForwardIter1, _ForwardIter1, _ForwardIter2, _ForwardIter2)
+ */
+ template<typename _ForwardIterator1, typename _ForwardIterator2,
+ typename _BinaryPredicate>
+ _GLIBCXX20_CONSTEXPR
+ inline _ForwardIterator1
+ search(_ForwardIterator1 __first1, _ForwardIterator1 __last1,
+ _ForwardIterator2 __first2, _ForwardIterator2 __last2,
+ _BinaryPredicate __predicate)
+ {
+ // concept requirements
+ __glibcxx_function_requires(_ForwardIteratorConcept<_ForwardIterator1>)
+ __glibcxx_function_requires(_ForwardIteratorConcept<_ForwardIterator2>)
+ __glibcxx_function_requires(_BinaryPredicateConcept<_BinaryPredicate,
+ typename iterator_traits<_ForwardIterator1>::value_type,
+ typename iterator_traits<_ForwardIterator2>::value_type>)
+ __glibcxx_requires_valid_range(__first1, __last1);
+ __glibcxx_requires_valid_range(__first2, __last2);
+
+ return std::__search(__first1, __last1, __first2, __last2,
+ __gnu_cxx::__ops::__iter_comp_iter(__predicate));
+ }
+
+_GLIBCXX_END_NAMESPACE_ALGO
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace std
diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
index c7c6a5a7924..4a4b8b2b2e6 100644
--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -64,7 +64,7 @@
# include <vector>
# include <array>
# endif
-# include <bits/stl_algo.h> // std::search
+# include <bits/stl_algobase.h> // std::search
#endif
#if __cplusplus >= 202002L
# include <bits/ranges_cmp.h> // std::identity, ranges::equal_to etc.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Replace __gnu_cxx::__ops::__negate with std::not_fn
2023-05-22 20:50 [PATCH] Replace __gnu_cxx::__ops::__negate with std::not_fn François Dumont
@ 2023-05-22 20:55 ` Jonathan Wakely
2023-05-24 4:51 ` François Dumont
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Wakely @ 2023-05-22 20:55 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2136 bytes --]
On Mon, 22 May 2023 at 21:51, François Dumont via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:
> I was thinking that it might be nice to get rid of predefined_ops.h
> content.
>
> So here is a start with __negate. Drawback is that stl_algo.h has to
> include <functional>.
We definitely don't want that. std::not_fn could be move to its own header.
But I'm not sure this is a good change anyway, as we can't do it
unconditionally. Pre-C++17 code would still be using the predefined_ops.h
function objects, so we can't remove that code. And we'll get template
bloat from instantiating the algos twice, once with the old function
objects and once with std::not_fn.
> For now I just get rid of stl_algo.h include in
> <functional> to rather use stl_algobase.h. But maybe it would be better
> to also isolate std::not_fn in a dedicated header file so that
> stl_algo.h do not have to include all <functional>.
>
> libstdc++: Replace __gnu_cxx::__ops::__negate with std::not_fn
>
> Replace the internal __gnu_cxx::__ops::__negate function and
> associated
> __gnu_cxx::__ops::_Iter_negate by the C++17 std::not_fn.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/predefined_ops.h: Include <version>.
>
No, please don't include <version> anywhere. If you do that, it means
<functional> now defines every feature test macro in the entire library,
which makes it look like you can get smart pointers and ranges and
constexpr math all from <functional>.
> [__cpp_lib_not_fn](__gnu_cxx::__ops::_Iter_negate): Remove.
> [__cpp_lib_not_fn](__gnu_cxx::__ops::__negate): Remove.
> * include/bits/stl_algo.h: Include <functional> for C++17
> and later.
> [__cpp_lib_not_fn](__find_if_not): Use std::not_fn.
> (std::__search, std::search(_FwdIt1, _FwdIt1, _FwdIt2,
> _FwdIt2, _BinPred)): Move...
> * include/bits/stl_algobase.h: ...here.
> * include/std/functional: Replace <stl_algo.h> include by
> <stl_algobase.h>.
>
> Tests still running.
>
> François
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Replace __gnu_cxx::__ops::__negate with std::not_fn
2023-05-22 20:55 ` Jonathan Wakely
@ 2023-05-24 4:51 ` François Dumont
0 siblings, 0 replies; 3+ messages in thread
From: François Dumont @ 2023-05-24 4:51 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2111 bytes --]
On 22/05/2023 22:55, Jonathan Wakely wrote:
>
>
> On Mon, 22 May 2023 at 21:51, François Dumont via Libstdc++
> <libstdc++@gcc.gnu.org <mailto:libstdc%2B%2B@gcc.gnu.org>> wrote:
>
> I was thinking that it might be nice to get rid of
> predefined_ops.h content.
>
> So here is a start with __negate. Drawback is that stl_algo.h has to
> include <functional>.
>
>
> We definitely don't want that. std::not_fn could be move to its own
> header.
>
> But I'm not sure this is a good change anyway, as we can't do it
> unconditionally. Pre-C++17 code would still be using the
> predefined_ops.h function objects, so we can't remove that code. And
> we'll get template bloat from instantiating the algos twice, once with
> the old function objects and once with std::not_fn.
True, what do you advise then ? Should I just forget about it ?
Introduce a std::__not_fn for pre-C++17 ?
I am studying this last proposal, let me know if it is just impossible
or a waste of time.
>
> For now I just get rid of stl_algo.h include in
> <functional> to rather use stl_algobase.h. But maybe it would be
> better
> to also isolate std::not_fn in a dedicated header file so that
> stl_algo.h do not have to include all <functional>.
>
> libstdc++: Replace __gnu_cxx::__ops::__negate with std::not_fn
>
> Replace the internal __gnu_cxx::__ops::__negate function and
> associated
> __gnu_cxx::__ops::_Iter_negate by the C++17 std::not_fn.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/predefined_ops.h: Include <version>.
>
>
> No, please don't include <version> anywhere. If you do that, it means
> <functional> now defines every feature test macro in the entire
> library, which makes it look like you can get smart pointers and
> ranges and constexpr math all from <functional>.
Ok, I wasn't aware about the interest of <version>. I see now, limited
to user code.
I'm testing only the move of std::search to stl_algobase.h to avoid
stl_algo.h include in <functional>. I'll submit it later.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-05-24 4:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-22 20:50 [PATCH] Replace __gnu_cxx::__ops::__negate with std::not_fn François Dumont
2023-05-22 20:55 ` Jonathan Wakely
2023-05-24 4:51 ` François Dumont
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).