Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 22, 2025

Conversation

frederick-vs-ja
Copy link
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.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner April 16, 2025 07:08
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews Apr 16, 2025
@StephanTLavavej StephanTLavavej added bug Something isn't working ranges C++20/23 ranges labels Apr 16, 2025
@StephanTLavavej StephanTLavavej self-assigned this Apr 16, 2025
@StephanTLavavej
Copy link
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 StephanTLavavej removed their assignment Apr 19, 2025
@StephanTLavavej StephanTLavavej moved this from Initial Review to Ready To Merge in STL Code Reviews Apr 19, 2025
@StephanTLavavej StephanTLavavej moved this from Ready To Merge to Merging in STL Code Reviews Apr 22, 2025
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej merged commit 381e671 into microsoft:main Apr 22, 2025
39 checks passed
@github-project-automation github-project-automation bot moved this from Merging to Done in STL Code Reviews Apr 22, 2025
@StephanTLavavej
Copy link
Member

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

@frederick-vs-ja frederick-vs-ja deleted the string-volatile-range branch April 22, 2025 23:50
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
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

<string>: cannot be constructed from a volatile char array using the from_range constructor
2 participants