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

Sporadic failure of P0645R10_text_formatting_formatting #2449

Closed
AlexGuteniev opened this issue Dec 27, 2021 · 3 comments · Fixed by #2569
Closed

Sporadic failure of P0645R10_text_formatting_formatting #2449

AlexGuteniev opened this issue Dec 27, 2021 · 3 comments · Fixed by #2569
Labels
bug Something isn't working fixed Something works now, yay! format C++20/23 format

Comments

@AlexGuteniev
Copy link
Contributor

This run:
https://dev.azure.com/vclibs/STL/_build/results?buildId=9974&view=logs&j=e5928d94-6045-5a4b-1479-6369daf1c6e1&t=e774f207-3822-5f8e-5e8b-2cdb9d7ebe0b

The subsequent run succeeded, though the change between the runs was in unrelated area, and except of one variable un-shadowing that does not affect anything, it was whitespace/comments change: 2b43c89

@StephanTLavavej
Copy link
Member

The error was:

Assertion failed: format("{:.2000a}", 1.0) == expected, file C:\a\1\s\tests\std\tests\P0645R10_text_formatting_formatting\test.cpp, line 836

I don't understand what happened here. Perhaps we should update the test with additional logging, to print the result of format() when it's unexpected here?

@StephanTLavavej StephanTLavavej added bug Something isn't working format C++20/23 format info needed We need more info before working on this labels Jan 12, 2022
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jan 20, 2022

Happened again in: https://dev.azure.com/vclibs/STL/_build/results?buildId=10119&view=ms.vss-test-web.build-test-results-tab&runId=1184952&resultId=102554&paneView=debug

Slightly different line/expression this time (note line 820 instead of line 836):

Assertion failed: format("{:.2000a}", value) == expected, file C:\a\1\s\tests\std\tests\P0645R10_text_formatting_formatting\test.cpp, line 820

@StephanTLavavej
Copy link
Member

#2494 fortunately printed the actual string before the expected string (the latter was lost due to log truncation, #2557), @statementreply in #2499 (comment) noticed what was wrong with that string indicating a bug in the "huge precision, clamp it and then append zeros later" codepath, I was able to repro it locally and find the root cause in

STL/stl/inc/format

Lines 2510 to 2546 in afe0800

auto _Exponent_start = _Result.ptr;
auto _Radix_point = _Result.ptr;
auto _Integral_end = _Result.ptr;
auto _Zeroes_to_append = 0;
auto _Separators = 0;
string _Groups;
if (_Is_finite) {
if (_Specs._Alt || _Specs._Localized) {
for (auto _It = _Buffer_start; _It < _Result.ptr; ++_It) {
if (*_It == '.') {
_Radix_point = _It;
} else if (*_It == _Exponent) {
_Exponent_start = _It;
}
}
_Integral_end = (_STD min)(_Radix_point, _Exponent_start);
if (_Specs._Alt && _Radix_point == _Result.ptr) {
// TRANSITION, decimal point may be wider
++_Width;
_Append_decimal = true;
}
if (_Specs._Localized) {
_Groups = _STD use_facet<numpunct<_CharT>>(_Locale._Get()).grouping();
_Separators = _Count_separators(static_cast<size_t>(_Integral_end - _Buffer_start), _Groups);
}
}
switch (_Format) {
case chars_format::hex:
case chars_format::scientific:
if (_Extra_precision != 0) {
while (*_Exponent_start != _Exponent) { // Trailing zeroes are in front of the exponent
--_Exponent_start;
}

where while (*_Exponent_start != _Exponent) initially reads _Result.ptr (this is past-the-end of what to_chars() wrote), and @barcharcraz has a fix. 🎉

This sporadically failed because the garbage character being read is almost always not the exponent character ('p' for hex, 'e' for scientific), but if it is, the code gets confused about where the exponent starts, leading to catastrophe shortly afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed Something works now, yay! format C++20/23 format
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants