-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Fix #14630: Number separators not processed correctly #22064
Conversation
I can confirm that on my local build on macOS 14.2.1, a non-breaking space ( Merging this would fix #14630. |
std::memcpy(&sepBuffer[0], sep.data(), sep.size()); | ||
for (int32_t j = static_cast<int32_t>(sep.size()) - 1; j >= 0; j--) | ||
{ | ||
auto remainingLen = TSize - i; | ||
auto cpyLen = std::min(sep.size(), remainingLen); | ||
std::memcpy(&buffer[i], sep.data(), cpyLen); | ||
i += static_cast<TIndex>(cpyLen); | ||
buffer[i++] = sepBuffer[j]; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably fine, but I'd feel safer if we'd just use std::reverse_copy
instead.
I'd like to review this as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to write some tests for that, but time is a constraint as usual. No issues spotted in this PR.
The bug was caused because numbers are formatted in reverse order, but this did not take into account that thousands separators may consist of multiple bytes - the non-breaking space being one example. To keep things something, this reverses the byte order of the separators when copying them into the temporary buffer, so that everything is in the correct order when it gets reversed for the final time.
As a bonus, this also fixes a bug where it would truncate multibyte sequences if the buffer was almost full.
To test this, replace STR_5151 with a multibyte character, or multiple different characters and compare the results between
develop
and this PR.