[PATCH] libstdc++: Implement stringstream from string_view [P2495R3]
Jonathan Wakely
jwakely.gcc@gmail.com
Tue May 20 09:26:29 GMT 2025
On Tue, 20 May 2025 at 10:08, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>
> On Tue, 20 May 2025 at 09:10, Tomasz Kaminski <tkaminsk@redhat.com> wrote:
> >
> >
> >
> > On Mon, May 19, 2025 at 11:28 PM Nathan Myers <ncm@cantrip.org> wrote:
> > In the title, we usually put link to bugzilla PR119741 in your case, not the paper.
> > Then link the paper in commit descritpion.
>
> Right. When there's no bugzilla I'll sometimes put the paper number in
> parens, e.g. in https://gcc.gnu.org/g:91f4550e1700 which has this
> summary:
> libstdc++: Move std::monostate to <utility> for C++26 (P0472R2)
>
> But the paper number should not be in square brackets, that should be
> reserved for a bugzilla PR number if there is one (and for this
> feature, we do have a bugzilla PR).
>
>
> >
> >> Add constructors to stringbuf, stringstream, istringstream,
> >> and ostringstream, and a matching overload of str(sv) in each,
> >> that take anything convertible to a string_view where the
> >> existing functions take a string.
> >
> >
> > After you put bugzilla number, git gcc-verify will suggest you to add following node:
> > <tab>PR libstdc++/119741
> >
> >>
> >>
> >> libstdc++-v3/ChangeLog:
> >>
> >> P2495R3 stringstream to init from string_view-ish
> >
> > We usually put only the change files here. Did git gcc-verify accepted it.
> >>
> >> * include/std/sstream: full implementation, really just
> >> decls, requires clause and plumbing.
> >> * include/std/bits/version.def, .h: new preprocessor symbol
> >> __cpp_lib_sstream_from_string_view.
> >> * testsuite/27_io/basic_stringbuf/cons/char/3.cc: New tests.
> >> * testsuite/27_io/basic_istringstream/cons/char/2.cc: New tests.
> >> * testsuite/27_io/basic_ostringstream/cons/char/4.cc: New tests.
> >> * testsuite/27_io/basic_stringstream/cons/char/2.cc: New tests.
> >> ---
> >> libstdc++-v3/ChangeLog | 11 +
> >> libstdc++-v3/include/bits/version.def | 11 +-
> >> libstdc++-v3/include/bits/version.h | 10 +
> >> libstdc++-v3/include/std/sstream | 181 +++++++++++++--
> >> .../27_io/basic_istringstream/cons/char/2.cc | 193 ++++++++++++++++
> >> .../27_io/basic_ostringstream/cons/char/4.cc | 193 ++++++++++++++++
> >> .../27_io/basic_stringbuf/cons/char/3.cc | 216 ++++++++++++++++++
> >> .../27_io/basic_stringstream/cons/char/2.cc | 194 ++++++++++++++++
> >> 8 files changed, 990 insertions(+), 19 deletions(-)
> >> create mode 100644 libstdc++-v3/testsuite/27_io/basic_istringstream/cons/char/2.cc
> >> create mode 100644 libstdc++-v3/testsuite/27_io/basic_ostringstream/cons/char/4.cc
> >> create mode 100644 libstdc++-v3/testsuite/27_io/basic_stringbuf/cons/char/3.cc
> >> create mode 100644 libstdc++-v3/testsuite/27_io/basic_stringstream/cons/char/2.cc
> >>
> >> diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
> >> index b45f8c2c7a5..ac0ff4a386f 100644
> >> --- a/libstdc++-v3/ChangeLog
> >> +++ b/libstdc++-v3/ChangeLog
> >> @@ -41,6 +41,17 @@
> >> PR libstdc++/119246
> >> * include/std/format: Updated check for _GLIBCXX_FORMAT_F128.
> >>
> >> +2025-05-14 Nathan Myers <nmyers@redhat.com>
> >> + P2495R3 stringstream to init from string_view-ish
> >> + * include/std/sstream: full implementation, really just
> >> + decls, requires clause and plumbing.
> >> + * include/std/bits/version.def, .h: new preprocessor symbol
> >> + __cpp_lib_sstream_from_string_view.
> >> + * testsuite/27_io/basic_stringbuf/cons/char/3.cc: New tests.
> >> + * testsuite/27_io/basic_istringstream/cons/char/2.cc: New tests.
> >> + * testsuite/27_io/basic_ostringstream/cons/char/4.cc: New tests.
> >> + * testsuite/27_io/basic_stringstream/cons/char/2.cc: New tests.
> >> +
> >
> > Changelogs are now automatically generated from commit messages, so you do not
> > need to edit this file.
> >>
> >> 2025-05-14 Tomasz Kamiński <tkaminsk@redhat.com>
> >>
> >> PR libstdc++/119125
> >> diff --git a/libstdc++-v3/include/bits/version.def b/libstdc++-v3/include/bits/version.def
> >> index 6ca148f0488..567c56b4117 100644
> >> --- a/libstdc++-v3/include/bits/version.def
> >> +++ b/libstdc++-v3/include/bits/version.def
> >> @@ -649,7 +649,7 @@ ftms = {
> >> };
> >> values = {
> >> v = 1;
> >> - /* For when there's no gthread. */
> >> + // For when there is no gthread.
> >> cxxmin = 17;
> >> hosted = yes;
> >> gthread = no;
> >> @@ -1961,6 +1961,15 @@ ftms = {
> >> };
> >> };
> >>
> >> +ftms = {
> >> + name = sstream_from_string_view;
> >> + values = {
> >> + v = 202302;
> >> + cxxmin = 26;
> >> + hosted = yes;
> >> + };
> >> +};
> >> +
> >> // Standard test specifications.
> >> stds[97] = ">= 199711L";
> >> stds[03] = ">= 199711L";
> >> diff --git a/libstdc++-v3/include/bits/version.h b/libstdc++-v3/include/bits/version.h
> >> index 48a090c14a3..5d1beb83a25 100644
> >> --- a/libstdc++-v3/include/bits/version.h
> >> +++ b/libstdc++-v3/include/bits/version.h
> >> @@ -2193,4 +2193,14 @@
> >> #endif /* !defined(__cpp_lib_modules) && defined(__glibcxx_want_modules) */
> >> #undef __glibcxx_want_modules
> >>
> >> +#if !defined(__cpp_lib_sstream_from_string_view)
> >> +# if (__cplusplus > 202302L) && _GLIBCXX_HOSTED
> >> +# define __glibcxx_sstream_from_string_view 202302L
> >> +# if defined(__glibcxx_want_all) || defined(__glibcxx_want_sstream_from_string_view)
> >> +# define __cpp_lib_sstream_from_string_view 202302L
> >> +# endif
> >> +# endif
> >> +#endif /* !defined(__cpp_lib_sstream_from_string_view) && defined(__glibcxx_want_sstream_from_string_view) */
> >> +#undef __glibcxx_want_sstream_from_string_view
> >> +
> >> #undef __glibcxx_want_all
> >> diff --git a/libstdc++-v3/include/std/sstream b/libstdc++-v3/include/std/sstream
> >> index ad0c16a91e8..9b2b0eb53fc 100644
> >> --- a/libstdc++-v3/include/std/sstream
> >> +++ b/libstdc++-v3/include/std/sstream
> >> @@ -38,9 +38,14 @@
> >> #endif
> >>
> >> #include <bits/requires_hosted.h> // iostream
> >> +#include <bits/version.h>
> >>
> >> #include <istream>
> >> #include <ostream>
> >> +#ifdef __cpp_lib_sstream_from_string_view
> >> +# include <type_traits> // is_convertible_v
> >> +#endif
> >> +
> >> #include <bits/alloc_traits.h> // allocator_traits, __allocator_like
> >>
> >> #if __cplusplus > 201703L && _GLIBCXX_USE_CXX11_ABI
> >> @@ -52,8 +57,6 @@
> >> # define _GLIBCXX_SSTREAM_ALWAYS_INLINE [[__gnu__::__always_inline__]]
> >> #endif
> >>
> >> -
> >> -
> >> namespace std _GLIBCXX_VISIBILITY(default)
> >> {
> >> _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >> @@ -159,6 +162,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >> { __rhs._M_sync(const_cast<char_type*>(__rhs._M_string.data()), 0, 0); }
> >>
> >> #if __cplusplus > 201703L && _GLIBCXX_USE_CXX11_ABI
> >> + // P0408 Efficient access to basic_stringbuf buffer
> >> explicit
> >> basic_stringbuf(const allocator_type& __a)
> >> : basic_stringbuf(ios_base::in | std::ios_base::out, __a)
> >> @@ -197,7 +201,35 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >> | ios_base::out)
> >> : basic_stringbuf(__s, __mode, allocator_type{})
> >> { }
> >> +#endif
> >> +
> >> +#ifdef __cpp_lib_sstream_from_string_view
> >> + template<typename _Tp>
> >> + explicit
> >> + basic_stringbuf(const _Tp& __t, ios_base::openmode __mode = ios_base::in | ios_base::out)
> >> + requires (is_convertible_v<const _Tp&, basic_string_view<_CharT, _Traits>>)
> >> + : basic_stringbuf(__t, __mode, allocator_type{})
> >> + { }
> >> +
> >> + template<typename _Tp>
> >> + basic_stringbuf(const _Tp& __t, const allocator_type& __a)
> >> + requires (is_convertible_v<const _Tp&, basic_string_view<_CharT, _Traits>>)
> >> + : basic_stringbuf(__t, ios_base::in | ios_base::out, __a)
> >> + { }
> >>
> >> + template<typename _Tp>
> >> + basic_stringbuf(const _Tp& __t, ios_base::openmode __mode, const allocator_type& __a)
> >> + requires (is_convertible_v<const _Tp&, basic_string_view<_CharT, _Traits>>)
> >> + : basic_stringbuf(__mode, __a)
> >
> > Any reason for not initializing _M_string(t, __a) directly here, string has string_view constructor.
>
> More importantly, it has a "convertible to string_view" constructor,
> which matches the constraint on _Tp here. So it does look like we can
> just construct _M_string directly from __t and __a.
>
> >>
> >> + {
> >> + basic_string_view<_CharT, _Traits> __sv{__t};
> >> + _M_string = __sv;
> >> + _M_stringbuf_init(__mode);
> >> + }
> >> +#endif // C++26
> >> +
> >> +#if __cplusplus > 201703L && _GLIBCXX_USE_CXX11_ABI
> >> + // P0408 Efficient access to basic_stringbuf buffer
> >> basic_stringbuf(basic_stringbuf&& __rhs, const allocator_type& __a)
> >> : basic_stringbuf(std::move(__rhs), __a, __xfer_bufptrs(__rhs, this))
> >> { __rhs._M_sync(const_cast<char_type*>(__rhs._M_string.data()), 0, 0); }
> >> @@ -262,6 +294,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >> #if __cplusplus > 201703L
> >> #if _GLIBCXX_USE_CXX11_ABI
> >> #if __cpp_concepts
> >> + // P0407 Allocator-aware basic_streambuf
> >> template<__allocator_like _SAlloc>
> >> _GLIBCXX_NODISCARD
> >> basic_string<_CharT, _Traits, _SAlloc>
> >> @@ -317,6 +350,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >>
> >> #if __cplusplus > 201703L && _GLIBCXX_USE_CXX11_ABI
> >> #if __cpp_concepts
> >> + // P0407 Allocator-aware basic_streambuf
> >> template<__allocator_like _SAlloc>
> >> requires (!is_same_v<_SAlloc, _Alloc>)
> >> void
> >> @@ -335,6 +369,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >> }
> >> #endif
> >>
> >> +#ifdef __cpp_lib_sstream_from_string_view
> >> + template <typename _Tp>
> >> + void
> >> + str(const _Tp& __t)
> >> + requires (is_convertible_v<const _Tp&, basic_string_view<_CharT, _Traits>>)
> >> + {
> >> + basic_string_view<_CharT, _Traits> __sv{__t};
> >> + _M_string = __sv;
> >> + _M_stringbuf_init(_M_mode);
> >> + }
> >> +#endif // C++26
> >> +
> >> protected:
> >> // Common initialization code goes here.
> >> void
> >> @@ -521,6 +567,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >> { }
> >>
> >> #if __cplusplus > 201703L && _GLIBCXX_USE_CXX11_ABI
> >> + // P0408 Efficient access to basic_stringbuf buffer
> >> +
> >> // The move constructor initializes an __xfer_bufptrs temporary then
> >> // delegates to this constructor to performs moves during its lifetime.
> >> basic_stringbuf(basic_stringbuf&& __rhs, const allocator_type& __a,
> >> @@ -584,7 +632,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >> */
> >> basic_istringstream()
> >> : __istream_type(), _M_stringbuf(ios_base::in)
> >> - { this->init(&_M_stringbuf); }
> >> + { this->init(std::__addressof(_M_stringbuf)); }
> >>
> >> /**
> >> * @brief Starts with an empty string buffer.
> >> @@ -601,7 +649,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >> explicit
> >> basic_istringstream(ios_base::openmode __mode)
> >> : __istream_type(), _M_stringbuf(__mode | ios_base::in)
> >> - { this->init(&_M_stringbuf); }
> >> + { this->init(std::__addressof(_M_stringbuf)); }
> >>
> >> /**
> >> * @brief Starts with an existing string buffer.
> >> @@ -620,7 +668,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >> basic_istringstream(const __string_type& __str,
> >> ios_base::openmode __mode = ios_base::in)
> >> : __istream_type(), _M_stringbuf(__str, __mode | ios_base::in)
> >> - { this->init(&_M_stringbuf); }
> >> + { this->init(std::__addressof(_M_stringbuf)); }
> >>
> >> /**
> >> * @brief The destructor does nothing.
> >> @@ -637,9 +685,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >> basic_istringstream(basic_istringstream&& __rhs)
> >> : __istream_type(std::move(__rhs)),
> >> _M_stringbuf(std::move(__rhs._M_stringbuf))
> >> - { __istream_type::set_rdbuf(&_M_stringbuf); }
> >> + { __istream_type::set_rdbuf(std::__addressof(_M_stringbuf)); }
> >>
> >> #if __cplusplus > 201703L && _GLIBCXX_USE_CXX11_ABI
> >> + // P0408 Efficient access to basic_stringbuf buffer
> >> basic_istringstream(ios_base::openmode __mode, const allocator_type& __a)
> >> : __istream_type(), _M_stringbuf(__mode | ios_base::in, __a)
> >> { this->init(std::__addressof(_M_stringbuf)); }
> >> @@ -667,10 +716,33 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >> explicit
> >> basic_istringstream(const basic_string<_CharT, _Traits, _SAlloc>& __str,
> >> ios_base::openmode __mode = ios_base::in)
> >> +#ifdef __cpp_lib_sstream_from_string_view
> >> + requires (!is_same_v<_SAlloc, allocator_type>)
> >> +#endif // C++26
> >
> > Standard have same constraint of ostringstream (https://eel.is/c++draft/string.streams#ostringstream.cons-6):
> > Constraints: is_same_v<SAlloc,Allocator> is false.
> > Do we miss some library issue?
>
> P2495R3 adds this to basic_istringstream with this note:
>
> [DRAFTING NOTE: Drive-by fix, this adds a missing constraint present
> in stringstream and ostringstream.]
>
> The constraints were added elsewhere by P0408 so I missed adding those
> constraints in https://gcc.gnu.org/g:95cb0fc8c51841
> Nathan, could you please add that same !is_same_v constraint to the
> basic_stringbuf, basic_ostringstream and basic_stringstream
> constructors as part of this patch?
Do those constrains actually do anything, except slow down compilation?
We have:
explicit basic_stringbuf(
const basic_string<charT, traits, Allocator>& s,
ios_base::openmode which = ios_base::in | ios_base::out);
And:
template<class SAlloc>
explicit basic_stringbuf(
const basic_string<charT, traits, SAlloc>& s,
ios_base::openmode which = ios_base::in | ios_base::out);
Constraints: is_same_v<SAlloc, Allocator> is false.
If Salloc is Allocator then the first one is a better match anyway, so
what's the constraint for?
More information about the Libstdc++
mailing list