Re: [PATCH] libstdc++: Add c++2a <syncstream>

[Jonathan Wakely <jwakely@redhat.com>]

On 15/10/20 07:46 -0700, Thomas Rodgers wrote:
>+  template <typename _CharT, typename _Traits = char_traits<_CharT>,
>+	    typename _Alloc = allocator<_CharT>>
>+    class basic_osyncstream : public basic_ostream<_CharT, _Traits>
>+    {
>+    public:
>+      // Types:
>+      using char_type = _CharT;
>+      using traits_type = _Traits;
>+      using allocator_type = _Alloc;
>+      using int_type = typename traits_type::int_type;
>+      using pos_type = typename traits_type::pos_type;
>+      using off_type = typename traits_type::off_type;
>+      using syncbuf_type = basic_syncbuf<_CharT, _Traits, _Alloc>;
>+      using streambuf_type = typename syncbuf_type::streambuf_type;
>+
>+      using __ostream_type = basic_ostream<_CharT, _Traits>;
>+
>+    private:
>+      syncbuf_type _M_syncbuf;
>+
>+    public:
>+      basic_osyncstream(streambuf_type* __buf, const allocator_type& __a)
>+	: _M_syncbuf(__buf, __a)
>+      { this->init(&_M_syncbuf); }
>+
>+      explicit basic_osyncstream(streambuf_type* __buf)
>+	: _M_syncbuf(__buf)
>+      { this->init(&_M_syncbuf); }
>+
>+      basic_osyncstream(basic_ostream<char_type, traits_type>& __os,
>+		        const allocator_type& __a)
>+	: basic_osyncstream(__os.rdbuf(), __a)
>+      { this->init(&_M_syncbuf); }
>+
>+      explicit basic_osyncstream(basic_ostream<char_type, traits_type>& __os)
>+	: basic_osyncstream(__os.rdbuf())
>+      { this->init(&_M_syncbuf); }
>+
>+      basic_osyncstream(basic_osyncstream&& __rhs) noexcept
>+	: __ostream_type(std::move(__rhs)),
>+	_M_syncbuf(std::move(__rhs._M_syncbuf))
>+      { __ostream_type::set_rdbuf(&_M_syncbuf); }
>+
>+      ~basic_osyncstream() = default;
>+
>+      basic_osyncstream& operator=(basic_osyncstream&& __rhs) noexcept
>+      {
>+	if (&__rhs != this)

Rather than adding std::__addressof here, I'm not sure we need the
check for self-assignment at all. The ostream base's move assignment
is safe on self-assignment, and the syncbuf is too (because it checks
for it).

>+	  {
>+	    __ostream_type::operator=(std::move(__rhs));
>+	    _M_syncbuf = std::move(__rhs._M_syncbuf);
>+	    __ostream_type::set_rdbuf(&_M_syncbuf);

I think this set_rdbuf is not needed.

I think the move assignment could be defaulted.

>+	  }
>+	return *this;
>+      }
>+
>+      syncbuf_type* rdbuf() const noexcept
>+      { return const_cast<syncbuf_type*>(&_M_syncbuf); }
>+
>+      streambuf_type* get_wrapped() const noexcept
>+      { return _M_syncbuf.get_wrapped(); }
>+
>+      void emit()
>+      { _M_syncbuf.emit(); }

This needs to check the result of _M_syncbuf.emit() and possibly set
failbit.