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

[Jonathan Wakely <jwakely@redhat.com>]

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++
>> <libstdc++@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();
> 	  }
>       };
>   }
>