Implement LWG-3662 string::append/assign(NTBS, pos, n) should not construct a temporary string#6235
Open
YexuanXiao wants to merge 10 commits into
Open
Implement LWG-3662 string::append/assign(NTBS, pos, n) should not construct a temporary string#6235YexuanXiao wants to merge 10 commits into
YexuanXiao wants to merge 10 commits into
Conversation
…struct a temporary string
Contributor
There was a problem hiding this comment.
Pull request overview
Implements LWG-3662 by adding basic_string::append/assign(const CharT* ntbs, size_type pos, size_type n) overloads that operate directly on the NTBS (avoiding constructing a temporary basic_string), and adds a regression test to ensure the implementation doesn’t rely on default-constructing the allocator.
Changes:
- Add
basic_string::append(const _Elem*, size_type, size_type)implemented via_Traits::length+_Appendwith clamping. - Add
basic_string::assign(const _Elem*, size_type, size_type)implemented via_Traits::length+_Assignwith clamping. - Add
test_LWG3662()using a non-default-constructible allocator to catch temporary-string construction.
Show a summary per file
| File | Description |
|---|---|
| tests/std/tests/VSO_0174871_string_replace/test.cpp | Adds test_LWG3662() regression coverage for the new NTBS pos/n overloads and out-of-range behavior. |
| stl/inc/xstring | Introduces new append/assign(NTBS, pos, n) overloads intended to avoid temporary basic_string construction. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
stl/inc/xstring:1662
- Same SAL issue as the new
appendoverload:_In_reads_(_Count)is misleading for this NTBS overload because_Traits::length(_Ptr)reads until the null terminator regardless of_Count. Please change this to_In_z_for consistency with other NTBS-taking overloads in this header.
_CONSTEXPR20 basic_string& assign(
_In_reads_(_Count) const _Elem* const _Ptr, const size_type _Off, _CRT_GUARDOVERFLOW const size_type _Count) {
// assign(string_view(_Ptr).substr(_Off, _Count))
const size_type _Length = _Convert_size<size_type>(_Traits::length(_Ptr));
if (_Off > _Length) {
- Files reviewed: 2/2 changed files
- Comments generated: 3
Noticed by Copilot.
StephanTLavavej
approved these changes
Jun 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6203.