On Thu, 2 Jul 2026 at 12:45, Yuao Ma <[email protected]> wrote: > > On Thu, Jul 2, 2026 at 4:56 AM Jonathan Wakely <[email protected]> wrote: > > > > On Wed, 1 Jul 2026 at 17:56, Yuao Ma <[email protected]> wrote: > > > > > > On Thu, Jul 2, 2026 at 12:00 AM Jonathan Wakely <[email protected]> > > > wrote: > > > > > > > > On Wed, 01 Jul 2026 at 23:11 +0800, Yuao Ma wrote: > > > > >On second thoughts, I think we can actually just remove the check > > > > >altogether. It delegates to the string_view overload, which ultimately > > > > >calls the overload without __pos anyway. > > > > > > > > > >On Wed, Jul 1, 2026 at 11:08 PM Jonathan Wakely <[email protected]> > > > > >wrote: > > > > >> > > > > >> On Wed, 1 Jul 2026 at 15:47, Tomasz Kaminski <[email protected]> > > > > >> wrote: > > > > >> > > > > > >> > Are the following checks correct? I think we need string of at > > > > >> > least length __pos + __len. > > > > >> > > > > >> The macro is defined as: > > > > >> # define __glibcxx_requires_string_len(_String,_Len) \ > > > > >> _GLIBCXX_DEBUG_PEDASSERT(_String != 0 || _Len == 0) > > > > >> > > > > >> So it says we can't have a null pointer unless n==0 is true. I think > > > > >> that's the right check here. > > > > >> > > > > >> > > > > >> > > > > >> > __glibcxx_requires_string_len(__s, __n); > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > On Wed, Jul 1, 2026 at 4:41 PM Yuao Ma <[email protected]> wrote: > > > > >> >> > > > > >> >> Hi! > > > > >> >> > > > > >> >> This patch implements LWG3662 for C++11 ABI and COW ABI of > > > > >> >> strings. > > > > >> >> > > > > >> >> Tested on x86_64 linux, ok for trunk? > > > > >> >> > > > > >> >> Thanks, > > > > >> >> Yuao > > > > >> > > > > > > > > >From abae92d920dcb27beab2720ac079f1e852fafb66 Mon Sep 17 00:00:00 2001 > > > > >From: Yuao Ma <[email protected]> > > > > >Date: Wed, 1 Jul 2026 23:09:35 +0800 > > > > >Subject: [PATCH] libstdc++: implement LWG3662 > > > > > basic_string::append/assign(NTBS, pos, n) suboptimal > > > > > > > > > >This patch implements LWG3662 for both ABIs of std::string. > > > > > > > > > >libstdc++-v3/ChangeLog: > > > > > > > > > > * include/bits/basic_string.h: Add new append/assign API. > > > > > > > > Please follow the documented form for the ChangeLog and name the > > > > functions being added: > > > > > > > > > > Fixed. Sometimes I forget to add the function name when > > > gcc-commit-mklog doesn't generate it automatically. > > > > > > > * include/bits/basic_string.h (append, assign): Add new > > > > overloads. > > > > > > > > > * include/bits/cow_string.h: Ditto. > > > > > * testsuite/21_strings/basic_string/modifiers/append/char/2.cc: > > > > > Test new API. > > > > > > > > Please break this line and the next three lines after the colon. > > > > > > > > 'git log' indents the commit log by 4 spaces, which means the > > > > ChangeLog entry is indended by 12 spaces, so if your ChangeLog entry > > > > already uses 85 lines it's going to be much too long. > > > > > > > > > > Fixed. > > > > > > > > * > > > > > testsuite/21_strings/basic_string/modifiers/append/wchar_t/2.cc: > > > > > Ditto. > > > > > * testsuite/21_strings/basic_string/modifiers/assign/char/3.cc: > > > > > Ditto. > > > > > * > > > > > testsuite/21_strings/basic_string/modifiers/assign/wchar_t/3.cc: > > > > > Ditto. > > > > >--- > > > > > libstdc++-v3/include/bits/basic_string.h | 28 +++++++++++++++++++ > > > > > libstdc++-v3/include/bits/cow_string.h | 26 +++++++++++++++++ > > > > > .../basic_string/modifiers/append/char/2.cc | 4 +++ > > > > > .../modifiers/append/wchar_t/2.cc | 4 +++ > > > > > .../basic_string/modifiers/assign/char/3.cc | 4 +++ > > > > > .../modifiers/assign/wchar_t/3.cc | 4 +++ > > > > > 6 files changed, 70 insertions(+) > > > > > > > > > >diff --git a/libstdc++-v3/include/bits/basic_string.h > > > > >b/libstdc++-v3/include/bits/basic_string.h > > > > >index 65d92ebfbcf..4f07b1c14f1 100644 > > > > >--- a/libstdc++-v3/include/bits/basic_string.h > > > > >+++ b/libstdc++-v3/include/bits/basic_string.h > > > > >@@ -1639,6 +1639,20 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 > > > > > return _M_append(__s, __n); > > > > > } > > > > > > > > > > > > > Please add a _GLIBCXX_RESOLVE_LIB_DEFECTS comment: > > > > > > > > // _GLIBCXX_RESOLVE_LIB_DEFECTS > > > > // 3662. basic_string::append/assign(NTBS, pos, n) suboptimal > > > > > > > > > > Fixed all 4 cases. > > > > > > > >+ /** > > > > >+ * @brief Append a C substring. > > > > >+ * @param __s The C string to append. > > > > >+ * @param __pos The position in the C string to append from. > > > > >+ * @param __n The number of characters to append. > > > > >+ * @return Reference to this string. > > > > >+ */ > > > > >+ _GLIBCXX20_CONSTEXPR > > > > >+ basic_string& > > > > >+ append(const _CharT* __s, size_type __pos, size_type __n) > > > > >+ { > > > > >+ return append(__sv_type(__s).substr(__pos, __n)); > > > > >+ } > > > > > > > > This should be one line: > > > > > > > > { return append(__sv_type(__s).substr(__pos, __n)); } > > > > > > > > > > Fixed all 4 cases. > > > > > > > >+ > > > > > /** > > > > > * @brief Append multiple characters. > > > > > * @param __n The number of characters to append. > > > > >@@ -1902,6 +1916,20 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 > > > > > traits_type::length(__s)); > > > > > } > > > > > > > > _GLIBCXX_RESOLVE_LIB_DEFECTS comment. > > > > > > > > >+ /** > > > > >+ * @brief Set value to a C substring. > > > > >+ * @param __s The C string to use. > > > > >+ * @param __pos The position in the C string to assign from. > > > > >+ * @param __n Number of characters to use. > > > > >+ * @return Reference to this string. > > > > >+ */ > > > > >+ _GLIBCXX20_CONSTEXPR > > > > >+ basic_string& > > > > >+ assign(const _CharT* __s, size_type __pos, size_type __n) > > > > >+ { > > > > >+ return assign(__sv_type(__s).substr(__pos, __n)); > > > > >+ } > > > > > > > > Just one line. > > > > > > > > >+ > > > > > /** > > > > > * @brief Set value to multiple characters. > > > > > * @param __n Length of the resulting string. > > > > >diff --git a/libstdc++-v3/include/bits/cow_string.h > > > > >b/libstdc++-v3/include/bits/cow_string.h > > > > >index 7c945f6b998..7708c2474ba 100644 > > > > >--- a/libstdc++-v3/include/bits/cow_string.h > > > > >+++ b/libstdc++-v3/include/bits/cow_string.h > > > > >@@ -1345,6 +1345,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > > > return this->append(__s, traits_type::length(__s)); > > > > > } > > > > > > > > _GLIBCXX_RESOLVE_LIB_DEFECTS comment. > > > > > > > > >+ /** > > > > >+ * @brief Append a C substring. > > > > >+ * @param __s The C string to append. > > > > >+ * @param __pos The position in the C string to append from. > > > > >+ * @param __n The number of characters to append. > > > > >+ * @return Reference to this string. > > > > >+ */ > > > > >+ basic_string& > > > > >+ append(const _CharT* __s, size_type __pos, size_type __n) > > > > >+ { > > > > >+ return append(__sv_type(__s).substr(__pos, __n)); > > > > >+ } > > > > > > > > One line. > > > > > > > > >+ > > > > > /** > > > > > * @brief Append multiple characters. > > > > > * @param __n The number of characters to append. > > > > >@@ -1517,6 +1530,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > > > return this->assign(__s, traits_type::length(__s)); > > > > > } > > > > > > > > _GLIBCXX_RESOLVE_LIB_DEFECTS comment. > > > > > > > > >+ /** > > > > >+ * @brief Set value to a C substring. > > > > >+ * @param __s The C string to use. > > > > >+ * @param __pos The position in the C string to assign from. > > > > >+ * @param __n Number of characters to use. > > > > >+ * @return Reference to this string. > > > > >+ */ > > > > >+ basic_string& > > > > >+ assign(const _CharT* __s, size_type __pos, size_type __n) > > > > >+ { > > > > >+ return assign(__sv_type(__s).substr(__pos, __n)); > > > > >+ } > > > > > > > > One line. > > > > > > > > >+ > > > > > /** > > > > > * @brief Set value to multiple characters. > > > > > * @param __n Length of the resulting string. > > > > >diff --git > > > > >a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/append/char/2.cc > > > > > > > > > >b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/append/char/2.cc > > > > >index d70fc190b45..5dd39347574 100644 > > > > >--- > > > > >a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/append/char/2.cc > > > > >+++ > > > > >b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/append/char/2.cc > > > > >@@ -24,6 +24,7 @@ > > > > > > > > > > // append(const _CharT* __s, size_type __n) > > > > > // append(const _CharT* __s) > > > > >+// append(const _CharT* __s, size_type __pos, size_type __n) > > > > > void > > > > > test02() > > > > > { > > > > >@@ -55,6 +56,9 @@ test02() > > > > > > > > > > two.append(two.c_str(), 3); > > > > > VERIFY( two == "Written in your eyeseyesWri" ); > > > > >+ > > > > >+ two.append(two.c_str(), 8, 2); > > > > >+ VERIFY( two == "Written in your eyeseyesWriin" ); > > > > > } > > > > > > > > Do we have a test for the s.append("a\0b", 2, 1) example I gave, which > > > > should throw std::out_of_range? I think that would be good to have. > > > > > > > > > > Added. > > > > > > OK for trunk, thanks. > > > > Sadly the CI caught some errors: https://patchwork.sourceware.org/patch/138220 > > I believe the issue is that, unlike in libc++, __sv_type in libstdc++ > is guarded by FTM. Because we are implementing this LWG issue as a DR, > using it in pre-C++17 mode causes an error. To fix this properly, I > think we need to implement a custom helper similar to _M_limit and > std::__sv_limit, since we cannot reuse the existing ones from either > string_view or basic_string. WDYT?
No, just don't backport the new overloads before C++17. > > BTW, the CI is quite useful - is there a way to manually trigger it? If you use forge.sourceware.org and create a pull request there, you can get the linaro CI to run.
