On Wed, 14 May 2025, Tomasz Kaminski wrote:

> 
> 
> On Wed, May 14, 2025 at 9:41 AM Tomasz Kaminski <tkami...@redhat.com> wrote:
> 
> 
> On Tue, May 13, 2025 at 11:16 PM Patrick Palka <ppa...@redhat.com> wrote:
>       On Mon, 12 May 2025, Tomasz Kamiński wrote:
> 
>       > Based on the provision in C++26 [func.wrap.general] p2 this patch 
> adjust the generic
>       > move_only_function(_Fn&&) constructor, such that when _Fn refers to 
> selected
>       > move_only_function instantiations, the ownership of the target object 
> is direclty
>       > transfered to constructor object. This avoid cost of double 
> indireciton in this situation.
>       > We apply this also in C++23 mode.
>       >
>       > We also fix handling of self assigments, to match behavior required 
> by standard,
>       > due use of copy and swap idiom.
>       >
>       > An instantiations MF1 of move_only_function can transfer target of 
> another
>       > instantiation MF2, if it can be constructed via usual rules 
> (__is_callable_from<_MF2>),
>       > and their invoker are convertible (__is_invocer_convertible<MF2, 
> MF1>()), i.e.:
>       > * MF1 is less noexcept than MF2,
>       > * return types are the same after stripping cv-quals
>       > * adujsted parameters type are the same (__poly::_param_t), i.e. 
> param of types T and T&&
>       >   are compatible for non-trivially copyable objects.
>       > Compatiblity of cv ref qualification is checked via 
> __is_callable_from<_MF2>.
>       >
>       > To achieve above the generation of _M_invoke functions is moved to 
> _Invoke class
>       > templates, that only depends on noexcept, return type and adjusted 
> parameter of the
>       > signature. To make the invoker signature compatible between const and 
> mutable
>       > qualified signatures, we always accept _Storage as const& and perform 
> a const_cast
>       > for locally stored object. This approach guarantees that we never 
> strip const from
>       > const object.
>       >
>       > Another benefit of this approach is that 
> move_only_function<void(std::string)>
>       > and move_only_function<void(std::string&&)> use same funciton 
> pointer, which should
>       > reduce binary size.
>       >
>       > The _Storage and _Manager functionality was also extracted and 
> adjusted from
>       > _Mo_func base, in preparation for implementation for 
> copyable_function and
>       > function_ref. The _Storage was adjusted to store functions pointers 
> as void(*)().
>       > The manage function, now accepts _Op enum parameter, and supports 
> additional
>       > operations:
>       >  * _Op::_Address stores address of target object in destination
>       >  * _Op::_Copy, when enabled, copies from source to destination
>       > Furthremore, we provide a type-independent mamange functions for 
> handling all:
>       >  * function pointer types
>       >  * trivially copyable object stored locally.
>       > Similary as in case of invoker, we always pass source as const (for 
> copy),
>       > and cast away constness in case of move operations, where we know 
> that source
>       > is mutable.
>       >
>       > Finally, the new helpers are defined in __polyfunc internal namespace.
>       >
>       >       PR libstdc++/119125
>       >
>       > libstdc++-v3/ChangeLog:
>       >
>       >       * include/bits/mofunc_impl.h: Added 
> __glibcxx_move_only_function guard.
>       >       (std::move_only_function): Adjusted for changes in 
> bits/move_only_function.h
>       >       (move_only_function::move_only_function(_Fn&&)): Special case 
> move_only_functions
>       >       with same invoker.
>       >       (move_only_function::operator=(move_only_function&&)): Handle 
> self assigment.
>       >       * include/bits/move_only_function.h (__polyfunc::_Ptrs)
>       >       (__polyfunc::_Storage): Refactored from _Mo_func::_Storage.
>       >       (__polyfunc::_Manager): Refactored from _Mo_func::_S_manager.
>       >       (__polyfunc::__param_t): Moved from 
> move_only_function::__param_t.
>       >       (__polyfunc::_Base_invoker, __polyfunc::_Invoke): Refactored 
> from
>       >       move_only_function::_S_invoke.
>       >       (std::_Mofunc_base): Moved into __polyfunc::_Mo_base with parts
>       >       extracted to __polyfunc::_Storage and __polyfunc::_Manager.
>       >       (__polyfunc::__deref_as, __polyfunc::__invoker_of, 
> __polyfunc::__base_of)
>       >       (__polyfunc::__is_invoker_convertible): Define.
>       >       (std::__is_move_only_function_v): Renamed to 
> __is_polymorphic_function_v.
>       >       (std::__is_polymorphic_function_v): Renamed from 
> __is_polymorphic_function_v.
>       >       * testsuite/20_util/move_only_function/call.cc: Test for 
> functions pointers.
>       >       * testsuite/20_util/move_only_function/conv.cc: New test.
>       >       * testsuite/20_util/move_only_function/move.cc: Tests for self 
> assigment.
>       > ---
>       >  libstdc++-v3/include/bits/mofunc_impl.h       |  75 +--
>       >  .../include/bits/move_only_function.h         | 454 
> +++++++++++++-----
>       >  .../20_util/move_only_function/call.cc        |  14 +
>       >  .../20_util/move_only_function/conv.cc        | 188 ++++++++
>       >  .../20_util/move_only_function/move.cc        |  11 +
>       >  5 files changed, 588 insertions(+), 154 deletions(-)
>       >  create mode 100644 
> libstdc++-v3/testsuite/20_util/move_only_function/conv.cc
>       >
>       > diff --git a/libstdc++-v3/include/bits/mofunc_impl.h 
> b/libstdc++-v3/include/bits/mofunc_impl.h
>       > index 318a55e618f..839f19e0389 100644
>       > --- a/libstdc++-v3/include/bits/mofunc_impl.h
>       > +++ b/libstdc++-v3/include/bits/mofunc_impl.h
>       > @@ -62,8 +62,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       >    template<typename _Res, typename... _ArgTypes, bool _Noex>
>       >      class move_only_function<_Res(_ArgTypes...) _GLIBCXX_MOF_CV
>       >                              _GLIBCXX_MOF_REF noexcept(_Noex)>
>       > -    : _Mofunc_base
>       > +    : __polyfunc::_Mo_base
>       >      {
>       > +      using _Base = __polyfunc::_Mo_base;
>       > +      using _Invoker = __polyfunc::_Invoker<_Noex, _Res, 
> _ArgTypes...>;
>       > +      using _Signature = _Invoker::_Signature;
>       > +
>       >        template<typename _Tp>
>       >       using __callable
>       >         = __conditional_t<_Noex,
>       > @@ -87,7 +91,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       > 
>       >        /// Moves the target object, leaving the source empty.
>       >        move_only_function(move_only_function&& __x) noexcept
>       > -      : _Mofunc_base(static_cast<_Mofunc_base&&>(__x)),
>       > +      : _Base(static_cast<_Base&&>(__x)),
>       >       _M_invoke(std::__exchange(__x._M_invoke, nullptr))
>       >        { }
>       > 
>       > @@ -99,13 +103,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       >       {
>       >         if constexpr (is_function_v<remove_pointer_t<_Vt>>
>       >                       || is_member_pointer_v<_Vt>
>       > -                     || __is_move_only_function_v<_Vt>)
>       > +                     || __is_polymorphic_function_v<_Vt>)
>       >           {
>       >             if (__f == nullptr)
>       >               return;
>       >           }
>       > -       _M_init<_Vt>(std::forward<_Fn>(__f));
>       > -       _M_invoke = &_S_invoke<_Vt>;
>       > +       if constexpr (__is_polymorphic_function_v<_Vt>
>       > +                       && __polyfunc::__is_invoker_convertible<_Vt, 
> move_only_function>())
>       > +         {
>       > +           // Handle cases where _Fn is const reference to 
> copyable_function,
>       > +           // by firstly creatiogn temporary and moving from it. 
> This may move
> 
>       typo, 'creating'
> 
>       > +           // locally stored object twice.
>       > +           _Vt __tmp(std::forward<_Fn>(__f));
>       > +           _M_move(__polyfunc::__base_of(__tmp));
>       > +           _M_invoke = 
> std::__exchange(__polyfunc::__invoker_of(__tmp), nullptr);
>       > +         }
>       > +       else
>       > +         {
>       > +           _M_init<_Vt>(std::forward<_Fn>(__f));
>       > +           _M_invoke = _Invoker::template _S_storage<_Vt 
> _GLIBCXX_MOF_INV_QUALS>();
>       > +         }
>       >       }
>       > 
>       >        /// Stores a target object initialized from the arguments.
>       > @@ -115,7 +132,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       >       explicit
>       >       move_only_function(in_place_type_t<_Tp>, _Args&&... __args)
>       >       noexcept(_S_nothrow_init<_Tp, _Args...>())
>       > -     : _M_invoke(&_S_invoke<_Tp>)
>       > +     : _M_invoke(_Invoker::template _S_storage<_Tp 
> _GLIBCXX_MOF_INV_QUALS>())
>       >       {
>       >         static_assert(is_same_v<decay_t<_Tp>, _Tp>);
>       >         _M_init<_Tp>(std::forward<_Args>(__args)...);
>       > @@ -129,7 +146,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       >       move_only_function(in_place_type_t<_Tp>, initializer_list<_Up> 
> __il,
>       >                          _Args&&... __args)
>       >       noexcept(_S_nothrow_init<_Tp, initializer_list<_Up>&, 
> _Args...>())
>       > -     : _M_invoke(&_S_invoke<_Tp>)
>       > +     : _M_invoke(_Invoker::template _S_storage<_Tp 
> _GLIBCXX_MOF_INV_QUALS>())
>       >       {
>       >         static_assert(is_same_v<decay_t<_Tp>, _Tp>);
>       >         _M_init<_Tp>(__il, std::forward<_Args>(__args)...);
>       > @@ -139,8 +156,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       >        move_only_function&
>       >        operator=(move_only_function&& __x) noexcept
>       >        {
>       > -     _Mofunc_base::operator=(static_cast<_Mofunc_base&&>(__x));
>       > -     _M_invoke = std::__exchange(__x._M_invoke, nullptr);
>       > +     // Standard requires support of self assigment, by specifying 
> it as
>       > +     // copy and swap.
>       > +     if (this != addressof(__x)) [[likely]]
>       > +       {
>       > +         _Base::operator=(static_cast<_Base&&>(__x));
>       > +         _M_invoke = std::__exchange(__x._M_invoke, nullptr);
>       > +       }
>       >       return *this;
>       >        }
>       > 
>       > @@ -148,7 +170,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       >        move_only_function&
>       >        operator=(nullptr_t) noexcept
>       >        {
>       > -     _Mofunc_base::operator=(nullptr);
>       > +     _M_reset();
>       >       _M_invoke = nullptr;
>       >       return *this;
>       >        }
>       > @@ -167,7 +189,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       >        ~move_only_function() = default;
>       > 
>       >        /// True if a target object is present, false otherwise.
>       > -      explicit operator bool() const noexcept { return _M_invoke != 
> nullptr; }
>       > +      explicit operator bool() const noexcept
>       > +      { return _M_invoke != nullptr; }
>       > 
>       >        /** Invoke the target object.
>       >         *
>       > @@ -181,14 +204,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       >        operator()(_ArgTypes... __args) _GLIBCXX_MOF_CV_REF 
> noexcept(_Noex)
>       >        {
>       >       __glibcxx_assert(*this != nullptr);
>       > -     return _M_invoke(this, std::forward<_ArgTypes>(__args)...);
>       > +     return _M_invoke(this->_M_storage, 
> std::forward<_ArgTypes>(__args)...);
>       >        }
>       > 
>       >        /// Exchange the target objects (if any).
>       >        void
>       >        swap(move_only_function& __x) noexcept
>       >        {
>       > -     _Mofunc_base::swap(__x);
>       > +     _Base::swap(__x);
>       >       std::swap(_M_invoke, __x._M_invoke);
>       >        }
>       > 
>       > @@ -203,25 +226,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       >        { return __x._M_invoke == nullptr; }
>       > 
>       >      private:
>       > -      template<typename _Tp>
>       > -     using __param_t = __conditional_t<is_scalar_v<_Tp>, _Tp, _Tp&&>;
>       > +      typename _Invoker::__storage_func_t _M_invoke = nullptr;
>       > 
>       > -      using _Invoker = _Res (*)(_Mofunc_base _GLIBCXX_MOF_CV*,
>       > -                             __param_t<_ArgTypes>...) 
> noexcept(_Noex);
>       > +      template<typename _Func>
>       > +       friend auto&
> 
>       Should be indented two spaces more than template-head
> 
>       > +       __polyfunc::__invoker_of(_Func&) noexcept;
>       > 
>       > -      template<typename _Tp>
>       > -     static _Res
>       > -     _S_invoke(_Mofunc_base _GLIBCXX_MOF_CV* __self,
>       > -               __param_t<_ArgTypes>... __args) noexcept(_Noex)
>       > -     {
>       > -       using _TpCv = _Tp _GLIBCXX_MOF_CV;
>       > -       using _TpInv = _Tp _GLIBCXX_MOF_INV_QUALS;
>       > -       return std::__invoke_r<_Res>(
>       > -           std::forward<_TpInv>(*_S_access<_TpCv>(__self)),
>       > -           std::forward<__param_t<_ArgTypes>>(__args)...);
>       > -     }
>       > +      template<typename _Func>
>       > +        friend auto&
>       > +             __polyfunc::__base_of(_Func&) noexcept;
> 
>       Spaces/tab issue on this line
> 
>       > 
>       > -      _Invoker _M_invoke = nullptr;
>       > +      template<typename _Dst, typename _Src>
>       > +        friend consteval bool
> 
>       Should be a tab instead of spaces
> 
>       > +     __polyfunc::__is_invoker_convertible() noexcept;
>       >      };
>       > 
>       >  #undef _GLIBCXX_MOF_CV_REF
>       > diff --git a/libstdc++-v3/include/bits/move_only_function.h 
> b/libstdc++-v3/include/bits/move_only_function.h
>       > index 42b33d01901..5eb688a0ef4 100644
>       > --- a/libstdc++-v3/include/bits/move_only_function.h
>       > +++ b/libstdc++-v3/include/bits/move_only_function.h
>       > @@ -45,145 +45,348 @@ namespace std _GLIBCXX_VISIBILITY(default)
>       >  {
>       >  _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       > 
>       > -  template<typename... _Signature>
>       > -    class move_only_function; // not defined
>       > -
>       >    /// @cond undocumented
>       > -  class _Mofunc_base
>       > -  {
>       > -  protected:
>       > -    _Mofunc_base() noexcept
>       > -    : _M_manage(_S_empty)
>       > -    { }
>       > -
>       > -    _Mofunc_base(_Mofunc_base&& __x) noexcept
>       > -    {
>       > -      _M_manage = std::__exchange(__x._M_manage, _S_empty);
>       > -      _M_manage(_M_storage, &__x._M_storage);
>       > -    }
>       > -
>       > -    template<typename _Tp, typename... _Args>
>       > -      static constexpr bool
>       > -      _S_nothrow_init() noexcept
>       > -      {
>       > -     if constexpr (__stored_locally<_Tp>)
>       > -       return is_nothrow_constructible_v<_Tp, _Args...>;
>       > -     return false;
>       > -      }
>       > -
>       > -    template<typename _Tp, typename... _Args>
>       > -      void
>       > -      _M_init(_Args&&... __args) noexcept(_S_nothrow_init<_Tp, 
> _Args...>())
>       > -      {
>       > -     if constexpr (__stored_locally<_Tp>)
>       > -       ::new (_M_storage._M_addr()) 
> _Tp(std::forward<_Args>(__args)...);
> 
>       Why is global operator new/delete used in some call sites but not
>       others?
> 
> It should always be global new, fixing it.
> 
> I have checked, and previously we were using a global placement, when using 
> storage,
> that was paired with explicit invocaiton of destructor. However, for heap 
> allocating new
> we were using non-global new and matching delete. I think I will preserve 
> that, there is
> no reason to non-use user provided new when allocating on heap.
>  
> 
>       > -     else
>       > -       _M_storage._M_p = new _Tp(std::forward<_Args>(__args)...);
>       > -
>       > -     _M_manage = &_S_manage<_Tp>;
>       > -      }
>       > -
>       > -    _Mofunc_base&
>       > -    operator=(_Mofunc_base&& __x) noexcept
>       > -    {
>       > -      _M_manage(_M_storage, nullptr);
>       > -      _M_manage = std::__exchange(__x._M_manage, _S_empty);
>       > -      _M_manage(_M_storage, &__x._M_storage);
>       > -      return *this;
>       > -    }
>       > -
>       > -    _Mofunc_base&
>       > -    operator=(nullptr_t) noexcept
>       > -    {
>       > -      _M_manage(_M_storage, nullptr);
>       > -      _M_manage = _S_empty;
>       > -      return *this;
>       > -    }
>       > -
>       > -    ~_Mofunc_base() { _M_manage(_M_storage, nullptr); }
>       > -
>       > -    void
>       > -    swap(_Mofunc_base& __x) noexcept
>       > -    {
>       > -      // Order of operations here is more efficient if __x is empty.
>       > -      _Storage __s;
>       > -      __x._M_manage(__s, &__x._M_storage);
>       > -      _M_manage(__x._M_storage, &_M_storage);
>       > -      __x._M_manage(_M_storage, &__s);
>       > -      std::swap(_M_manage, __x._M_manage);
>       > -    }
>       > -
>       > -    template<typename _Tp, typename _Self>
>       > -      static _Tp*
>       > -      _S_access(_Self* __self) noexcept
>       > -      {
>       > -     if constexpr (__stored_locally<remove_const_t<_Tp>>)
>       > -       return static_cast<_Tp*>(__self->_M_storage._M_addr());
>       > -     else
>       > -       return static_cast<_Tp*>(__self->_M_storage._M_p);
>       > -      }
>       > +  template<typename _Tp>
>       > +    constexpr bool __is_polymorphic_function_v = false;
> 
>       This should be 'inline' I think, otherwise there might be 'std' module
>       issues?
> 
> Isn't them being a variable template sufficient?
> I have added inline, because __is_move_only_function was inline.

Oops, I mistakenly thought GCC doesn't yet implement CWG2387, but we do
for GCC 15 since r15-2798-g82cd63a63eaa61, so the 'inline' indeed is
unnecessary.  Sorry about that!

> 
>       > 
>       > -  private:
>       > -    struct _Storage
>       > +  namespace __polyfunc
>       > +  {
>       > +    union _Ptrs
>       >      {
>       > -      void*       _M_addr() noexcept       { return &_M_bytes[0]; }
>       > -      const void* _M_addr() const noexcept { return &_M_bytes[0]; }
>       > -
>       > -      // We want to have enough space to store a simple delegate 
> type.
>       > -      struct _Delegate { void (_Storage::*__pfm)(); _Storage* __obj; 
> };
>       > -      union {
>       > -     void* _M_p;
>       > -     alignas(_Delegate) alignas(void(*)())
>       > -       unsigned char _M_bytes[sizeof(_Delegate)];
>       > -      };
>       > +      void* _M_obj;
>       > +      void (*_M_func)();
>       >      };
>       > 
>       > -    template<typename _Tp>
>       > -      static constexpr bool __stored_locally
>       > -     = sizeof(_Tp) <= sizeof(_Storage) && alignof(_Tp) <= 
> alignof(_Storage)
>       > -         && is_nothrow_move_constructible_v<_Tp>;
>       > -
>       > -    // A function that either destroys the target object stored in 
> __target,
>       > -    // or moves the target object from *__src to __target.
>       > -    using _Manager = void (*)(_Storage& __target, _Storage* __src) 
> noexcept;
>       > +   struct _Storage
>       > +   {
>       > +     void*       _M_addr() noexcept       { return &_M_bytes[0]; }
>       > +     void const* _M_addr() const noexcept { return &_M_bytes[0]; }
>       > +
>       > +     template<typename _Tp>
>       > +       static consteval bool
>       > +       _S_stored_locally() noexcept
>       > +       {
>       > +      return sizeof(_Tp) <= sizeof(_Storage)
>       > +             && alignof(_Tp) <= alignof(_Storage)
>       > +             && is_nothrow_move_constructible_v<_Tp>;
>       > +       }
>       > +
>       > +     template<typename _Tp, typename... _Args>
>       > +       static consteval bool
>       > +       _S_nothrow_init() noexcept
>       > +       {
>       > +      if constexpr (_S_stored_locally<_Tp>())
>       > +        return is_nothrow_constructible_v<_Tp, _Args...>;
>       > +      return false;
>       > +       }
>       > +
>       > +     template<typename _Tp, typename... _Args>
>       > +       void
>       > +       _M_init(_Args&&... __args) noexcept(_S_nothrow_init<_Tp, 
> _Args...>())
>       > +       {
>       > +      if constexpr (is_function_v<remove_pointer_t<_Tp>>)
>       > +        {
>       > +          static_assert( sizeof...(__args) <= 1 );
>       > +          // __args can have up to one element, returns nullptr if 
> empty.
>       > +          _Tp __func = (nullptr, ..., __args);
>       > +          _M_ptrs._M_func = reinterpret_cast<void(*)()>(__func);
>       > +        }
>       > +      else if constexpr (!_S_stored_locally<_Tp>())
>       > +        _M_ptrs._M_obj = new _Tp(std::forward<_Args>(__args)...);
>       > +      else
>       > +        ::new (_M_addr()) _Tp(std::forward<_Args>(__args)...);
>       > +       }
>       > +
>       > +     template<typename _Tp>
>       > +       [[__gnu__::__always_inline__]]
>       > +       _Tp*
>       > +       _M_ptr() const noexcept
>       > +       {
>       > +      if constexpr (!_S_stored_locally<remove_const_t<_Tp>>())
>       > +        return static_cast<_Tp*>(_M_ptrs._M_obj);
>       > +      else if constexpr (is_const_v<_Tp>)
>       > +        return static_cast<_Tp*>(_M_addr());
>       > +      else
>       > +        // _Manager and _Invoker pass _Storage by const&, even for 
> mutable sources.
>       > +        return static_cast<_Tp*>(const_cast<void*>(_M_addr()));
>       > +       }
>       > +
>       > +     template<typename _Ref>
>       > +       [[__gnu__::__always_inline__]]
>       > +       _Ref
>       > +       _M_ref() const noexcept
>       > +       {
>       > +         using _Tp = remove_reference_t<_Ref>;
>       > +         if constexpr (is_function_v<remove_pointer_t<_Tp>>)
>       > +        return reinterpret_cast<_Tp>(_M_ptrs._M_func);
>       > +      else
>       > +        return static_cast<_Ref>(*_M_ptr<_Tp>());
>       > +       }
>       > +
>       > +     // We want to have enough space to store a simple delegate type.
>       > +     struct _Delegate { void (_Storage::*__pfm)(); _Storage* __obj; 
> };
>       > +     union {
>       > +       _Ptrs _M_ptrs;
>       > +       alignas(_Delegate) alignas(void(*)())
>       > +       unsigned char _M_bytes[sizeof(_Delegate)];
>       > +     };
>       > +   };
>       > +
>       > +   struct _Manager
>       > +   {
>       > +     enum class _Op
>       > +     {
>       > +       // saves address of entity in *__src to __target._M_ptrs,
>       > +       _Address,
>       > +       // moves entity stored in *__src to __target, __src becomes 
> empty
>       > +       _Move,
>       > +       // copies entity stored in *__src to __target, supported only 
> if
>       > +       // _ProvideCopy is specified.
>       > +       _Copy,
>       > +       // destroys entity stored in __target, __src is ignoring
>       > +       _Destroy,
>       > +     };
>       > +
>       > +    // A function that performs operation __op on the __target and 
> possibly __src.
>       > +    using _Func = void (*)(_Op __op, _Storage& __target, const 
> _Storage* __src) noexcept;
>       > 
>       >      // The no-op manager function for objects with no target.
>       > -    static void _S_empty(_Storage&, _Storage*) noexcept { }
>       > +    static void _S_empty(_Op, _Storage&, const _Storage*) noexcept { 
> }
>       > 
>       > -    // The real manager function for a target object of type _Tp.
>       > -    template<typename _Tp>
>       > -      static void
>       > -      _S_manage(_Storage& __target, _Storage* __src) noexcept
>       > +    template<bool _ProvideCopy, typename _Tp>
>       > +      consteval static auto
>       > +      _S_select()
>       >        {
>       > -     if constexpr (__stored_locally<_Tp>)
>       > -       {
>       > -         if (__src)
>       > -           {
>       > -             _Tp* __rval = static_cast<_Tp*>(__src->_M_addr());
>       > -             ::new (__target._M_addr()) _Tp(std::move(*__rval));
>       > -             __rval->~_Tp();
>       > -           }
>       > -         else
>       > -           static_cast<_Tp*>(__target._M_addr())->~_Tp();
>       > -       }
>       > +     if constexpr (is_function_v<remove_pointer_t<_Tp>>)
>       > +       return &_S_func;
>       > +     else if constexpr (!_Storage::_S_stored_locally<_Tp>())
>       > +       return &_S_ptr<_ProvideCopy, _Tp>;
>       > +     else if constexpr (is_trivially_copyable_v<_Tp>)
>       > +       return &_S_trivial;
>       >       else
>       > -       {
>       > -         if (__src)
>       > -           __target._M_p = __src->_M_p;
>       > -         else
>       > -           delete static_cast<_Tp*>(__target._M_p);
>       > -       }
>       > +       return &_S_local<_ProvideCopy, _Tp>;
>       >        }
>       > 
>       > -    _Storage _M_storage;
>       > -    _Manager _M_manage;
>       > -  };
>       > +   private:
>       > +     static void
>       > +     _S_func(_Op __op, _Storage& __target, const _Storage* __src) 
> noexcept
>       > +     {
>       > +       switch (__op)
>       > +       {
>       > +      case _Op::_Address:
>       > +      case _Op::_Move:
>       > +      case _Op::_Copy:
>       > +        __target._M_ptrs._M_func = __src->_M_ptrs._M_func;
>       > +        return;
>       > +      case _Op::_Destroy:
>       > +        return;
>       > +       }
>       > +     }
>       > +
>       > +     static void
>       > +     _S_trivial(_Op __op, _Storage& __target, const _Storage* __src) 
> noexcept
>       > +     {
>       > +       switch (__op)
>       > +       {
>       > +      case _Op::_Address:
>       > +        __target._M_ptrs._M_obj = 
> const_cast<void*>(__src->_M_addr());
>       > +        return;
>       > +      case _Op::_Move:
>       > +      case _Op::_Copy:
>       > +        // N.B. _Storage starts lifetime of _M_bytes char array,
>       > +        // that implicitly creates, amongst other, are possibly 
> trivially
>       > +        // copyable objects, so we copy any present in __src.
>       > +        new (&__target) _Storage(*__src);
>       > +        return;
>       > +      case _Op::_Destroy:
>       > +        return;
>       > +       }
>       > +     }
>       > +
>       > +     template<bool _Provide_copy, typename _Tp>
>       > +       static void
>       > +       _S_local(_Op __op, _Storage& __target, const _Storage* __src)
>       > +       noexcept(!_Provide_copy)
>       > +       {
>       > +      switch (__op)
>       > +      {
>       > +        case _Op::_Address:
>       > +          __target._M_ptrs._M_obj = __src->_M_ptr<_Tp>();
>       > +          return;
>       > +        case _Op::_Move:
>       > +          {
>       > +            _Tp* __obj = __src->_M_ptr<_Tp>();
>       > +            ::new(__target._M_addr()) _Tp(std::move(*__obj));
>       > +            __obj->~_Tp();
>       > +          }
>       > +          return;
>       > +        case _Op::_Destroy:
>       > +          __target._M_ptr<_Tp>()->~_Tp();
>       > +          return;
>       > +        case _Op::_Copy:
>       > +          if constexpr (_Provide_copy)
>       > +            ::new (__target._M_addr()) _Tp(__src->_M_ref<const 
> _Tp&>());
>       > +          else
>       > +            __builtin_unreachable();
>       > +          return;
>       > +      }
>       > +       }
>       > +
>       > +     template<bool _Provide_copy, typename _Tp>
>       > +       static void
>       > +       _S_ptr(_Op __op, _Storage& __target, const _Storage* __src)
>       > +       noexcept(!_Provide_copy)
>       > +       {
>       > +      switch (__op)
>       > +      {
>       > +        case _Op::_Address:
>       > +        case _Op::_Move:
>       > +          __target._M_ptrs._M_obj = __src->_M_ptrs._M_obj;
>       > +          return;
>       > +        case _Op::_Destroy:
>       > +          ::delete __target._M_ptr<_Tp>();
>       > +          return;
>       > +        case _Op::_Copy:
>       > +          if constexpr (_Provide_copy)
>       > +            __target._M_ptrs._M_obj = ::new _Tp(__src->_M_ref<const 
> _Tp&>());
>       > +          else
>       > +            __builtin_unreachable();
>       > +          return;
>       > +       }
>       > +     }
>       > +   };
>       > +
>       > +   template<bool _Noex, typename _Ret, typename... _Args>
>       > +     struct _Base_invoker
>       > +     {
>       > +       using _Signature = _Ret(*)(_Args...) noexcept(_Noex);
>       > +
>       > +       using __storage_func_t = _Ret(*)(const _Storage&, _Args...) 
> noexcept(_Noex);
>       > +       template<typename _Tp>
>       > +      static consteval __storage_func_t
>       > +      _S_storage()
>       > +      { return &_S_call_storage<_Adjust_target<_Tp>>; }
>       > +
>       > +     private:
>       > +       template<typename _Tp, typename _Td = remove_cvref_t<_Tp>>
>       > +      using _Adjust_target =
>       > +        __conditional_t<is_pointer_v<_Td> || 
> is_member_pointer_v<_Td>, _Td, _Tp>;
>       > +
>       > +       template<typename _Tp>
>       > +      static _Ret
>       > +      _S_call_storage(const _Storage& __ref, _Args... __args) 
> noexcept(_Noex)
>       > +      {
>       > +        // N.B. static_cast<_Args> in contrast to forward<_Args> 
> does not bind
>       > +        // reference if argument is passed by value.
>       > +        return std::__invoke_r<_Ret>(__ref._M_ref<_Tp>(),
>       > +                                     static_cast<_Args>(__args)...);
>       > +      }
>       > +     };
>       > +
>       > +   template<typename _Tp>
>       > +     using __param_t = __conditional_t<is_scalar_v<_Tp>, _Tp, _Tp&&>;
>       > +
>       > +   template<bool _Noex, typename _Ret, typename... _Args>
>       > +     using _Invoker = _Base_invoker<_Noex, remove_cv_t<_Ret>, 
> __param_t<_Args>...>;
>       > +
>       > +   class _Mo_base
>       > +   {
>       > +   protected:
>       > +     _Mo_base() noexcept
>       > +     : _M_manage(_Manager::_S_empty)
>       > +     { }
>       > +
>       > +     _Mo_base(_Mo_base&& __x) noexcept
>       > +     { _M_move(__x); }
>       > +
>       > +     template<typename _Tp, typename... _Args>
>       > +       static consteval bool
>       > +       _S_nothrow_init() noexcept
>       > +       { return _Storage::_S_nothrow_init<_Tp, _Args...>(); }
>       > +
>       > +     template<typename _Tp, typename... _Args>
>       > +       void
>       > +       _M_init(_Args&&... __args)
>       > +       noexcept(_S_nothrow_init<_Tp, _Args...>())
>       > +       {
>       > +      _M_storage._M_init<_Tp>(std::forward<_Args>(__args)...);
>       > +      _M_manage = _Manager::_S_select<false, _Tp>();
>       > +       }
>       > +
>       > +     void _M_move(_Mo_base& __x) noexcept
> 
>       Missing newline before function name
> 
>       > +     {
>       > +       using _Op = _Manager::_Op;
>       > +       _M_manage = std::__exchange(__x._M_manage, 
> _Manager::_S_empty);
>       > +       _M_manage(_Op::_Move, _M_storage, &__x._M_storage);
>       > +     }
>       > +
>       > +     _Mo_base&
>       > +     operator=(_Mo_base&& __x) noexcept
>       > +     {
>       > +       _M_destroy();
>       > +       _M_move(__x);
>       > +       return *this;
>       > +     }
>       > +
>       > +     void _M_reset() noexcept
> 
>       Same
> 
>       LGTM otherwise
> 
>       > +     {
>       > +       _M_destroy();
>       > +       _M_manage = _Manager::_S_empty;
>       > +     }
>       > +
>       > +     ~_Mo_base()
>       > +     { _M_destroy(); }
>       > +
>       > +     void
>       > +     swap(_Mo_base& __x) noexcept
>       > +     {
>       > +       using _Op = _Manager::_Op;
>       > +       // Order of operations here is more efficient if __x is empty.
>       > +       _Storage __s;
>       > +       __x._M_manage(_Op::_Move, __s, &__x._M_storage);
>       > +       _M_manage(_Op::_Move, __x._M_storage, &_M_storage);
>       > +       __x._M_manage(_Op::_Move, _M_storage, &__s);
>       > +       std::swap(_M_manage, __x._M_manage);
>       > +     }
>       > +
>       > +     _Storage _M_storage;
>       > +
>       > +   private:
>       > +     void _M_destroy() noexcept
>       > +     { _M_manage(_Manager::_Op::_Destroy, _M_storage, nullptr); }
>       > +
>       > +     _Manager::_Func _M_manage;
>       > +   };
>       > +
>       > +   template<typename _Func>
>       > +     auto&
>       > +     __invoker_of(_Func& __f) noexcept
>       > +     { return __f._M_invoke; }
>       > +
>       > +   template<typename _Func>
>       > +     auto&
>       > +     __base_of(_Func& __f) noexcept
>       > +     { return static_cast<__like_t<_Func&, typename 
> _Func::_Base>>(__f); }
>       > +
>       > +   template<typename _Src, typename _Dst>
>       > +     consteval bool
>       > +     __is_invoker_convertible() noexcept
>       > +     {
>       > +       if constexpr (requires { typename _Src::_Signature; })
>       > +      return is_convertible_v<typename _Src::_Signature,
>       > +                              typename _Dst::_Signature>;
>       > +       else
>       > +      return false;
>       > +     }
>       > +} // namespace __polyfunc
>       > +  /// @endcond
>       > 
>       > +  template<typename... _Signature>
>       > +    class move_only_function; // not defined
>       > +
>       > +  /// @cond undocumented
>       >    template<typename _Tp>
>       > -    inline constexpr bool __is_move_only_function_v = false;
>       > -  template<typename _Tp>
>       > -    constexpr bool 
> __is_move_only_function_v<move_only_function<_Tp>> = true;
>       > -  /// @endcond
>       > +    constexpr bool 
> __is_polymorphic_function_v<move_only_function<_Tp>> = true;
>       > 
>       >    namespace __detail::__variant
>       >    {
>       > @@ -196,6 +399,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       >        : true_type
>       >        { };
>       >    }  // namespace __detail::__variant
>       > +  /// @endcond
>       > 
>       >  _GLIBCXX_END_NAMESPACE_VERSION
>       >  } // namespace std
>       > diff --git 
> a/libstdc++-v3/testsuite/20_util/move_only_function/call.cc 
> b/libstdc++-v3/testsuite/20_util/move_only_function/call.cc
>       > index bfc609afe37..217de374763 100644
>       > --- a/libstdc++-v3/testsuite/20_util/move_only_function/call.cc
>       > +++ b/libstdc++-v3/testsuite/20_util/move_only_function/call.cc
>       > @@ -190,6 +190,19 @@ test04()
>       >    VERIFY( std::move(std::as_const(f5))() == 3 );
>       >  }
>       > 
>       > +void
>       > +test05()
>       > +{
>       > +  int (*fp)() = [] { return 0; };
>       > +  move_only_function<int()> f0{fp};
>       > +  VERIFY( f0() == 0 );
>       > +  VERIFY( std::move(f0)() == 0 );
>       > +
>       > +  const move_only_function<int() const> f1{fp};
>       > +  VERIFY( f1() == 0 );
>       > +  VERIFY( std::move(f1)() == 0 );
>       > +}
>       > +
>       >  struct Incomplete;
>       > 
>       >  void
>       > @@ -206,5 +219,6 @@ int main()
>       >    test02();
>       >    test03();
>       >    test04();
>       > +  test05();
>       >    test_params();
>       >  }
>       > diff --git 
> a/libstdc++-v3/testsuite/20_util/move_only_function/conv.cc 
> b/libstdc++-v3/testsuite/20_util/move_only_function/conv.cc
>       > new file mode 100644
>       > index 00000000000..3da5e9e90a3
>       > --- /dev/null
>       > +++ b/libstdc++-v3/testsuite/20_util/move_only_function/conv.cc
>       > @@ -0,0 +1,188 @@
>       > +// { dg-do run { target c++23 } }
>       > +// { dg-require-effective-target hosted }
>       > +
>       > +#include <functional>
>       > +#include <testsuite_hooks.h>
>       > +
>       > +using std::move_only_function;
>       > +
>       > +static_assert( 
> !std::is_constructible_v<std::move_only_function<void()>,
>       > +                                     
> std::move_only_function<void()&>> );
>       > +static_assert( 
> !std::is_constructible_v<std::move_only_function<void()>,
>       > +                                     
> std::move_only_function<void()&&>> );
>       > +static_assert( 
> !std::is_constructible_v<std::move_only_function<void()&>,
>       > +                                     
> std::move_only_function<void()&&>> );
>       > +static_assert( 
> !std::is_constructible_v<std::move_only_function<void() const>,
>       > +                                     
> std::move_only_function<void()>> );
>       > +
>       > +// Non-trivial args, guarantess that type is not passed by copy
>       > +struct CountedArg
>       > +{
>       > +  CountedArg() = default;
>       > +  CountedArg(const CountedArg& f) noexcept : counter(f.counter) { 
> ++counter; }
>       > +  CountedArg& operator=(CountedArg&&) = delete;
>       > +
>       > +  int counter = 0;
>       > +};
>       > +CountedArg const c;
>       > +
>       > +// When move_only_functions is constructed from other 
> move_only_function,
>       > +// the compiler can avoid double indirection per C++26 
> [func.wrap.general] p2.
>       > +
>       > +void
>       > +test01()
>       > +{
>       > +  auto f = [](CountedArg const& arg) noexcept { return arg.counter; 
> };
>       > +  std::move_only_function<int(CountedArg) const noexcept> m1(f);
>       > +  VERIFY( m1(c) == 1 );
>       > +
>       > +  std::move_only_function<int(CountedArg) const> m2(std::move(m1));
>       > +  VERIFY( m2(c) == 1 );
>       > +
>       > +  std::move_only_function<int(CountedArg)> m3(std::move(m2));
>       > +  VERIFY( m3(c) == 1 );
>       > +
>       > +  // Invokers internally uses Counted&& for non-trivial types,
>       > +  // sinature remain compatible.
>       > +  std::move_only_function<int(CountedArg&&)> m4(std::move(m3));
>       > +  VERIFY( m4({}) == 0 );
>       > +
>       > +  std::move_only_function<int(CountedArg&&)&&> m5(std::move(m4));
>       > +  VERIFY( std::move(m5)({}) == 0 );
>       > +
>       > +  m4 = f;
>       > +  std::move_only_function<int(CountedArg&&)&> m7(std::move(m4));
>       > +  VERIFY( m7({}) == 0 );
>       > +
>       > +  m4 = f;
>       > +  std::move_only_function<int(CountedArg&&)&> m8(std::move(m4));
>       > +  VERIFY( m8({}) == 0 );
>       > +
>       > +  // Incompatible signatures
>       > +  m1 = f;
>       > +  std::move_only_function<long(CountedArg) const noexcept> 
> m9(std::move(m1));
>       > +  VERIFY( m9(c) == 2 );
>       > +}
>       > +
>       > +void
>       > +test02()
>       > +{
>       > +  auto f = [](CountedArg const& arg) noexcept { return arg.counter; 
> };
>       > +  std::move_only_function<int(CountedArg) const noexcept> m1(f);
>       > +  VERIFY( m1(c) == 1 );
>       > +
>       > +  std::move_only_function<int(CountedArg) const> m2;
>       > +  m2 = std::move(m1);
>       > +  VERIFY( m2(c) == 1 );
>       > +
>       > +  std::move_only_function<int(CountedArg)> m3;
>       > +  m3 = std::move(m2);
>       > +  VERIFY( m3(c) == 1 );
>       > +
>       > +  // Invokers internally uses Counted&& for non-trivial types,
>       > +  // sinature remain compatible.
>       > +  std::move_only_function<int(CountedArg&&)> m4;
>       > +  m4 = std::move(m3);
>       > +  VERIFY( m4({}) == 0 );
>       > +
>       > +  std::move_only_function<int(CountedArg&&)&&> m5;
>       > +  m5 = std::move(m4);
>       > +  VERIFY( std::move(m5)({}) == 0 );
>       > +
>       > +  m4 = f;
>       > +  std::move_only_function<int(CountedArg&&)&> m7;
>       > +  m7 = std::move(m4);
>       > +  VERIFY( m7({}) == 0 );
>       > +
>       > +  m4 = f;
>       > +  std::move_only_function<int(CountedArg&&)&> m8;
>       > +  m8 = std::move(m4);
>       > +  VERIFY( m8({}) == 0 );
>       > +
>       > +  m1 = f;
>       > +  std::move_only_function<long(CountedArg) const noexcept> m9;
>       > +  m9 = std::move(m1);
>       > +  VERIFY( m9(c) == 2 );
>       > +}
>       > +
>       > +void
>       > +test03()
>       > +{
>       > +  std::move_only_function<int(long) const noexcept> e;
>       > +  VERIFY( e == nullptr );
>       > +
>       > +  std::move_only_function<int(long) const> e2(std::move(e));
>       > +  VERIFY( e2 == nullptr );
>       > +  e2 = std::move(e);
>       > +  VERIFY( e2 == nullptr );
>       > +
>       > +  std::move_only_function<bool(int) const> e3(std::move(e));
>       > +  VERIFY( e3 == nullptr );
>       > +  e3 = std::move(e);
>       > +  VERIFY( e3 == nullptr );
>       > +}
>       > +
>       > +void
>       > +test04()
>       > +{
>       > +  struct F
>       > +  {
>       > +    int operator()(CountedArg const& arg) noexcept
>       > +    { return arg.counter; }
>       > +
>       > +    int operator()(CountedArg const& arg) const noexcept
>       > +    { return arg.counter + 1000; }
>       > +  };
>       > +
>       > +  F f;
>       > +  std::move_only_function<int(CountedArg) const> m1(f);
>       > +  VERIFY( m1(c) == 1001 );
>       > +
>       > +  // Call const overload as std::move_only_function<int(CountedArg) 
> const>
>       > +  // inside std::move_only_function<int(CountedArg)> would do.
>       > +  std::move_only_function<int(CountedArg)> m2(std::move(m1));
>       > +  VERIFY( m2(c) == 1001 );
>       > +
>       > +  std::move_only_function<int(CountedArg)> m3(f);
>       > +  VERIFY( m3(c) == 1 );
>       > +}
>       > +
>       > +void
>       > +test05()
>       > +{
>       > +  auto f = [](CountedArg const& arg) noexcept { return arg.counter; 
> };
>       > +  std::move_only_function<int(CountedArg)> w1(f);
>       > +  // move_only_function stores move_only_function due incompatibile 
> signatures
>       > +  std::move_only_function<int(CountedArg const&)> w2(std::move(w1));
>       > +  // copy is made when passing to int(CountedArg)
>       > +  VERIFY( w2(c) == 1 );
>       > +  // wrapped 3 times
>       > +  w1 = std::move(w2);
>       > +  VERIFY( w1(c) == 2 );
>       > +  // wrapped 4 times
>       > +  w2 = std::move(w1);
>       > +  VERIFY( w2(c) == 2 );
>       > +  // wrapped 5 times
>       > +  w1 = std::move(w2);
>       > +  VERIFY( w1(c) == 3 );
>       > +}
>       > +
>       > +void
>       > +test06()
>       > +{
>       > +  // No special interoperability with std::function
>       > +  auto f = [](CountedArg const& arg) noexcept { return arg.counter; 
> };
>       > +  std::function<int(CountedArg)> f1(f);
>       > +  std::move_only_function<int(CountedArg) const> m1(std::move(f1));
>       > +  VERIFY( m1(c) == 2 );
>       > +}
>       > +
>       > +int main()
>       > +{
>       > +  test01();
>       > +  test02();
>       > +  test03();
>       > +  test04();
>       > +  test05();
>       > +  test06();
>       > +}
>       > diff --git 
> a/libstdc++-v3/testsuite/20_util/move_only_function/move.cc 
> b/libstdc++-v3/testsuite/20_util/move_only_function/move.cc
>       > index 51e31a6323d..6da02c9cd81 100644
>       > --- a/libstdc++-v3/testsuite/20_util/move_only_function/move.cc
>       > +++ b/libstdc++-v3/testsuite/20_util/move_only_function/move.cc
>       > @@ -32,6 +32,12 @@ test01()
>       >    VERIFY( m1().copy == 1 );
>       >    VERIFY( m1().move == 0 );
>       > 
>       > +  // Standard specifies move assigment as copy and swap
>       > +  m1 = std::move(m1);
>       > +  VERIFY( m1 != nullptr );
>       > +  VERIFY( m1().copy == 1 );
>       > +  VERIFY( m1().move == 0 );
>       > +
>       >    // This will move construct a new target object and destroy the 
> old one:
>       >    auto m2 = std::move(m1);
>       >    VERIFY( m1 == nullptr && m2 != nullptr );
>       > @@ -80,6 +86,11 @@ test02()
>       >    VERIFY( m1().copy == 1 );
>       >    VERIFY( m1().move == 0 );
>       > 
>       > +  m1 = std::move(m1);
>       > +  VERIFY( m1 != nullptr );
>       > +  VERIFY( m1().copy == 1 );
>       > +  VERIFY( m1().move == 0 );
>       > +
>       >    // The target object is on the heap so this just moves a pointer:
>       >    auto m2 = std::move(m1);
>       >    VERIFY( m1 == nullptr && m2 != nullptr );
>       > --
>       > 2.49.0
>       >
>       >
> 
> 
> 

Reply via email to