public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely@redhat.com>
To: Thomas Rodgers <rodgert@appliantology.com>
Cc: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org, trodgers@redhat.com
Subject: Re: [PATCH] libstdc++: Add c++2a <syncstream>
Date: Thu, 15 Oct 2020 19:13:12 +0100	[thread overview]
Message-ID: <20201015181312.GH3402@redhat.com> (raw)
In-Reply-To: <20201015144606.1402141-1-rodgert@appliantology.com>

On 15/10/20 07:46 -0700, Thomas Rodgers wrote:
>From: Thomas Rodgers <trodgers@redhat.com>
>
>* Note: depends on a sufficiently C++20ified basic_stringbuf<>.
>
>libstdc++/Changelog:
>	libstdc++-v3/include/Makefile.am (std_headers): Add new header.
>	libstdc++-v3/include/Makefile.in: Regenerate.
>	libstdc++-v3/include/std/streambuf
>        (__detail::__streambuf_core_access): Define.
>        (basic_streambuf): Befriend __detail::__streambuf_core_access.
>	libstdc++-v3/include/std/syncstream: New header.
>	libstdc++-v3/include/std/version: Add __cpp_lib_syncbuf:
>	libstdc++-v3/testsuite/27_io/basic_syncbuf/1.cc: New test.
>	libstdc++-v3/testsuite/27_io/basic_syncbuf/2.cc: Likewise.
>	libstdc++-v3/testsuite/27_io/basic_syncbuf/basic_ops/1.cc:
>        Likewise.
>	libstdc++-v3/testsuite/27_io/basic_syncbuf/requirements/types.cc:
>        Likewise.
>	libstdc++-v3/testsuite/27_io/basic_syncbuf/sync_ops/1.cc:
>        Likewise.
>	libstdc++-v3/testsuite/27_io/basic_syncstream/1.cc: Likewise.
>	libstdc++-v3/testsuite/27_io/basic_syncstream/2.cc: Likewise.
>	libstdc++-v3/testsuite/27_io/basic_syncstream/basic_ops/1.cc:
>        Likewise.
>	libstdc++-v3/testsuite/27_io/basic_syncstream/requirements/types.cc:
>        Likewise.
>
>---
> libstdc++-v3/include/Makefile.am              |   1 +
> libstdc++-v3/include/Makefile.in              |   1 +
> libstdc++-v3/include/std/streambuf            |  81 +++++
> libstdc++-v3/include/std/syncstream           | 333 ++++++++++++++++++
> libstdc++-v3/include/std/version              |   4 +
> .../testsuite/27_io/basic_syncbuf/1.cc        |  28 ++
> .../testsuite/27_io/basic_syncbuf/2.cc        |  27 ++
> .../27_io/basic_syncbuf/basic_ops/1.cc        | 138 ++++++++
> .../27_io/basic_syncbuf/requirements/types.cc |  42 +++
> .../27_io/basic_syncbuf/sync_ops/1.cc         | 130 +++++++
> .../testsuite/27_io/basic_syncstream/1.cc     |  28 ++
> .../testsuite/27_io/basic_syncstream/2.cc     |  27 ++
> .../27_io/basic_syncstream/basic_ops/1.cc     | 135 +++++++
> .../basic_syncstream/requirements/types.cc    |  43 +++
> 14 files changed, 1018 insertions(+)
> create mode 100644 libstdc++-v3/include/std/syncstream
> create mode 100644 libstdc++-v3/testsuite/27_io/basic_syncbuf/1.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/basic_syncbuf/2.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/basic_syncbuf/basic_ops/1.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/basic_syncbuf/requirements/types.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/basic_syncbuf/sync_ops/1.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/basic_syncstream/1.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/basic_syncstream/2.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/basic_syncstream/basic_ops/1.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/basic_syncstream/requirements/types.cc
>
>diff --git a/libstdc++-v3/include/Makefile.am b/libstdc++-v3/include/Makefile.am
>index 28d273924ee..61aaff7a2f4 100644
>--- a/libstdc++-v3/include/Makefile.am
>+++ b/libstdc++-v3/include/Makefile.am
>@@ -73,6 +73,7 @@ std_headers = \
> 	${std_srcdir}/shared_mutex \
> 	${std_srcdir}/span \
> 	${std_srcdir}/sstream \
>+	${std_srcdir}/syncstream \

