Skip to content

<string>: Fix construction and insertion of basic_string with volatile ranges - #5409

Merged
Stephan T. Lavavej (StephanTLavavej) merged 5 commits into
microsoft:mainfrom
frederick-vs-ja:string-volatile-range
Apr 22, 2025
Merged

<string>: Fix construction and insertion of basic_string with volatile ranges#5409
Stephan T. Lavavej (StephanTLavavej) merged 5 commits into
microsoft:mainfrom
frederick-vs-ja:string-volatile-range

Conversation

@frederick-vs-ja

Copy link
Copy Markdown
Contributor

Formerly, (const) volatile CharT arrays were inconsistently handled in legacy iterator pair functions and new functions added in C++23 WG21-P1206R7. In legacy functions, (const) volatile CharT* were not specially treated due to _Is_elem_cptr, but in P1206R7 functions, they were consistently treated with (const) CharT* due to _Contiguous_range_of.

This PR chooses to follow the convention of _Contiguous_range_of and changes _Is_elem_cptr to _Is_elem_cvptr to handle (const) volatile CharT*. For volatile contiguous ranges, we need to use non-vectorized element-wise assignment.

Fixes #5402.

Comment thread stl/inc/xstring Outdated
Comment thread stl/inc/xstring Outdated
Comment thread tests/std/tests/GH_005402_string_with_volatile_range/test.cpp
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks! 😻 This diff looked a lot scarier than it actually was - just had to deal with the code movement.

Please double-check my _Traits_copy_batch change.

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) moved this from Initial Review to Ready To Merge in STL Code Reviews Apr 19, 2025
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) moved this from Ready To Merge to Merging in STL Code Reviews Apr 22, 2025
@StephanTLavavej

Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 381e671 into microsoft:main Apr 22, 2025
@github-project-automation github-project-automation Bot moved this from Merging to Done in STL Code Reviews Apr 22, 2025
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for fixing everyone's favorite cv-qualifier! 😹 🐱 🐈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ranges C++20/23 ranges

2 participants