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.
