On Thu, Jul 2, 2026 at 8:03 PM Jonathan Wakely <[email protected]> wrote: > > 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. >
Make sense. Fixed in the new patch. > > > > 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. >
From f64894b97b109ee8481b044399fc6b02aa9179c1 Mon Sep 17 00:00:00 2001 From: Yuao Ma <[email protected]> Date: Thu, 2 Jul 2026 20:32:17 +0800 Subject: [PATCH] libstdc++: implement LWG3662 basic_string::append/assign(NTBS, pos, n) suboptimal This patch implements LWG3662 for both ABIs of strings. libstdc++-v3/ChangeLog: * include/bits/basic_string.h(append, assign): Add new overloads. * include/bits/cow_string.h(append, assign): Ditto. * testsuite/21_strings/basic_string/modifiers/append/char/2.cc: Test new overloads. * 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 | 16 +++++++++++ .../modifiers/append/wchar_t/2.cc | 16 +++++++++++ .../basic_string/modifiers/assign/char/3.cc | 16 +++++++++++ .../modifiers/assign/wchar_t/3.cc | 16 +++++++++++ 6 files changed, 118 insertions(+) diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index 65d92ebfbcf..fd9419220a4 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -1767,6 +1767,20 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 + std::__sv_check(__sv.size(), __pos, "basic_string::append"), std::__sv_limit(__sv.size(), __pos, __n)); } + + // _GLIBCXX_RESOLVE_LIB_DEFECTS + // 3662. basic_string::append/assign(NTBS, pos, n) suboptimal + /** + * @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)); } #endif // C++17 /** @@ -2040,6 +2054,20 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 + std::__sv_check(__sv.size(), __pos, "basic_string::assign"), std::__sv_limit(__sv.size(), __pos, __n)); } + + // _GLIBCXX_RESOLVE_LIB_DEFECTS + // 3662. basic_string::append/assign(NTBS, pos, n) suboptimal + /** + * @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)); } #endif // C++17 #if __cplusplus >= 201103L diff --git a/libstdc++-v3/include/bits/cow_string.h b/libstdc++-v3/include/bits/cow_string.h index 7c945f6b998..0f1a2e082d0 100644 --- a/libstdc++-v3/include/bits/cow_string.h +++ b/libstdc++-v3/include/bits/cow_string.h @@ -1427,6 +1427,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION + std::__sv_check(__sv.size(), __pos, "basic_string::append"), std::__sv_limit(__sv.size(), __pos, __n)); } + + // _GLIBCXX_RESOLVE_LIB_DEFECTS + // 3662. basic_string::append/assign(NTBS, pos, n) suboptimal + /** + * @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)); } #endif // C++17 /** @@ -1600,6 +1613,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION + std::__sv_check(__sv.size(), __pos, "basic_string::assign"), std::__sv_limit(__sv.size(), __pos, __n)); } + + // _GLIBCXX_RESOLVE_LIB_DEFECTS + // 3662. basic_string::append/assign(NTBS, pos, n) suboptimal + /** + * @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)); } #endif // C++17 /** 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..d9ccbd76896 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 @@ -19,11 +19,13 @@ // 21.3.5 string modifiers +#include <stdexcept> #include <string> #include <testsuite_hooks.h> // 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 +57,20 @@ 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" ); + + try { + two.append("a\0b", 2, 1); + VERIFY( false ); + } + catch(std::out_of_range& fail) { + VERIFY( true ); + } + catch(...) { + VERIFY( false ); + } } int main() diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/append/wchar_t/2.cc b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/append/wchar_t/2.cc index 1922874de7f..d0d95fb3f27 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/append/wchar_t/2.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/append/wchar_t/2.cc @@ -19,11 +19,13 @@ // 21.3.5 string modifiers +#include <stdexcept> #include <string> #include <testsuite_hooks.h> // 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 +57,20 @@ test02() two.append(two.c_str(), 3); VERIFY( two == L"Written in your eyeseyesWri" ); + + two.append(two.c_str(), 8, 2); + VERIFY( two == L"Written in your eyeseyesWriin" ); + + try { + two.append(L"a\0b", 2, 1); + VERIFY( false ); + } + catch(std::out_of_range& fail) { + VERIFY( true ); + } + catch(...) { + VERIFY( false ); + } } int main() diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/assign/char/3.cc b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/assign/char/3.cc index 1bd3deb30f4..75f2dcf0130 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/assign/char/3.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/assign/char/3.cc @@ -19,11 +19,13 @@ // 21.3.5 string modifiers +#include <stdexcept> #include <string> #include <testsuite_hooks.h> // assign(const _CharT* __s, size_type __n) // assign(const _CharT* __s) +// assign(const _CharT* __s, size_type __pos, size_type __n) void test03() { @@ -47,6 +49,20 @@ test03() one.assign(one.c_str() + 8, 6); VERIFY( one == "by the" ); + + one.assign(one.c_str(), 3, 3); + VERIFY( one == "the" ); + + try { + one.assign("a\0b", 2, 1); + VERIFY( false ); + } + catch(std::out_of_range& fail) { + VERIFY( true ); + } + catch(...) { + VERIFY( false ); + } } int main() diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/assign/wchar_t/3.cc b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/assign/wchar_t/3.cc index e7fcb231dff..821cde246bd 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/assign/wchar_t/3.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/assign/wchar_t/3.cc @@ -19,11 +19,13 @@ // 21.3.5 string modifiers +#include <stdexcept> #include <string> #include <testsuite_hooks.h> // assign(const _CharT* __s, size_type __n) // assign(const _CharT* __s) +// assign(const _CharT* __s, size_type __pos, size_type __n) void test03() { @@ -47,6 +49,20 @@ test03() one.assign(one.c_str() + 8, 6); VERIFY( one == L"by the" ); + + one.assign(one.c_str(), 3, 3); + VERIFY( one == L"the" ); + + try { + one.assign(L"a\0b", 2, 1); + VERIFY( false ); + } + catch(std::out_of_range& fail) { + VERIFY( true ); + } + catch(...) { + VERIFY( false ); + } } int main() -- 2.54.0
