microsoft / STL

MSVC's implementation of the C++ Standard Library.
Other
10.23k stars 1.51k forks source link

`<format>`: Alternate form general floating-point can mishandle width #5011

Open StephanTLavavej opened 1 month ago

StephanTLavavej commented 1 month ago

Reported to me by Attila Feher. https://godbolt.org/z/djh5KxjW6

#include <format>
#include <iostream>
using namespace std;

int main() {
    std::cout << "Expected: [1.e-37]\n";
    std::cout << format("  Actual: [{:#6.0g}]\n", 1.234e-37);
}

libstdc++ and libc++ behave as expected, but MSVC prints:

Expected: [1.e-37]
  Actual: [ 1.e-37]

:detective: Analysis

I debugged into this, and although I'm not absolutely certain of the root cause yet, this line appears to be involved:

https://github.com/microsoft/STL/blob/926d458f82ae1711d4e92c0341f541a520ef6198/stl/inc/format#L2278

_Zeroes_to_append is computed as -1 because _Extra_precision and _Precision are both 0, and _Digits is 1.

We've correctly computed _Width to be 6, but the negative _Zeroes_to_append damages it, which appears to be the proximate cause of the bug:

https://github.com/microsoft/STL/blob/926d458f82ae1711d4e92c0341f541a520ef6198/stl/inc/format#L2297

Later, we use _Zeroes_to_append in a clamped manner (i.e. negative values are treated as zero):

https://github.com/microsoft/STL/blob/926d458f82ae1711d4e92c0341f541a520ef6198/stl/inc/format#L2328-L2330

However, we need to completely understand the problem and all possible codepaths before developing a fix (i.e. we can't just clamp it to zero and call it a day). Note that the chars_format::general codepath further adjusts _Zeroes_to_append:

https://github.com/microsoft/STL/blob/926d458f82ae1711d4e92c0341f541a520ef6198/stl/inc/format#L2278-L2289

cpplearner commented 1 month ago

For the g specifier (which corresponds to chars_format::general, which corresponds to printf's %g), a precision of 0 should behave as if it were 1. It seems that we are missing this adjustment.

StephanTLavavej commented 1 month ago

Ah, great catch! We correctly handle that in <charconv>, so it seems that <format> is missing the same logic:

https://github.com/microsoft/STL/blob/926d458f82ae1711d4e92c0341f541a520ef6198/stl/inc/charconv#L2346-L2356