* [PATCH 1/3] libstdc++: Remove std::bind_front specialization for no bound args
@ 2023-09-12 1:08 Patrick Palka
2023-09-12 1:08 ` [PATCH 2/3] libstdc++: Fix std::bind_front perfect forwarding [PR111327] Patrick Palka
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Patrick Palka @ 2023-09-12 1:08 UTC (permalink / raw)
To: gcc-patches; +Cc: libstdc++, Patrick Palka
This specialization for the case of no bound args, added by
r13-4214-gcbd05ca5ab1231, seems to be mostly obsoleted by
r13-5033-ge2eab3c4edb6aa which added [[no_unique_address]] to the
main template's data members. And the compile time advantage of
avoiding an empty tuple and index_sequence seems minimal. Removing this
specialization also means we don't have to fix the PR111327 bug in
another place.
PR libstdc++/111327
libstdc++-v3/ChangeLog:
* include/std/functional (_Bind_front0): Remove.
(_Bind_front_t): Adjust.
---
libstdc++-v3/include/std/functional | 63 +----------------------------
1 file changed, 1 insertion(+), 62 deletions(-)
diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
index 60d4d1f3dd2..7d1b890bb4e 100644
--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -996,69 +996,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
[[no_unique_address]] std::tuple<_BoundArgs...> _M_bound_args;
};
- // Avoid the overhead of an empty tuple<> if there are no bound args.
- template<typename _Fd>
- struct _Bind_front0
- {
- static_assert(is_move_constructible_v<_Fd>);
-
- // First parameter is to ensure this constructor is never used
- // instead of the copy/move constructor.
- template<typename _Fn>
- explicit constexpr
- _Bind_front0(int, _Fn&& __fn)
- noexcept(is_nothrow_constructible_v<_Fd, _Fn>)
- : _M_fd(std::forward<_Fn>(__fn))
- { }
-
- _Bind_front0(const _Bind_front0&) = default;
- _Bind_front0(_Bind_front0&&) = default;
- _Bind_front0& operator=(const _Bind_front0&) = default;
- _Bind_front0& operator=(_Bind_front0&&) = default;
- ~_Bind_front0() = default;
-
- template<typename... _CallArgs>
- constexpr
- invoke_result_t<_Fd&, _CallArgs...>
- operator()(_CallArgs&&... __call_args) &
- noexcept(is_nothrow_invocable_v<_Fd&, _CallArgs...>)
- { return std::invoke(_M_fd, std::forward<_CallArgs>(__call_args)...); }
-
- template<typename... _CallArgs>
- constexpr
- invoke_result_t<const _Fd&, _CallArgs...>
- operator()(_CallArgs&&... __call_args) const &
- noexcept(is_nothrow_invocable_v<const _Fd&, _CallArgs...>)
- { return std::invoke(_M_fd, std::forward<_CallArgs>(__call_args)...); }
-
- template<typename... _CallArgs>
- constexpr
- invoke_result_t<_Fd, _CallArgs...>
- operator()(_CallArgs&&... __call_args) &&
- noexcept(is_nothrow_invocable_v<_Fd, _CallArgs...>)
- {
- return std::invoke(std::move(_M_fd),
- std::forward<_CallArgs>(__call_args)...);
- }
-
- template<typename... _CallArgs>
- constexpr
- invoke_result_t<const _Fd, _CallArgs...>
- operator()(_CallArgs&&... __call_args) const &&
- noexcept(is_nothrow_invocable_v<const _Fd, _CallArgs...>)
- {
- return std::invoke(std::move(_M_fd),
- std::forward<_CallArgs>(__call_args)...);
- }
-
- private:
- [[no_unique_address]] _Fd _M_fd;
- };
-
template<typename _Fn, typename... _Args>
- using _Bind_front_t
- = __conditional_t<sizeof...(_Args) == 0, _Bind_front0<decay_t<_Fn>>,
- _Bind_front<decay_t<_Fn>, decay_t<_Args>...>>;
+ using _Bind_front_t = _Bind_front<decay_t<_Fn>, decay_t<_Args>...>;
/** Create call wrapper by partial application of arguments to function.
*
--
2.42.0.158.g94e83dcf5b
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 2/3] libstdc++: Fix std::bind_front perfect forwarding [PR111327]
2023-09-12 1:08 [PATCH 1/3] libstdc++: Remove std::bind_front specialization for no bound args Patrick Palka
@ 2023-09-12 1:08 ` Patrick Palka
2023-09-12 13:16 ` Jonathan Wakely
2023-09-12 1:08 ` [PATCH 3/3] libstdc++: Fix std::not_fn " Patrick Palka
2023-09-12 12:46 ` [PATCH 1/3] libstdc++: Remove std::bind_front specialization for no bound args Patrick Palka
2 siblings, 1 reply; 7+ messages in thread
From: Patrick Palka @ 2023-09-12 1:08 UTC (permalink / raw)
To: gcc-patches; +Cc: libstdc++, Patrick Palka
In order to properly implement a perfect forwarding call wrapper
(before 'deducing this' at least) we need a total of 8 operator()
overloads, 4 main ones and 4 deleted ones for each const/ref qual pair,
as described in section 5.5 of P0847R6. Otherwise the wrapper may
not perfectly forward according to the value category and constness
of the wrapped object. This patch fixes this bug in std::bind_front.
PR libstdc++/111327
libstdc++-v3/ChangeLog:
* include/std/functional (_Bind_front::operator()): Add deleted
fallback overloads for each const/ref qualifier pair. Give the
main overloads dummy constraints to make them more specialized
than the deleted overloads.
* testsuite/20_util/function_objects/bind_front/111327.cc: New test.
---
libstdc++-v3/include/std/functional | 16 ++++++++
.../function_objects/bind_front/111327.cc | 41 +++++++++++++++++++
2 files changed, 57 insertions(+)
create mode 100644 libstdc++-v3/testsuite/20_util/function_objects/bind_front/111327.cc
diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
index 7d1b890bb4e..c50b9e4d365 100644
--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -938,6 +938,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
~_Bind_front() = default;
template<typename... _CallArgs>
+ requires true
constexpr
invoke_result_t<_Fd&, _BoundArgs&..., _CallArgs...>
operator()(_CallArgs&&... __call_args) &
@@ -948,6 +949,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename... _CallArgs>
+ requires true
constexpr
invoke_result_t<const _Fd&, const _BoundArgs&..., _CallArgs...>
operator()(_CallArgs&&... __call_args) const &
@@ -959,6 +961,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename... _CallArgs>
+ requires true
constexpr
invoke_result_t<_Fd, _BoundArgs..., _CallArgs...>
operator()(_CallArgs&&... __call_args) &&
@@ -969,6 +972,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename... _CallArgs>
+ requires true
constexpr
invoke_result_t<const _Fd, const _BoundArgs..., _CallArgs...>
operator()(_CallArgs&&... __call_args) const &&
@@ -979,6 +983,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
std::forward<_CallArgs>(__call_args)...);
}
+ template<typename... _CallArgs>
+ void operator()(_CallArgs&&...) & = delete;
+
+ template<typename... _CallArgs>
+ void operator()(_CallArgs&&...) const & = delete;
+
+ template<typename... _CallArgs>
+ void operator()(_CallArgs&&...) && = delete;
+
+ template<typename... _CallArgs>
+ void operator()(_CallArgs&&...) const && = delete;
+
private:
using _BoundIndices = index_sequence_for<_BoundArgs...>;
diff --git a/libstdc++-v3/testsuite/20_util/function_objects/bind_front/111327.cc b/libstdc++-v3/testsuite/20_util/function_objects/bind_front/111327.cc
new file mode 100644
index 00000000000..6eb51994476
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/function_objects/bind_front/111327.cc
@@ -0,0 +1,41 @@
+// PR libstdc++/111327 - std::bind_front doesn't perfectly forward according
+// to value category of the call wrapper object
+// { dg-options "-std=gnu++20" }
+// { dg-do compile { target c++20 } }
+
+#include <functional>
+#include <utility>
+
+struct F {
+ void operator()(...) & = delete;
+ void operator()(...) const &;
+};
+
+struct G {
+ void operator()(...) && = delete;
+ void operator()(...) const &&;
+};
+
+int main() {
+ auto f0 = std::bind_front(F{});
+ f0(); // { dg-error "deleted" }
+ std::move(f0)();
+ std::as_const(f0)();
+ std::move(std::as_const(f0))();
+
+ auto g0 = std::bind_front(G{});
+ g0(); // { dg-error "deleted" }
+ std::move(g0)(); // { dg-error "deleted" }
+ std::move(std::as_const(g0))();
+
+ auto f1 = std::bind_front(F{}, 42);
+ f1(); // { dg-error "deleted" }
+ std::move(f1)();
+ std::as_const(f1)();
+ std::move(std::as_const(f1))();
+
+ auto g1 = std::bind_front(G{}, 42);
+ g1(); // { dg-error "deleted" }
+ std::move(g1)(); // { dg-error "deleted" }
+ std::move(std::as_const(g1))();
+}
--
2.42.0.158.g94e83dcf5b
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/3] libstdc++: Fix std::bind_front perfect forwarding [PR111327]
2023-09-12 1:08 ` [PATCH 2/3] libstdc++: Fix std::bind_front perfect forwarding [PR111327] Patrick Palka
@ 2023-09-12 13:16 ` Jonathan Wakely
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2023-09-12 13:16 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, libstdc++
On Tue, 12 Sept 2023 at 02:09, Patrick Palka via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> In order to properly implement a perfect forwarding call wrapper
> (before 'deducing this' at least) we need a total of 8 operator()
> overloads, 4 main ones and 4 deleted ones for each const/ref qual pair,
> as described in section 5.5 of P0847R6. Otherwise the wrapper may
> not perfectly forward according to the value category and constness
> of the wrapped object. This patch fixes this bug in std::bind_front.
OK for trunk, thanks.
>
> PR libstdc++/111327
>
> libstdc++-v3/ChangeLog:
>
> * include/std/functional (_Bind_front::operator()): Add deleted
> fallback overloads for each const/ref qualifier pair. Give the
> main overloads dummy constraints to make them more specialized
> than the deleted overloads.
> * testsuite/20_util/function_objects/bind_front/111327.cc: New test.
> ---
> libstdc++-v3/include/std/functional | 16 ++++++++
> .../function_objects/bind_front/111327.cc | 41 +++++++++++++++++++
> 2 files changed, 57 insertions(+)
> create mode 100644 libstdc++-v3/testsuite/20_util/function_objects/bind_front/111327.cc
>
> diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
> index 7d1b890bb4e..c50b9e4d365 100644
> --- a/libstdc++-v3/include/std/functional
> +++ b/libstdc++-v3/include/std/functional
> @@ -938,6 +938,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> ~_Bind_front() = default;
>
> template<typename... _CallArgs>
> + requires true
> constexpr
> invoke_result_t<_Fd&, _BoundArgs&..., _CallArgs...>
> operator()(_CallArgs&&... __call_args) &
> @@ -948,6 +949,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> }
>
> template<typename... _CallArgs>
> + requires true
> constexpr
> invoke_result_t<const _Fd&, const _BoundArgs&..., _CallArgs...>
> operator()(_CallArgs&&... __call_args) const &
> @@ -959,6 +961,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> }
>
> template<typename... _CallArgs>
> + requires true
> constexpr
> invoke_result_t<_Fd, _BoundArgs..., _CallArgs...>
> operator()(_CallArgs&&... __call_args) &&
> @@ -969,6 +972,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> }
>
> template<typename... _CallArgs>
> + requires true
> constexpr
> invoke_result_t<const _Fd, const _BoundArgs..., _CallArgs...>
> operator()(_CallArgs&&... __call_args) const &&
> @@ -979,6 +983,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> std::forward<_CallArgs>(__call_args)...);
> }
>
> + template<typename... _CallArgs>
> + void operator()(_CallArgs&&...) & = delete;
> +
> + template<typename... _CallArgs>
> + void operator()(_CallArgs&&...) const & = delete;
> +
> + template<typename... _CallArgs>
> + void operator()(_CallArgs&&...) && = delete;
> +
> + template<typename... _CallArgs>
> + void operator()(_CallArgs&&...) const && = delete;
> +
> private:
> using _BoundIndices = index_sequence_for<_BoundArgs...>;
>
> diff --git a/libstdc++-v3/testsuite/20_util/function_objects/bind_front/111327.cc b/libstdc++-v3/testsuite/20_util/function_objects/bind_front/111327.cc
> new file mode 100644
> index 00000000000..6eb51994476
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/function_objects/bind_front/111327.cc
> @@ -0,0 +1,41 @@
> +// PR libstdc++/111327 - std::bind_front doesn't perfectly forward according
> +// to value category of the call wrapper object
> +// { dg-options "-std=gnu++20" }
> +// { dg-do compile { target c++20 } }
> +
> +#include <functional>
> +#include <utility>
> +
> +struct F {
> + void operator()(...) & = delete;
> + void operator()(...) const &;
> +};
> +
> +struct G {
> + void operator()(...) && = delete;
> + void operator()(...) const &&;
> +};
> +
> +int main() {
> + auto f0 = std::bind_front(F{});
> + f0(); // { dg-error "deleted" }
> + std::move(f0)();
> + std::as_const(f0)();
> + std::move(std::as_const(f0))();
> +
> + auto g0 = std::bind_front(G{});
> + g0(); // { dg-error "deleted" }
> + std::move(g0)(); // { dg-error "deleted" }
> + std::move(std::as_const(g0))();
> +
> + auto f1 = std::bind_front(F{}, 42);
> + f1(); // { dg-error "deleted" }
> + std::move(f1)();
> + std::as_const(f1)();
> + std::move(std::as_const(f1))();
> +
> + auto g1 = std::bind_front(G{}, 42);
> + g1(); // { dg-error "deleted" }
> + std::move(g1)(); // { dg-error "deleted" }
> + std::move(std::as_const(g1))();
> +}
> --
> 2.42.0.158.g94e83dcf5b
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] libstdc++: Fix std::not_fn perfect forwarding [PR111327]
2023-09-12 1:08 [PATCH 1/3] libstdc++: Remove std::bind_front specialization for no bound args Patrick Palka
2023-09-12 1:08 ` [PATCH 2/3] libstdc++: Fix std::bind_front perfect forwarding [PR111327] Patrick Palka
@ 2023-09-12 1:08 ` Patrick Palka
2023-09-12 13:16 ` Jonathan Wakely
2023-09-12 12:46 ` [PATCH 1/3] libstdc++: Remove std::bind_front specialization for no bound args Patrick Palka
2 siblings, 1 reply; 7+ messages in thread
From: Patrick Palka @ 2023-09-12 1:08 UTC (permalink / raw)
To: gcc-patches; +Cc: libstdc++, Patrick Palka
The previous patch fixed perfect forwarding for std::bind_front.
This patch fixes the same issue for std::not_fn.
Tested on x86_64-pc-linux-gnu, does this look OK for trunk and
perhaps 13?
PR libstdc++/111327
libstdc++-v3/ChangeLog:
* include/std/functional (_GLIBCXX_NOT_FN_CALL_OP): Also define
a deleted fallback operator() overload. Constrain both the
main and deleted overloads accordingly.
* testsuite/20_util/function_objects/not_fn/111327.cc: New test.
---
libstdc++-v3/include/std/functional | 10 +++++--
.../20_util/function_objects/not_fn/111327.cc | 29 +++++++++++++++++++
2 files changed, 37 insertions(+), 2 deletions(-)
create mode 100644 libstdc++-v3/testsuite/20_util/function_objects/not_fn/111327.cc
diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
index c50b9e4d365..9551e38dfdb 100644
--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -1061,7 +1061,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// forwarding _M_fn and the function arguments with the same qualifiers,
// and deducing the return type and exception-specification.
#define _GLIBCXX_NOT_FN_CALL_OP( _QUALS ) \
- template<typename... _Args> \
+ template<typename... _Args, \
+ typename = enable_if_t<__is_invocable<_Fn _QUALS, _Args...>::value>> \
_GLIBCXX20_CONSTEXPR \
decltype(_S_not<__inv_res_t<_Fn _QUALS, _Args...>>()) \
operator()(_Args&&... __args) _QUALS \
@@ -1070,7 +1071,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ \
return !std::__invoke(std::forward< _Fn _QUALS >(_M_fn), \
std::forward<_Args>(__args)...); \
- }
+ } \
+ \
+ template<typename... _Args, \
+ typename = enable_if_t<!__is_invocable<_Fn _QUALS, _Args...>::value>> \
+ void operator()(_Args&&... __args) _QUALS = delete;
+
_GLIBCXX_NOT_FN_CALL_OP( & )
_GLIBCXX_NOT_FN_CALL_OP( const & )
_GLIBCXX_NOT_FN_CALL_OP( && )
diff --git a/libstdc++-v3/testsuite/20_util/function_objects/not_fn/111327.cc b/libstdc++-v3/testsuite/20_util/function_objects/not_fn/111327.cc
new file mode 100644
index 00000000000..93e00ee8057
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/function_objects/not_fn/111327.cc
@@ -0,0 +1,29 @@
+// PR libstdc++/111327 - std::bind_front (and std::not_fn) doesn't perfectly
+// forward according to value category of the call wrapper object
+// { dg-do compile { target c++17 } }
+
+#include <functional>
+#include <utility>
+
+struct F {
+ void operator()(...) & = delete;
+ bool operator()(...) const &;
+};
+
+struct G {
+ void operator()(...) && = delete;
+ bool operator()(...) const &&;
+};
+
+int main() {
+ auto f = std::not_fn(F{});
+ f(); // { dg-error "deleted" }
+ std::move(f)();
+ std::as_const(f)();
+ std::move(std::as_const(f))();
+
+ auto g = std::not_fn(G{});
+ g(); // { dg-error "deleted" }
+ std::move(g)(); // { dg-error "deleted" }
+ std::move(std::as_const(g))();
+}
--
2.42.0.158.g94e83dcf5b
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] libstdc++: Fix std::not_fn perfect forwarding [PR111327]
2023-09-12 1:08 ` [PATCH 3/3] libstdc++: Fix std::not_fn " Patrick Palka
@ 2023-09-12 13:16 ` Jonathan Wakely
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2023-09-12 13:16 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, libstdc++
On Tue, 12 Sept 2023 at 02:11, Patrick Palka via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> The previous patch fixed perfect forwarding for std::bind_front.
> This patch fixes the same issue for std::not_fn.
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk and
> perhaps 13?
Yes for both, thanks.
>
> PR libstdc++/111327
>
> libstdc++-v3/ChangeLog:
>
> * include/std/functional (_GLIBCXX_NOT_FN_CALL_OP): Also define
> a deleted fallback operator() overload. Constrain both the
> main and deleted overloads accordingly.
> * testsuite/20_util/function_objects/not_fn/111327.cc: New test.
> ---
> libstdc++-v3/include/std/functional | 10 +++++--
> .../20_util/function_objects/not_fn/111327.cc | 29 +++++++++++++++++++
> 2 files changed, 37 insertions(+), 2 deletions(-)
> create mode 100644 libstdc++-v3/testsuite/20_util/function_objects/not_fn/111327.cc
>
> diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
> index c50b9e4d365..9551e38dfdb 100644
> --- a/libstdc++-v3/include/std/functional
> +++ b/libstdc++-v3/include/std/functional
> @@ -1061,7 +1061,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> // forwarding _M_fn and the function arguments with the same qualifiers,
> // and deducing the return type and exception-specification.
> #define _GLIBCXX_NOT_FN_CALL_OP( _QUALS ) \
> - template<typename... _Args> \
> + template<typename... _Args, \
> + typename = enable_if_t<__is_invocable<_Fn _QUALS, _Args...>::value>> \
> _GLIBCXX20_CONSTEXPR \
> decltype(_S_not<__inv_res_t<_Fn _QUALS, _Args...>>()) \
> operator()(_Args&&... __args) _QUALS \
> @@ -1070,7 +1071,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> { \
> return !std::__invoke(std::forward< _Fn _QUALS >(_M_fn), \
> std::forward<_Args>(__args)...); \
> - }
> + } \
> + \
> + template<typename... _Args, \
> + typename = enable_if_t<!__is_invocable<_Fn _QUALS, _Args...>::value>> \
> + void operator()(_Args&&... __args) _QUALS = delete;
> +
> _GLIBCXX_NOT_FN_CALL_OP( & )
> _GLIBCXX_NOT_FN_CALL_OP( const & )
> _GLIBCXX_NOT_FN_CALL_OP( && )
> diff --git a/libstdc++-v3/testsuite/20_util/function_objects/not_fn/111327.cc b/libstdc++-v3/testsuite/20_util/function_objects/not_fn/111327.cc
> new file mode 100644
> index 00000000000..93e00ee8057
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/function_objects/not_fn/111327.cc
> @@ -0,0 +1,29 @@
> +// PR libstdc++/111327 - std::bind_front (and std::not_fn) doesn't perfectly
> +// forward according to value category of the call wrapper object
> +// { dg-do compile { target c++17 } }
> +
> +#include <functional>
> +#include <utility>
> +
> +struct F {
> + void operator()(...) & = delete;
> + bool operator()(...) const &;
> +};
> +
> +struct G {
> + void operator()(...) && = delete;
> + bool operator()(...) const &&;
> +};
> +
> +int main() {
> + auto f = std::not_fn(F{});
> + f(); // { dg-error "deleted" }
> + std::move(f)();
> + std::as_const(f)();
> + std::move(std::as_const(f))();
> +
> + auto g = std::not_fn(G{});
> + g(); // { dg-error "deleted" }
> + std::move(g)(); // { dg-error "deleted" }
> + std::move(std::as_const(g))();
> +}
> --
> 2.42.0.158.g94e83dcf5b
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] libstdc++: Remove std::bind_front specialization for no bound args
2023-09-12 1:08 [PATCH 1/3] libstdc++: Remove std::bind_front specialization for no bound args Patrick Palka
2023-09-12 1:08 ` [PATCH 2/3] libstdc++: Fix std::bind_front perfect forwarding [PR111327] Patrick Palka
2023-09-12 1:08 ` [PATCH 3/3] libstdc++: Fix std::not_fn " Patrick Palka
@ 2023-09-12 12:46 ` Patrick Palka
2023-09-12 13:15 ` Jonathan Wakely
2 siblings, 1 reply; 7+ messages in thread
From: Patrick Palka @ 2023-09-12 12:46 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, libstdc++
On Mon, 11 Sep 2023, Patrick Palka wrote:
> This specialization for the case of no bound args, added by
> r13-4214-gcbd05ca5ab1231, seems to be mostly obsoleted by
> r13-5033-ge2eab3c4edb6aa which added [[no_unique_address]] to the
> main template's data members. And the compile time advantage of
> avoiding an empty tuple and index_sequence seems minimal. Removing this
> specialization also means we don't have to fix the PR111327 bug in
> another place.
FWIW I don't feel strongly about removing this specialization. If we
keep it We'd at least be able to reuse it for std::bind_back, and it
wouldn't be hard to fix the PR111327 bug in its implementation.
>
> PR libstdc++/111327
>
> libstdc++-v3/ChangeLog:
>
> * include/std/functional (_Bind_front0): Remove.
> (_Bind_front_t): Adjust.
> ---
> libstdc++-v3/include/std/functional | 63 +----------------------------
> 1 file changed, 1 insertion(+), 62 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
> index 60d4d1f3dd2..7d1b890bb4e 100644
> --- a/libstdc++-v3/include/std/functional
> +++ b/libstdc++-v3/include/std/functional
> @@ -996,69 +996,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> [[no_unique_address]] std::tuple<_BoundArgs...> _M_bound_args;
> };
>
> - // Avoid the overhead of an empty tuple<> if there are no bound args.
> - template<typename _Fd>
> - struct _Bind_front0
> - {
> - static_assert(is_move_constructible_v<_Fd>);
> -
> - // First parameter is to ensure this constructor is never used
> - // instead of the copy/move constructor.
> - template<typename _Fn>
> - explicit constexpr
> - _Bind_front0(int, _Fn&& __fn)
> - noexcept(is_nothrow_constructible_v<_Fd, _Fn>)
> - : _M_fd(std::forward<_Fn>(__fn))
> - { }
> -
> - _Bind_front0(const _Bind_front0&) = default;
> - _Bind_front0(_Bind_front0&&) = default;
> - _Bind_front0& operator=(const _Bind_front0&) = default;
> - _Bind_front0& operator=(_Bind_front0&&) = default;
> - ~_Bind_front0() = default;
> -
> - template<typename... _CallArgs>
> - constexpr
> - invoke_result_t<_Fd&, _CallArgs...>
> - operator()(_CallArgs&&... __call_args) &
> - noexcept(is_nothrow_invocable_v<_Fd&, _CallArgs...>)
> - { return std::invoke(_M_fd, std::forward<_CallArgs>(__call_args)...); }
> -
> - template<typename... _CallArgs>
> - constexpr
> - invoke_result_t<const _Fd&, _CallArgs...>
> - operator()(_CallArgs&&... __call_args) const &
> - noexcept(is_nothrow_invocable_v<const _Fd&, _CallArgs...>)
> - { return std::invoke(_M_fd, std::forward<_CallArgs>(__call_args)...); }
> -
> - template<typename... _CallArgs>
> - constexpr
> - invoke_result_t<_Fd, _CallArgs...>
> - operator()(_CallArgs&&... __call_args) &&
> - noexcept(is_nothrow_invocable_v<_Fd, _CallArgs...>)
> - {
> - return std::invoke(std::move(_M_fd),
> - std::forward<_CallArgs>(__call_args)...);
> - }
> -
> - template<typename... _CallArgs>
> - constexpr
> - invoke_result_t<const _Fd, _CallArgs...>
> - operator()(_CallArgs&&... __call_args) const &&
> - noexcept(is_nothrow_invocable_v<const _Fd, _CallArgs...>)
> - {
> - return std::invoke(std::move(_M_fd),
> - std::forward<_CallArgs>(__call_args)...);
> - }
> -
> - private:
> - [[no_unique_address]] _Fd _M_fd;
> - };
> -
> template<typename _Fn, typename... _Args>
> - using _Bind_front_t
> - = __conditional_t<sizeof...(_Args) == 0, _Bind_front0<decay_t<_Fn>>,
> - _Bind_front<decay_t<_Fn>, decay_t<_Args>...>>;
> + using _Bind_front_t = _Bind_front<decay_t<_Fn>, decay_t<_Args>...>;
>
> /** Create call wrapper by partial application of arguments to function.
> *
> --
> 2.42.0.158.g94e83dcf5b
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] libstdc++: Remove std::bind_front specialization for no bound args
2023-09-12 12:46 ` [PATCH 1/3] libstdc++: Remove std::bind_front specialization for no bound args Patrick Palka
@ 2023-09-12 13:15 ` Jonathan Wakely
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2023-09-12 13:15 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, libstdc++
On Tue, 12 Sept 2023 at 13:46, Patrick Palka via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> On Mon, 11 Sep 2023, Patrick Palka wrote:
>
> > This specialization for the case of no bound args, added by
> > r13-4214-gcbd05ca5ab1231, seems to be mostly obsoleted by
> > r13-5033-ge2eab3c4edb6aa which added [[no_unique_address]] to the
> > main template's data members. And the compile time advantage of
> > avoiding an empty tuple and index_sequence seems minimal. Removing this
> > specialization also means we don't have to fix the PR111327 bug in
> > another place.
>
> FWIW I don't feel strongly about removing this specialization. If we
> keep it We'd at least be able to reuse it for std::bind_back, and it
> wouldn't be hard to fix the PR111327 bug in its implementation.
Yeah, I'm ambivalent. But since you've got a patch to fix 111327 ready
which doesn't include this specialization, let's remove it.
The empty std::tuple is at least already explicitly specialized, so I
agree its overhead probably isn't very significant.
OK for trunk. I'm not sure if we should change it in gcc-13 now though.
>
> >
> > PR libstdc++/111327
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/std/functional (_Bind_front0): Remove.
> > (_Bind_front_t): Adjust.
> > ---
> > libstdc++-v3/include/std/functional | 63 +----------------------------
> > 1 file changed, 1 insertion(+), 62 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
> > index 60d4d1f3dd2..7d1b890bb4e 100644
> > --- a/libstdc++-v3/include/std/functional
> > +++ b/libstdc++-v3/include/std/functional
> > @@ -996,69 +996,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > [[no_unique_address]] std::tuple<_BoundArgs...> _M_bound_args;
> > };
> >
> > - // Avoid the overhead of an empty tuple<> if there are no bound args.
> > - template<typename _Fd>
> > - struct _Bind_front0
> > - {
> > - static_assert(is_move_constructible_v<_Fd>);
> > -
> > - // First parameter is to ensure this constructor is never used
> > - // instead of the copy/move constructor.
> > - template<typename _Fn>
> > - explicit constexpr
> > - _Bind_front0(int, _Fn&& __fn)
> > - noexcept(is_nothrow_constructible_v<_Fd, _Fn>)
> > - : _M_fd(std::forward<_Fn>(__fn))
> > - { }
> > -
> > - _Bind_front0(const _Bind_front0&) = default;
> > - _Bind_front0(_Bind_front0&&) = default;
> > - _Bind_front0& operator=(const _Bind_front0&) = default;
> > - _Bind_front0& operator=(_Bind_front0&&) = default;
> > - ~_Bind_front0() = default;
> > -
> > - template<typename... _CallArgs>
> > - constexpr
> > - invoke_result_t<_Fd&, _CallArgs...>
> > - operator()(_CallArgs&&... __call_args) &
> > - noexcept(is_nothrow_invocable_v<_Fd&, _CallArgs...>)
> > - { return std::invoke(_M_fd, std::forward<_CallArgs>(__call_args)...); }
> > -
> > - template<typename... _CallArgs>
> > - constexpr
> > - invoke_result_t<const _Fd&, _CallArgs...>
> > - operator()(_CallArgs&&... __call_args) const &
> > - noexcept(is_nothrow_invocable_v<const _Fd&, _CallArgs...>)
> > - { return std::invoke(_M_fd, std::forward<_CallArgs>(__call_args)...); }
> > -
> > - template<typename... _CallArgs>
> > - constexpr
> > - invoke_result_t<_Fd, _CallArgs...>
> > - operator()(_CallArgs&&... __call_args) &&
> > - noexcept(is_nothrow_invocable_v<_Fd, _CallArgs...>)
> > - {
> > - return std::invoke(std::move(_M_fd),
> > - std::forward<_CallArgs>(__call_args)...);
> > - }
> > -
> > - template<typename... _CallArgs>
> > - constexpr
> > - invoke_result_t<const _Fd, _CallArgs...>
> > - operator()(_CallArgs&&... __call_args) const &&
> > - noexcept(is_nothrow_invocable_v<const _Fd, _CallArgs...>)
> > - {
> > - return std::invoke(std::move(_M_fd),
> > - std::forward<_CallArgs>(__call_args)...);
> > - }
> > -
> > - private:
> > - [[no_unique_address]] _Fd _M_fd;
> > - };
> > -
> > template<typename _Fn, typename... _Args>
> > - using _Bind_front_t
> > - = __conditional_t<sizeof...(_Args) == 0, _Bind_front0<decay_t<_Fn>>,
> > - _Bind_front<decay_t<_Fn>, decay_t<_Args>...>>;
> > + using _Bind_front_t = _Bind_front<decay_t<_Fn>, decay_t<_Args>...>;
> >
> > /** Create call wrapper by partial application of arguments to function.
> > *
> > --
> > 2.42.0.158.g94e83dcf5b
> >
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-09-12 13:16 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-12 1:08 [PATCH 1/3] libstdc++: Remove std::bind_front specialization for no bound args Patrick Palka
2023-09-12 1:08 ` [PATCH 2/3] libstdc++: Fix std::bind_front perfect forwarding [PR111327] Patrick Palka
2023-09-12 13:16 ` Jonathan Wakely
2023-09-12 1:08 ` [PATCH 3/3] libstdc++: Fix std::not_fn " Patrick Palka
2023-09-12 13:16 ` Jonathan Wakely
2023-09-12 12:46 ` [PATCH 1/3] libstdc++: Remove std::bind_front specialization for no bound args Patrick Palka
2023-09-12 13:15 ` Jonathan Wakely
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).