Skip to content
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

C++20 constexpr _Copy_s and simplify char8_t compare #861

Merged
merged 1 commit into from
May 30, 2020

Conversation

StephanTLavavej
Copy link
Member

Followup to #491, in response to #491 (comment) and #491 (comment).

  • Mark our non-Standard extension char_traits::_Copy_s and basic_string_view::_Copy_s as _CONSTEXPR20. Similarly to how Misc constexpr #491 changed copy, previously _Copy_s was always constexpr in basic_string_view but never in char_traits; I similarly argue that this will not affect users because it would have required them to notice and provide a custom traits class (and in this case, the non-Standard extension is far more obscure than the Standard function). Making these constexpr in C++20 mode only is simpler to explain.

  • Simplify char_traits::compare for char8_t. This is possible because memcmp and __builtin_memcmp take const void* and return int, and their behavior is correct for char8_t. The other u8 intrinsics are type-sensitive so they can't be unified like this.

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label May 24, 2020
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner May 24, 2020 02:37
stl/inc/xstring Show resolved Hide resolved
@StephanTLavavej StephanTLavavej self-assigned this May 29, 2020
@StephanTLavavej StephanTLavavej merged commit 191b184 into microsoft:master May 30, 2020
@StephanTLavavej StephanTLavavej deleted the char_traits branch May 30, 2020 00:41
@StephanTLavavej
Copy link
Member Author

Thanks, @CaseyCarter, for these improvement ideas!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants