Re: [PATCH] libstdc++: Implement P2328 changes to join_view

[Jonathan Wakely via Gcc-patches]

On 30/04/21 17:34 -0400, Patrick Palka via Libstdc++ wrote:

On Fri, 30 Apr 2021, Tim Song wrote:

On Fri, Apr 30, 2021 at 12:11 PM Patrick Palka via Libstdc++
<libstd...@gcc.gnu.org> wrote:
>
> +       template<typename _Iter>
> +         _Tp&
> +         _M_emplace_deref(const _Iter& __i)
> +         {
> +           this->reset();
> +           return this->emplace(*__i);
> +         }

This incurs a move, avoiding of which is the sole reason for
emplace-deref's existence.

Ah thanks, I had missed that...  IIUC, if we instead derive from
optional's internal base class _Optional_base, we can easily get at the
underlying storage for the object and directly initialize it with '*__i'
and avoid the move. How does the the following adjustment to the patch
look?

Looks good to me, thanks.

-- >8 --

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges

index 971898f5492..f1db4a238ef 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -2254,7 +2254,7 @@ namespace views::__adaptor

    template<typename _Tp>
      requires is_object_v<_Tp>
-      struct __non_propagating_cache<_Tp> : private optional<_Tp>
+      struct __non_propagating_cache<_Tp> : private _Optional_base<_Tp>
      {
        __non_propagating_cache() = default;

@@ -2264,32 +2264,36 @@ namespace views::__adaptor

        constexpr
        __non_propagating_cache(__non_propagating_cache&& __other) noexcept
-       { __other.reset(); }
+       { __other._M_reset(); }

        constexpr __non_propagating_cache&
        operator=(const __non_propagating_cache& __other) noexcept
        {
          if (std::__addressof(__other) != this)
-           this->reset();
+           this->_M_reset();
          return *this;
        }

        constexpr __non_propagating_cache&
        operator=(__non_propagating_cache&& __other) noexcept
        {
-         this->reset();
-         __other.reset();
+         this->_M_reset();
+         __other._M_reset();
          return *this;
        }

-       using optional<_Tp>::operator*;
+       constexpr _Tp&
+       operator*() noexcept
+       { return this->_M_get(); }

        template<typename _Iter>
          _Tp&
          _M_emplace_deref(const _Iter& __i)
          {
-           this->reset();
-           return this->emplace(*__i);
+           this->_M_reset();
+           ::new ((void *) std::__addressof(this->_M_payload._M_payload)) 
_Tp(*__i);
+           this->_M_payload._M_engaged = true;
+           return this->_M_get();
          }
      };
  }