The new header should also be added to include/precompiled/stdc++.h
and doc/doxygen/user.cfg.in

> 	${std_srcdir}/stack \
> 	${std_srcdir}/stdexcept \
> 	${std_srcdir}/stop_token \
>diff --git a/libstdc++-v3/include/std/streambuf b/libstdc++-v3/include/std/streambuf
>index cae35e75bda..d6053e4c1ed 100644
>--- a/libstdc++-v3/include/std/streambuf
>+++ b/libstdc++-v3/include/std/streambuf
>@@ -48,6 +48,84 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> #define _IsUnused __attribute__ ((__unused__))
>
>+  template<typename _CharT, typename _Traits>
>+    class basic_streambuf;
>+
>+#if __cplusplus > 201703L
>+  namespace __detail
>+  {
>+    struct __streambuf_core_access
>+    {
>+      template<typename _CharT, typename _Traits>
>+	static void
>+	_S_imbue(basic_streambuf<_CharT, _Traits>& __b, const locale& __loc)
>+	{ __b.imbue(__loc); }

As discussed on IRC, this doesn't work. But as discussed below, I
don't think it's needed at all.

>+#endif // C++20
>+
>   template<typename _CharT, typename _Traits>
>     streamsize
>     __copy_streambufs_eof(basic_streambuf<_CharT, _Traits>*,
>@@ -456,6 +534,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       { return this->xsputn(__s, __n); }
>
>     protected:
>+#if __cplusplus > 201703L
>+      friend __detail::__streambuf_core_access;
>+#endif
>       /**
>        *  @brief  Base constructor.
>        *
>diff --git a/libstdc++-v3/include/std/syncstream b/libstdc++-v3/include/std/syncstream
>new file mode 100644
>index 00000000000..0a034ed03f4
>--- /dev/null
>+++ b/libstdc++-v3/include/std/syncstream
>@@ -0,0 +1,333 @@
>+// <syncstream> -*- C++ -*-
>+
>+// Copyright (C) 2020 Free Software Foundation, Inc.
>+//
>+// 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/ranges

s/ranges/syncstream/

>+ *  This is a Standard C++ Library header.
>+ */
>+
>+ #ifndef _GLIBCXX_SYNCSTREAM
>+ #define _GLIBCXX_SYNCSTREAM 1
>+
>+#if __cplusplus > 201703L && _GLIBCXX_USE_CXX11_ABI
>+
>+#define __cpp_lib_syncbuf 201803L
>+
>+#pragma GCC system_header
>+
>+#include <bits/alloc_traits.h>
>+#include <bits/allocator.h>
>+#include <bits/functional_hash.h>
>+#include <bits/std_mutex.h>
>+
>+#include <ostream>
>+#include <sstream>

The <ostream> include is redundant because <sstream> includes it
anyway.

Please put the standard headers before the <bits/xxx> ones. I don't
think you need <bits/alloc_traits.h> here, and <bits/allocator.h>
should already be included by <sstream>.


>+
>+namespace std _GLIBCXX_VISIBILITY(default)
>+{
>+_GLIBCXX_BEGIN_NAMESPACE_VERSION
>+
>+  template<typename _CharT, typename _Traits = char_traits<_CharT>,
>+	    typename _Alloc = allocator<_CharT>>
>+    class basic_syncbuf : public basic_streambuf<_CharT, _Traits>
>+    {
>+    public:
>+      using char_type = _CharT;
>+      using int_type = typename _Traits::int_type;
>+      using pos_type = typename _Traits::pos_type;
>+      using off_type = typename _Traits::off_type;
>+      using traits_type = _Traits;
>+      using allocator_type = _Alloc;
>+      using streambuf_type = basic_streambuf<_CharT, _Traits>;
>+
>+      basic_syncbuf()
>+      : basic_syncbuf(nullptr)

Please use basic_syncbuf(nullptr, allocator_type{}) here, so we skip
one level of delegation (and overload resolution for two args can
ignore the one-arg ctors).

>+      { }
>+
>+      explicit basic_syncbuf(streambuf_type* __obuf)

Newline after 'explicit' so that all the constructors have the class
name in the same column.


>+	: basic_syncbuf(__obuf, allocator_type{})
>+      { }
>+
>+      basic_syncbuf(streambuf_type* __obuf, const allocator_type& __alloc)
>+	: _M_wrapped(__obuf)
>+	, _M_impl(__alloc)
>+	, _M_mtx(__obuf ? &_S_get_mutex(__obuf) : nullptr)
>+      { }
>+
>+      basic_syncbuf(basic_syncbuf&& __other)
>+	: _M_wrapped(__other._M_wrapped)
>+	, _M_impl(std::move(__other._M_impl))
>+	, _M_mtx(__other._M_mtx)
>+	, _M_emit_on_sync(__other._M_emit_on_sync)
>+	, _M_needs_sync(__other._M_needs_sync)
>+      {
>+	__other._M_wrapped = nullptr;
>+      }
>+
>+      ~basic_syncbuf()
>+      {
>+	try

Include <bits/functexcept.h> and use __try and __catch so it compiles
with -fno-exceptions.

>+	  {
>+	    emit();
>+	  }
>+	catch (...)
>+	  { }
>+      }
>+
>+      basic_syncbuf& operator=(basic_syncbuf&& __other)
>+      {
>+	if (&__other != this)

This needs to be std::__addressof(__other) to stop ADL finding a
greedy operator& in an associated namespace of _CharT, _Traits, or
_Alloc.

>+	  {
>+	    emit();
>+	    _M_wrapped = __other._M_wrapped; __other._M_wrapped = nullptr;

Newline between statements.

>+	    _M_impl = std::move(__other._M_impl);

I think _M_impl needs to be moved first, because that can throw. We
need to do the part that can throw first, then if that succeeds it's
safe to modify the other members.

Although ... the standard says this function is noexcept ... except it
doesn't say that in the synopsis. It's noexcept in one place and not
another. Same for the member swap and non-member swap.

Sigh.

>+	    _M_mtx = __other._M_mtx;
>+	    _M_emit_on_sync = __other._M_emit_on_sync;
>+	    _M_needs_sync = __other._M_needs_sync;
>+	  }
>+	return *this;
>+      }
>+
>+      void
>+      swap(basic_syncbuf& __other)
>+      {
>+	if (&__other != this)

std::__addressof(__other) again.

>+	  {
>+	    std::swap(_M_wrapped, __other._M_wrapped);
>+	    std::swap(_M_impl, __other._M_impl);

Same issue with ordering the throwing operation first.

>+	    std::swap(_M_mtx, __other._M_mtx);
>+	    std::swap(_M_emit_on_sync, __other._M_emit_on_sync);
>+	    std::swap(_M_needs_sync, __other._M_needs_sync);
>+	  }
>+      }
>+
>+      bool
>+      emit()
>+      {
>+	if (!_M_wrapped)
>+	  return false;
>+
>+	auto __s = _M_impl.view();
>+	if (__s.empty())
>+	  return true;
>+
>+	const lock_guard<mutex> __l(*_M_mtx);
>+	auto __xsz = _M_wrapped->sputn(__s.data(), __s.size());
>+
>+	if (__xsz != __s.size())
>+	  return false;
>+
>+	if (_M_needs_sync)
>+	  {
>+	    _M_needs_sync = false;
>+	    if (_M_wrapped->pubsync() != 0)
>+	      return false;
>+	  }
>+	_M_impl.str("");

Hmm, I don't know what the standard specifies here. It says "On
success, the associated output is empty." But that doesn't tell us
anything about the state if we return false.

>+	return true;
>+      }
>+
>+      streambuf_type*
>+      get_wrapped() const noexcept
>+      { return _M_wrapped; }
>+
>+      allocator_type get_allocator() const noexcept
>+      { return _M_impl.get_allocator(); }
>+
>+      void
>+      set_emit_on_sync(bool __b) noexcept
>+      { _M_emit_on_sync = __b; }
>+
>+    protected:
>+      void
>+      imbue(const locale& __loc) override
>+      { __detail::__streambuf_core_access::_S_imbue(_M_impl, __loc); }

pubimbue, but why is it here anyway? I don't see anything in the
standard requiring it.

>+
>+      basic_streambuf<char_type,_Traits>*
>+      setbuf(char_type* __c, streamsize __n) override
>+      {
>+	return __detail::__streambuf_core_access::_S_setbuf(_M_impl, __c, __n);
>+      }

Not required by the standard.

>+
>+      pos_type
>+      seekoff(off_type __off, ios_base::seekdir __dir,
>+	      ios_base::openmode __mode) override
>+      {
>+	return __detail::__streambuf_core_access::_S_seekoff(_M_impl, __off,
>+							     __dir, __mode);

pubseekoff, but not required.

>+      }
>+
>+      pos_type
>+      seekpos(pos_type __pos, ios_base::openmode __mode) override
>+      {
>+	return __detail::__streambuf_core_access::_S_seekpos(_M_impl,
>+							     __pos, __mode);

pubseekpos, but not required.

>+      }
>+
>+      int
>+      sync() override
>+      {
>+	auto __res = __detail::__streambuf_core_access::_S_sync(_M_impl);
>+	if (__res == 0)
>+	  {
>+	    _M_needs_sync = true;
>+	    if (_M_emit_on_sync)
>+	      return emit() ? 0 : -1;
>+	  }
>+	return __res;
>+      }
>+
>+      streamsize
>+      showmanyc() override
>+      { return __detail::__streambuf_core_access::_S_showmanyc(_M_impl); }

I think this could use _M_impl.in_avail(), but it's not required.

>+      streamsize
>+      xsgetn(char_type* __s, streamsize __n) override
>+      {
>+	return __detail::__streambuf_core_access::_S_xsgetn(_M_impl, __s, __n);

Not needed, you can't read from a syncbuf.

>+      }
>+
>+      int_type
>+      underflow() override
>+      { return __detail::__streambuf_core_access::_S_underflow(_M_impl); }

Ditto.

>+
>+      int_type
>+      uflow() override
>+      { return __detail::__streambuf_core_access::_S_uflow(_M_impl); }

Ditto.

>+      int_type
>+      pbackfail(int_type __c) override
>+      { return __detail::__streambuf_core_access::_S_pbackfail(_M_impl, __c); }

You can't read, so you can't put characters back into the get area.

>+      streamsize
>+      xsputn(const char_type* __s, streamsize __n) override
>+      {
>+	return __detail::__streambuf_core_access::_S_xsputn(_M_impl, __s, __n);

This could be _M_impl.sputn(__s, __n)

I don't see any requirement to override it, although doing so might
improve performance.


>+      }
>+
>+    private:
>+      streambuf_type* _M_wrapped;
>+
>+      using __impl_type = basic_stringbuf<char_type, traits_type,
>+					  allocator_type>;
>+      __impl_type _M_impl;
>+      mutex* _M_mtx;
>+
>+      bool _M_emit_on_sync = false;
>+      bool _M_needs_sync = false;
>+
>+      static constexpr size_t _S_small_size = 128;

This seems to be unused.

>+
>+      static mutex&
>+      _S_get_mutex(void* __t)
>+      {
>+	const unsigned char __mask = 0xf;
>+	static mutex __m[__mask + 1];
>+
>+	auto __key = _Hash_impl::hash(__t) & __mask;
>+	return __m[__key];

This is OK for now but please add a FIXME: comment reminding us to put
it in the library

Keeping those mutexes in the library allows us to ensure that our
table of mutexes has a longer lifetime than anything that might use it
(which we don't currently do). Otherwise a global syncbuf can run its
destructor after the mutexes have been destroyed, leading to UB in the
emit() call in the destructor. Better to only fix that in one place.

We should also consider sharing a single table of mutexes for the
various things that need them, so we don't waste space in the library
with several different tables. Especially if we increase their
alignment so there's only one per cacheline.

>+      }
>+    };
>+
>+  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>;

This should be private.

>+
>+    private:
>+      syncbuf_type _M_syncbuf;
>+
>+    public:
>+      basic_osyncstream(streambuf_type* __buf, const allocator_type& __a)
>+	: _M_syncbuf(__buf, __a)
>+      { this->init(&_M_syncbuf); }

std::__addressof here and the constructors below.

>+
>+      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)
>+	  {
>+	    __ostream_type::operator=(std::move(__rhs));
>+	    _M_syncbuf = std::move(__rhs._M_syncbuf);
>+	    __ostream_type::set_rdbuf(&_M_syncbuf);
>+	  }
>+	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(); }
>+    };
>+
>+  template <class _CharT, class _Traits, class _Allocator>
>+    inline void
>+    swap(basic_syncbuf<_CharT, _Traits, _Allocator>& __x,

The standard says this has to be noexcept.

>+	 basic_syncbuf<_CharT, _Traits, _Allocator>& __y)
>+    { __x.swap(__y); }
>+
>+  using syncbuf = basic_syncbuf<char>;
>+  using wsyncbuf = basic_syncbuf<wchar_t>;
>+
>+  using osyncstream = basic_osyncstream<char>;
>+  using wosyncstream = basic_osyncstream<wchar_t>;
>+_GLIBCXX_END_NAMESPACE_VERSION
>+} // namespace std
>+#endif // C++2a
>+#endif	/* _GLIBCXX_SYNCSTREAM */
>+
>diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version
>index d5d42ed0a72..8681d70fbfa 100644
>--- a/libstdc++-v3/include/std/version
>+++ b/libstdc++-v3/include/std/version
>@@ -215,6 +215,10 @@
> #define __cpp_lib_interpolate 201902L
> #ifdef _GLIBCXX_HAS_GTHREADS
> # define __cpp_lib_jthread 201911L
>+# ifdef _GLIBCXX_USE_CXX11_ABI
>+// Only supported with cx11-abi
>+#  define __cpp_lib_syncbuf 201803L

Please keep the macros in alphabetical order. It should be further
down, not here.

Put it in order and guard it with
#if defined _GLIBCXX_USE_CXX11_ABI && _GLIBCXX_USE_CXX11_ABI


>+# endif
> #endif
> #define __cpp_lib_list_remove_return_type 201806L
> #define __cpp_lib_math_constants 201907L
>diff --git a/libstdc++-v3/testsuite/27_io/basic_syncbuf/1.cc b/libstdc++-v3/testsuite/27_io/basic_syncbuf/1.cc
>new file mode 100644
>index 00000000000..ebea9becdb3
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/27_io/basic_syncbuf/1.cc
>@@ -0,0 +1,28 @@
>+// Copyright (C) 2020 Free Software Foundation, Inc.
>+//
>+// 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.
>+
>+// You should have received a copy of the GNU General Public License along
>+// with this library; see the file COPYING3.  If not see
>+// <http://www.gnu.org/licenses/>.
>+
>+// { dg-options "-std=gnu++2a" }
>+// { dg-do compile { target c++2a } }
>+// { dg-require-effective-target cxx11-abi }
>+
>+#include <syncstream>
>+
>+#ifndef __cpp_lib_syncbuf
>+# error "Feature-test macro for syncbuf missing in <syncstream>"
>+#elif __cpp_lib_syncbuf!= 201803L

Add a space before the operator.

>+# error "Feature-test macro for syncbuf has wrong value in <syncstream>"
>+#endif
>diff --git a/libstdc++-v3/testsuite/27_io/basic_syncbuf/2.cc b/libstdc++-v3/testsuite/27_io/basic_syncbuf/2.cc
>new file mode 100644
>index 00000000000..43538f89fb8
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/27_io/basic_syncbuf/2.cc
>@@ -0,0 +1,27 @@
>+// Copyright (C) 2020 Free Software Foundation, Inc.
>+//
>+// 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.
>+
>+// You should have received a copy of the GNU General Public License along
>+// with this library; see the file COPYING3.  If not see
>+// <http://www.gnu.org/licenses/>.
>+
>+// { dg-options "-std=gnu++2a" }
>+// { dg-do compile { target c++2a } }
>+
>+#include <version>
>+
>+#ifndef __cpp_lib_syncbuf
>+# error "Feature-test macro for syncbuf missing in <version>"
>+#elif __cpp_lib_syncbuf!= 201803L

Space before the operator.


>+int main()
>+{
>+  test01();
>+  test02();
>+  test03();
>+  test04();
>+  return 0;

The return statement is unnecessary (we might still have something in
the manual saying it's needed, but it's wrong).

>diff --git a/libstdc++-v3/testsuite/27_io/basic_syncstream/1.cc b/libstdc++-v3/testsuite/27_io/basic_syncstream/1.cc
>new file mode 100644
>index 00000000000..ebea9becdb3
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/27_io/basic_syncstream/1.cc
>@@ -0,0 +1,28 @@
>+// Copyright (C) 2020 Free Software Foundation, Inc.
>+//
>+// 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.
>+
>+// You should have received a copy of the GNU General Public License along
>+// with this library; see the file COPYING3.  If not see
>+// <http://www.gnu.org/licenses/>.
>+
>+// { dg-options "-std=gnu++2a" }
>+// { dg-do compile { target c++2a } }
>+// { dg-require-effective-target cxx11-abi }
>+
>+#include <syncstream>
>+
>+#ifndef __cpp_lib_syncbuf
>+# error "Feature-test macro for syncbuf missing in <syncstream>"
>+#elif __cpp_lib_syncbuf!= 201803L

I don't think we need to test it again, we already did it in the
basic_syncbuf tests. This file and the next one can be removed.

>+# error "Feature-test macro for syncbuf has wrong value in <syncstream>"
>+#endif
>diff --git a/libstdc++-v3/testsuite/27_io/basic_syncstream/2.cc b/libstdc++-v3/testsuite/27_io/basic_syncstream/2.cc
>new file mode 100644
>index 00000000000..43538f89fb8
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/27_io/basic_syncstream/2.cc
>@@ -0,0 +1,27 @@
>+// Copyright (C) 2020 Free Software Foundation, Inc.
>+//
>+// 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.
>+
>+// You should have received a copy of the GNU General Public License along
>+// with this library; see the file COPYING3.  If not see
>+// <http://www.gnu.org/licenses/>.
>+
>+// { dg-options "-std=gnu++2a" }
>+// { dg-do compile { target c++2a } }
>+
>+#include <version>
>+
>+#ifndef __cpp_lib_syncbuf
>+# error "Feature-test macro for syncbuf missing in <version>"
>+#elif __cpp_lib_syncbuf!= 201803L
>+# error "Feature-test macro for syncbuf has wrong value in <version>"
>+#endif


  reply	other threads:[~2020-10-15 18:13 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-15 14:46 Thomas Rodgers
2020-10-15 18:13 ` Jonathan Wakely [this message]
2020-10-15 18:18 ` Jonathan Wakely
2020-10-15 19:29 ` Jonathan Wakely
2020-10-16  0:24   ` Thomas Rodgers
2020-10-21 16:53   ` Thomas Rodgers
2020-10-21 17:34     ` Jonathan Wakely
2020-10-21 19:39       ` Thomas Rodgers
2020-10-29 16:18     ` Jonathan Wakely
2020-10-29 21:12       ` Thomas Rodgers
2020-11-02 15:58         ` Thomas Rodgers
2020-11-02 16:10         ` Thomas Rodgers
2020-11-02 17:38           ` Jonathan Wakely
2020-11-02 18:43             ` Thomas Rodgers
2020-11-03 21:22               ` Christophe Lyon
2020-11-03 22:00                 ` Jonathan Wakely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201015181312.GH3402@redhat.com \
    --to=jwakely@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    --cc=rodgert@appliantology.com \
    --cc=trodgers@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).