[PATCH] libstdc++: implement LWG3662 basic_string::append/assign(NTBS, pos, n) suboptimal
Yuao Ma
addr2line@gmail.com
Thu Jul 2 12:34:12 GMT 2026
On Thu, Jul 2, 2026 at 8:03 PM Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Thu, 2 Jul 2026 at 12:45, Yuao Ma <addr2line@gmail.com> wrote:
> >
> > On Thu, Jul 2, 2026 at 4:56 AM Jonathan Wakely <jwakely@redhat.com> wrote:
> > >
> > > On Wed, 1 Jul 2026 at 17:56, Yuao Ma <addr2line@gmail.com> wrote:
> > > >
> > > > On Thu, Jul 2, 2026 at 12:00 AM Jonathan Wakely <jwakely@redhat.com> 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 <jwakely@redhat.com> wrote:
> > > > > >>
> > > > > >> On Wed, 1 Jul 2026 at 15:47, Tomasz Kaminski <tkaminsk@redhat.com> 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 <addr2line@gmail.com> 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 <c8ef@outlook.com>
> > > > > >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.
>
-------------- next part --------------
From f64894b97b109ee8481b044399fc6b02aa9179c1 Mon Sep 17 00:00:00 2001
From: Yuao Ma <c8ef@outlook.com>
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
More information about the Libstdc++
mailing list