On Wed, 14 May 2025 at 13:59, Tomasz Kaminski <tkami...@redhat.com> wrote:
>
>
> On Wed, May 14, 2025 at 1:06 PM Jonathan Wakely <jwak...@redhat.com> wrote:
>>
>> On 14/05/25 10:48 +0200, Tomasz Kamiński wrote:
>> >This patch implements C++26 copyable_function as specified in P2548R6.
>> >It also implements LWG 4255 that adjust move_only_function so constructing
>> >from empty copyable_function, produces empty functor. This falls from
>> >existing checks, after specializing __is_polymorphic_function_v for
>> >copyable_function specializations.
>> >
>> >For compatible invoker signatures, the move_only_function may be constructed
>> >from copyable_funciton without double indirection. To achieve that we derive
>> >_Cpy_base from _Mo_base, and specialize __is_polymorphic_function_v for
>> >copyable_function. Similary copyable_functions with compatible signatures
>> >can be converted without double indirection.
>> >
>> >As we starting to use _Op::_Copy operation from the _M_manage function,
>> >invocations of that functions may now throw exceptions, so noexcept needs
>> >to be removed from the signature of stored _M_manage pointers. This also
>> >affects operations in _Mo_base, however we already wrap _M_manage
>> >invocations
>> >in noexcept member functions (_M_move, _M_destroy, swap).
>> >
>> > PR libstdc++/119125
>> >
>> >libstdc++-v3/ChangeLog:
>> >
>> > * doc/doxygen/stdheader.cc: Addded cpyfunc_impl.h header.
>> > * include/Makefile.am: Add bits cpyfunc_impl.h.
>> > * include/Makefile.in: Add bits cpyfunc_impl.h.
>> > * include/bits/cpyfunc_impl.h: New file.
>> > * include/bits/mofunc_impl.h: Mention LWG 4255.
>> > * include/bits/move_only_function.h: Update header description
>> > and change guard to __cplusplus > 202002L.
>> > (_Manager::_Func): Remove noexcept.
>> > (std::__is_polymorphic_function_v<move_only_function<_Tp>>)
>> >
>> > (__variant::_Never_valueless_alt<std::move_only_function<_Signature...>>)
>> > (move_only_function) [__glibcxx_move_only_function]: Adjust guard.
>> > (std::__is_polymorphic_function_v<copyable_function<_Tp>>)
>> >
>> > (__variant::_Never_valueless_alt<std::copyable_function<_Signature...>>)
>> > (__polyfunc::_Cpy_base, std::copyable_function)
>> > [__glibcxx_copyable_function]: Define.
>> > * include/bits/version.def: Define copyable_function.
>> > * include/bits/version.h: Regenerate.
>> > * include/std/functional: Define __cpp_lib_copyable_function.
>> > * src/c++23/std.cc.in (copyable_function)
>> > [__cpp_lib_copyable_function]: Export.
>> > * testsuite/20_util/copyable_function/call.cc: New test based on
>> > move_only_function tests.
>> > * testsuite/20_util/copyable_function/cons.cc: New test based on
>> > move_only_function tests.
>> > * testsuite/20_util/copyable_function/conv.cc: New test based on
>> > move_only_function tests.
>> > * testsuite/20_util/copyable_function/copy.cc: New test.
>> > * testsuite/20_util/copyable_function/move.cc: New test based on
>> > move_only_function tests.
>> >---
>> >In addition to fixing formatting and typos, this patch adds export of
>> >the copyable_function to std module.
>> >
>> > libstdc++-v3/doc/doxygen/stdheader.cc | 1 +
>> > libstdc++-v3/include/Makefile.am | 1 +
>> > libstdc++-v3/include/Makefile.in | 1 +
>> > libstdc++-v3/include/bits/cpyfunc_impl.h | 269 ++++++++++++++++++
>> > libstdc++-v3/include/bits/mofunc_impl.h | 4 +
>> > .../include/bits/move_only_function.h | 94 +++++-
>> > libstdc++-v3/include/bits/version.def | 10 +
>> > libstdc++-v3/include/bits/version.h | 10 +
>> > libstdc++-v3/include/std/functional | 1 +
>> > libstdc++-v3/src/c++23/std.cc.in | 3 +
>> > .../20_util/copyable_function/call.cc | 224 +++++++++++++++
>> > .../20_util/copyable_function/cons.cc | 126 ++++++++
>> > .../20_util/copyable_function/conv.cc | 251 ++++++++++++++++
>> > .../20_util/copyable_function/copy.cc | 154 ++++++++++
>> > .../20_util/copyable_function/move.cc | 120 ++++++++
>> > 15 files changed, 1264 insertions(+), 5 deletions(-)
>> > create mode 100644 libstdc++-v3/include/bits/cpyfunc_impl.h
>> > create mode 100644 libstdc++-v3/testsuite/20_util/copyable_function/call.cc
>> > create mode 100644 libstdc++-v3/testsuite/20_util/copyable_function/cons.cc
>> > create mode 100644 libstdc++-v3/testsuite/20_util/copyable_function/conv.cc
>> > create mode 100644 libstdc++-v3/testsuite/20_util/copyable_function/copy.cc
>> > create mode 100644 libstdc++-v3/testsuite/20_util/copyable_function/move.cc
>> >
>> >diff --git a/libstdc++-v3/doc/doxygen/stdheader.cc
>> >b/libstdc++-v3/doc/doxygen/stdheader.cc
>> >index 3ee825feb66..8a201334410 100644
>> >--- a/libstdc++-v3/doc/doxygen/stdheader.cc
>> >+++ b/libstdc++-v3/doc/doxygen/stdheader.cc
>> >@@ -54,6 +54,7 @@ void init_map()
>> > headers["function.h"] = "functional";
>> > headers["functional_hash.h"] = "functional";
>> > headers["mofunc_impl.h"] = "functional";
>> >+ headers["cpyfunc_impl.h"] = "functional";
>> > headers["move_only_function.h"] = "functional";
>> > headers["invoke.h"] = "functional";
>> > headers["ranges_cmp.h"] = "functional";
>> >diff --git a/libstdc++-v3/include/Makefile.am
>> >b/libstdc++-v3/include/Makefile.am
>> >index 1140fa0dffd..5cc13381b02 100644
>> >--- a/libstdc++-v3/include/Makefile.am
>> >+++ b/libstdc++-v3/include/Makefile.am
>> >@@ -194,6 +194,7 @@ bits_headers = \
>> > ${bits_srcdir}/chrono_io.h \
>> > ${bits_srcdir}/codecvt.h \
>> > ${bits_srcdir}/cow_string.h \
>> >+ ${bits_srcdir}/cpyfunc_impl.h \
>> > ${bits_srcdir}/deque.tcc \
>> > ${bits_srcdir}/erase_if.h \
>> > ${bits_srcdir}/formatfwd.h \
>> >diff --git a/libstdc++-v3/include/Makefile.in
>> >b/libstdc++-v3/include/Makefile.in
>> >index c96e981acd6..6e5e97aa236 100644
>> >--- a/libstdc++-v3/include/Makefile.in
>> >+++ b/libstdc++-v3/include/Makefile.in
>> >@@ -547,6 +547,7 @@ bits_freestanding = \
>> > @GLIBCXX_HOSTED_TRUE@ ${bits_srcdir}/chrono_io.h \
>> > @GLIBCXX_HOSTED_TRUE@ ${bits_srcdir}/codecvt.h \
>> > @GLIBCXX_HOSTED_TRUE@ ${bits_srcdir}/cow_string.h \
>> >+@GLIBCXX_HOSTED_TRUE@ ${bits_srcdir}/cpyfunc_impl.h \
>> > @GLIBCXX_HOSTED_TRUE@ ${bits_srcdir}/deque.tcc \
>> > @GLIBCXX_HOSTED_TRUE@ ${bits_srcdir}/erase_if.h \
>> > @GLIBCXX_HOSTED_TRUE@ ${bits_srcdir}/formatfwd.h \
>> >diff --git a/libstdc++-v3/include/bits/cpyfunc_impl.h
>> >b/libstdc++-v3/include/bits/cpyfunc_impl.h
>> >new file mode 100644
>> >index 00000000000..90a6938ae24
>> >--- /dev/null
>> >+++ b/libstdc++-v3/include/bits/cpyfunc_impl.h
>> >@@ -0,0 +1,269 @@
>> >+// Implementation of std::copyable_function -*- C++ -*-
>> >+
>> >+// Copyright The GNU Toolchain Authors.
>> >+//
>> >+// This file is part of the GNU ISO C++ Library. This library is free
>> >+// software; you can redistribute it and/or modify it under the
>> >+// terms of the GNU General Public License as published by the
>> >+// Free Software Foundation; either version 3, or (at your option)
>> >+// any later version.
>> >+
>> >+// This library is distributed in the hope that it will be useful,
>> >+// but WITHOUT ANY WARRANTY; without even the implied warranty of
>> >+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> >+// GNU General Public License for more details.
>> >+
>> >+// Under Section 7 of GPL version 3, you are granted additional
>> >+// permissions described in the GCC Runtime Library Exception, version
>> >+// 3.1, as published by the Free Software Foundation.
>> >+
>> >+// You should have received a copy of the GNU General Public License and
>> >+// a copy of the GCC Runtime Library Exception along with this program;
>> >+// see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
>> >+// <http://www.gnu.org/licenses/>.
>> >+
>> >+/** @file include/bits/cpyfunc_impl.h
>> >+ * This is an internal header file, included by other library headers.
>> >+ * Do not attempt to use it directly. @headername{functional}
>> >+ */
>> >+
>> >+#ifndef _GLIBCXX_MOF_CV
>> >+# define _GLIBCXX_MOF_CV
>> >+#endif
>> >+
>> >+#ifdef _GLIBCXX_MOF_REF
>> >+# define _GLIBCXX_MOF_INV_QUALS _GLIBCXX_MOF_CV _GLIBCXX_MOF_REF
>> >+#else
>> >+# define _GLIBCXX_MOF_REF
>> >+# define _GLIBCXX_MOF_INV_QUALS _GLIBCXX_MOF_CV &
>> >+#endif
>> >+
>> >+#define _GLIBCXX_MOF_CV_REF _GLIBCXX_MOF_CV _GLIBCXX_MOF_REF
>> >+
>> >+namespace std _GLIBCXX_VISIBILITY(default)
>> >+{
>> >+_GLIBCXX_BEGIN_NAMESPACE_VERSION
>> >+
>> >+ /**
>> >+ * @brief Polymorphic copyable function wrapper.
>> >+ * @ingroup functors
>> >+ * @since C++26
>> >+ * @headerfile functional
>> >+ *
>> >+ * The `std::copyable_function` class template is a call wrapper similar
>> >+ * to `std::function`, but it does not provide information about target,
>>
>> s/target/its target/
>>
>> >+ * and preserves constness.
>> >+ *
>> >+ * It also supports const-qualification, ref-qualification, and
>> >+ * no-throw guarantees. The qualifications and exception-specification
>> >+ * of the `copyable_function::operator()` member function are respected
>> >+ * when invoking the target function.
>> >+ */
>> >+ template<typename _Res, typename... _ArgTypes, bool _Noex>
>> >+ class copyable_function<_Res(_ArgTypes...) _GLIBCXX_MOF_CV
>> >+ _GLIBCXX_MOF_REF noexcept(_Noex)>
>> >+ : __polyfunc::_Cpy_base
>> >+ {
>> >+ using _Base = __polyfunc::_Cpy_base;
>> >+ using _Invoker = __polyfunc::_Invoker<_Noex, _Res, _ArgTypes...>;
>> >+ using _Signature = _Invoker::_Signature;
>> >+
>> >+ template<typename _Tp>
>> >+ using __callable
>> >+ = __conditional_t<_Noex,
>> >+ is_nothrow_invocable_r<_Res, _Tp, _ArgTypes...>,
>> >+ is_invocable_r<_Res, _Tp, _ArgTypes...>>;
>> >+
>> >+ // [func.wrap.copy.con]/1 is-callable-from<VT>
>> >+ template<typename _Vt>
>> >+ static constexpr bool __is_callable_from
>> >+ = __and_v<__callable<_Vt _GLIBCXX_MOF_CV_REF>,
>> >+ __callable<_Vt _GLIBCXX_MOF_INV_QUALS>>;
>> >+
>> >+ public:
>> >+ using result_type = _Res;
>> >+
>> >+ /// Creates an empty object.
>> >+ copyable_function() noexcept { }
>> >+
>> >+ /// Creates an empty object.
>> >+ copyable_function(nullptr_t) noexcept { }
>> >+
>> >+ /// Moves the target object, leaving the source empty.
>> >+ copyable_function(copyable_function&& __x) noexcept
>> >+ : _Base(static_cast<_Base&&>(__x)),
>> >+ _M_invoke(std::__exchange(__x._M_invoke, nullptr))
>> >+ { }
>> >+
>> >+ /// Copies the target object.
>> >+ copyable_function(copyable_function const& __x)
>> >+ : _Base(static_cast<const _Base&>(__x)),
>> >+ _M_invoke(__x._M_invoke)
>> >+ { }
>> >+
>> >+ /// Stores a target object initialized from the argument.
>> >+ template<typename _Fn, typename _Vt = decay_t<_Fn>>
>> >+ requires (!is_same_v<_Vt, copyable_function>)
>> >+ && (!__is_in_place_type_v<_Vt>) && __is_callable_from<_Vt>
>> >+ copyable_function(_Fn&& __f) noexcept(_S_nothrow_init<_Vt, _Fn>())
>> >+ {
>> >+ static_assert(is_copy_constructible_v<_Vt>);
>> >+ if constexpr (is_function_v<remove_pointer_t<_Vt>>
>> >+ || is_member_pointer_v<_Vt>
>> >+ || __is_polymorphic_function_v<_Vt>)
>> >+ {
>> >+ if (__f == nullptr)
>> >+ return;
>> >+ }
>> >+
>> >+ if constexpr (!__is_polymorphic_function_v<_Vt>
>> >+ || !__polyfunc::__is_invoker_convertible<_Vt,
>> >copyable_function>())
>> >+ {
>> >+ _M_init<_Vt>(std::forward<_Fn>(__f));
>> >+ _M_invoke = _Invoker::template _S_storage<_Vt
>> >_GLIBCXX_MOF_INV_QUALS>();
>> >+ }
>> >+ else if constexpr (is_lvalue_reference_v<_Fn>)
>> >+ {
>> >+ _M_copy(__polyfunc::__base_of(__f));
>> >+ _M_invoke = __polyfunc::__invoker_of(__f);
>> >+ }
>> >+ else
>> >+ {
>> >+ _M_move(__polyfunc::__base_of(__f));
>> >+ _M_invoke = std::__exchange(__polyfunc::__invoker_of(__f),
>> >nullptr);
>> >+ }
>> >+ }
>> >+
>> >+ /// Stores a target object initialized from the arguments.
>> >+ template<typename _Tp, typename... _Args>
>> >+ requires is_constructible_v<_Tp, _Args...>
>> >+ && __is_callable_from<_Tp>
>> >+ explicit
>> >+ copyable_function(in_place_type_t<_Tp>, _Args&&... __args)
>> >+ noexcept(_S_nothrow_init<_Tp, _Args...>())
>> >+ : _M_invoke(_Invoker::template _S_storage<_Tp
>> >_GLIBCXX_MOF_INV_QUALS>())
>> >+ {
>> >+ static_assert(is_same_v<decay_t<_Tp>, _Tp>);
>> >+ static_assert(is_copy_constructible_v<_Tp>);
>> >+ _M_init<_Tp>(std::forward<_Args>(__args)...);
>> >+ }
>> >+
>> >+ /// Stores a target object initialized from the arguments.
>> >+ template<typename _Tp, typename _Up, typename... _Args>
>> >+ requires is_constructible_v<_Tp, initializer_list<_Up>&, _Args...>
>> >+ && __is_callable_from<_Tp>
>> >+ explicit
>> >+ copyable_function(in_place_type_t<_Tp>, initializer_list<_Up> __il,
>> >+ _Args&&... __args)
>> >+ noexcept(_S_nothrow_init<_Tp, initializer_list<_Up>&, _Args...>())
>> >+ : _M_invoke(_Invoker::template _S_storage<_Tp
>> >_GLIBCXX_MOF_INV_QUALS>())
>> >+ {
>> >+ static_assert(is_same_v<decay_t<_Tp>, _Tp>);
>> >+ static_assert(is_copy_constructible_v<_Tp>);
>> >+ _M_init<_Tp>(__il, std::forward<_Args>(__args)...);
>> >+ }
>> >+
>> >+ /// Stores a new target object, leaving `x` empty.
>> >+ copyable_function&
>> >+ operator=(copyable_function&& __x) noexcept
>> >+ {
>> >+ // Standard requires support of self assigment, by specifying it as
>> >+ // copy and swap.
>> >+ if (this != addressof(__x)) [[likely]]
>>
>> Qualify as std::addressof
>>
>> This is the only important change, the comments below aren't necessary
>> to act on.
>>
>> OK for trunk with the comment change noted above and the
>> std::addressof change here.
>>
>> >+ {
>> >+ _Base::operator=(static_cast<_Base&&>(__x));
>> >+ _M_invoke = std::__exchange(__x._M_invoke, nullptr);
>> >+ }
>> >+ return *this;
>> >+ }
>> >+
>> >+ /// Stores a copy of the source target object
>> >+ copyable_function&
>> >+ operator=(const copyable_function& __x)
>> >+ {
>> >+ copyable_function(__x).swap(*this);
>> >+ return *this;
>> >+ }
>> >+
>> >+ /// Destroys the target object (if any).
>> >+ copyable_function&
>> >+ operator=(nullptr_t) noexcept
>> >+ {
>> >+ _M_reset();
>> >+ _M_invoke = nullptr;
>> >+ return *this;
>> >+ }
>> >+
>> >+ /// Stores a new target object, initialized from the argument.
>> >+ template<typename _Fn>
>> >+ requires is_constructible_v<copyable_function, _Fn>
>> >+ copyable_function&
>> >+ operator=(_Fn&& __f)
>> >+ noexcept(is_nothrow_constructible_v<copyable_function, _Fn>)
>> >+ {
>> >+ copyable_function(std::forward<_Fn>(__f)).swap(*this);
>> >+ return *this;
>> >+ }
>> >+
>> >+ ~copyable_function() = default;
>> >+
>> >+ /// True if a target object is present, false otherwise.
>> >+ explicit operator bool() const noexcept
>> >+ { return _M_invoke != nullptr; }
>> >+
>> >+ /** Invoke the target object.
>> >+ *
>> >+ * The target object will be invoked using the supplied arguments,
>> >+ * and as an lvalue or rvalue, and as const or non-const, as dictated
>> >+ * by the template arguments of the `copyable_function`
>> >specialization.
>> >+ *
>> >+ * @pre Must not be empty.
>> >+ */
>> >+ _Res
>> >+ operator()(_ArgTypes... __args) _GLIBCXX_MOF_CV_REF noexcept(_Noex)
>> >+ {
>> >+ __glibcxx_assert(*this != nullptr);
>> >+ return _M_invoke(this->_M_storage,
>> >std::forward<_ArgTypes>(__args)...);
>> >+ }
>> >+
>> >+ /// Exchange the target objects (if any).
>> >+ void
>> >+ swap(copyable_function& __x) noexcept
>> >+ {
>> >+ _Base::swap(__x);
>> >+ std::swap(_M_invoke, __x._M_invoke);
>> >+ }
>> >+
>> >+ /// Exchange the target objects (if any).
>> >+ friend void
>> >+ swap(copyable_function& __x, copyable_function& __y) noexcept
>> >+ { __x.swap(__y); }
>> >+
>> >+ /// Check for emptiness by comparing with `nullptr`.
>> >+ friend bool
>> >+ operator==(const copyable_function& __x, nullptr_t) noexcept
>> >+ { return __x._M_invoke == nullptr; }
>> >+
>> >+ private:
>> >+ typename _Invoker::__storage_func_t _M_invoke = nullptr;
>> >+
>> >+ template<typename _Func>
>> >+ friend auto&
>> >+ __polyfunc::__invoker_of(_Func&) noexcept;
>> >+
>> >+ template<typename _Func>
>> >+ friend auto&
>> >+ __polyfunc::__base_of(_Func&) noexcept;
>> >+
>> >+ template<typename _Dst, typename _Src>
>> >+ friend consteval bool
>> >+ __polyfunc::__is_invoker_convertible() noexcept;
>> >+ };
>> >+
>> >+#undef _GLIBCXX_MOF_CV_REF
>> >+#undef _GLIBCXX_MOF_CV
>> >+#undef _GLIBCXX_MOF_REF
>> >+#undef _GLIBCXX_MOF_INV_QUALS
>> >+
>> >+_GLIBCXX_END_NAMESPACE_VERSION
>> >+} // namespace std
>> >diff --git a/libstdc++-v3/include/bits/mofunc_impl.h
>> >b/libstdc++-v3/include/bits/mofunc_impl.h
>> >index 5eb4b5a0047..509f596f8e3 100644
>> >--- a/libstdc++-v3/include/bits/mofunc_impl.h
>> >+++ b/libstdc++-v3/include/bits/mofunc_impl.h
>> >@@ -101,6 +101,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> > && (!__is_in_place_type_v<_Vt>) && __is_callable_from<_Vt>
>> > move_only_function(_Fn&& __f) noexcept(_S_nothrow_init<_Vt, _Fn>())
>> > {
>> >+ // _GLIBCXX_RESOLVE_LIB_DEFECTS
>> >+ // 4255. move_only_function constructor should recognize empty
>> >+ // copyable_functions
>> > if constexpr (is_function_v<remove_pointer_t<_Vt>>
>> > || is_member_pointer_v<_Vt>
>> > || __is_polymorphic_function_v<_Vt>)
>> >@@ -108,6 +111,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> > if (__f == nullptr)
>> > return;
>> > }
>> >+
>> > if constexpr (__is_polymorphic_function_v<_Vt>
>> > && __polyfunc::__is_invoker_convertible<_Vt,
>> > move_only_function>())
>> > {
>> >diff --git a/libstdc++-v3/include/bits/move_only_function.h
>> >b/libstdc++-v3/include/bits/move_only_function.h
>> >index 305fe986818..ecaded79d37 100644
>> >--- a/libstdc++-v3/include/bits/move_only_function.h
>> >+++ b/libstdc++-v3/include/bits/move_only_function.h
>> >@@ -1,4 +1,4 @@
>> >-// Implementation of std::move_only_function -*- C++ -*-
>> >+// Implementation of std::move_only_function and std::copyable_function
>> >-*- C++ -*-
>> >
>> > // Copyright The GNU Toolchain Authors.
>> > //
>> >@@ -36,7 +36,7 @@
>> >
>> > #include <bits/version.h>
>> >
>> >-#ifdef __glibcxx_move_only_function // C++ >= 23 && HOSTED
>> >+#if __cplusplus > 202002L && _GLIBCXX_HOSTED
>>
>> Could this be:
>> #if defined __glibcxx_move_only_function || defined
>> __glibcxx_copyable_function
>> ?
>>
>> Or don't bother checking it here at all, and rely on the fact that
>> <functional> only includes it when needed?
>
> Went for using defined(__glibcxx_copyable_function) ||
> defined(__glibcxx_copyable_function)
>>
>>
>>
>> >
>> > #include <bits/invoke.h>
>> > #include <bits/utility.h>
>> >@@ -99,7 +99,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> >
>> > template<typename _Tp>
>> > [[__gnu__::__always_inline__]]
>> >- _Tp*
>> >+ _Tp*
>>
>> What changed here? I'm not seeing any diff in this diff!
>
> It was removal of trailing whitespace. Locally removed it in previous commit,
> so I no longer see diff here.
Aha, maybe gmail removed the trailing space, so it wasn't present in
the patch that added _M_ptr and also wasn't present here.
