Fix ostream << floating_point not correctly handling precision #1173
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
BillyONeal
left a comment
BillyONeal
left a comment
There was a problem hiding this comment.
Left some perf nitpicks and a question
1. Hexfloat output ignores precision now as required by the standard. 2. Precision of zero is now correctly passed to sprintf. 3. Fixed output with negative precision no longer crashes.
statementreply
force-pushed
the
hexfloat_precision
branch
from
1e6cd04 to
98f8e7a
Compare
|
Reverted ABI breaking changes and force pushed. |
statementreply
changed the title
StephanTLavavej
left a comment
StephanTLavavej
left a comment
There was a problem hiding this comment.
Wow, thanks for finding and fixing the additional bugs here! I wasn't too surprised by the hexfloat bug (hexfloats being relatively "new" and obscure to most users), and the negative precision crash involves a pathological input, but I still can't quite believe that the zero-precision bug for scientific and general has been lurking all this time.
Your new logic makes sense and I've verified that its behavior matches Clang's on Compiler Explorer, modulo the Windows/Linux difference in %a interpretation (the Windows UCRT prints out all 13 hexits, while Linux trims trailing 0 hexits; both are permitted by the C Standard).
Runtime behavior changes in floating-point formatting have the potential to be problematic from a compatibility perspective (the UCRT has struggled endlessly with this), but (1) users seem to find STL behavior changes far less problematic (I have previously fixed bugs in the early VS 2015 timeframe with zero user complaints), (2) I highly doubt that anyone will complain about the hexfloat behavior change, and (3) the zero-precision change also seems unlikely to pose compatibility issues, plus there is a workaround. (If someone was depending on our earlier behavior, they can simply pass a precision of 6.) The usual reason users have compatibility issues is when they're serializing results to a database or something and want all of the digits to be the same (even if the digits are wrong). However, since 6 is the default, I find it hard to imagine why a user would be setting the precision to 0.
|
Aha - the zero-precision bug was noticed by a single user, four years ago, and the bug report got buried in our backlog. 😿 |
|
Thanks again for the precise fixes! 😹 |
GitHub action runners now feature Visual Studio 16.8.1 (previously: 16.7). According to its STL changelog, this update includes the following change: microsoft/STL#1173 (review) Therefore update the unit test to expect 13 trailing zeros.
GitHub action runners now feature Visual Studio 16.8.1 (previously: 16.7). According to its STL changelog, this update includes the following change: microsoft/STL#1173 (review) Therefore update the unit test to expect 13 trailing zeros for MSVC version >= 19.28.
1. Add /permissive flag to MSVC unit test compilation. As of 16.8, MSVC implicitly adds /permissive- when using /std:c++latest. See: https://docs.microsoft.com/en-us/cpp/build/reference/permissive-standards-conformance?view=msvc-160 It seems that with this flag, MSVC fails to compile user-defined literals when prefixed with a negative sign, e.g. "-100_u8". 2. MSVC previously respected a stream's precision using std::hexfloat, which is non-compliant with the spec (std::hexfloat is meant to ignore precision). Unfortunately it does not behave the same as Clang and GCC still; MSVC will always zero-pad a fixed amount. See: microsoft/STL#1173
1. Add /permissive flag to MSVC unit test compilation. As of 16.8, MSVC implicitly adds /permissive- when using /std:c++latest. See: https://docs.microsoft.com/en-us/cpp/build/reference/permissive-standards-conformance?view=msvc-160 It seems that with this flag, MSVC fails to compile user-defined literals when prefixed with a negative sign, e.g. "-100_u8". 2. MSVC previously respected a stream's precision using std::hexfloat, which is non-compliant with the spec (std::hexfloat is meant to ignore precision). Unfortunately it does not behave the same as Clang and GCC still; MSVC will always zero-pad a fixed amount. See: microsoft/STL#1173
|
As part of a fuller test of Round-tripping decimal-binary-decimal on several compilers using iostream and charconv - see roundtripping tests Jan2021.txt](https://github.com/microsoft/STL/files/5764778/roundtripping.tests.Jan2021.txt) I am pleased confirm that hex format round-tripping hexfloat now works correctly using MSVC 16.9.0 preview 1. |
7 participants
[facet.num.put.virtuals]/5
Zero or negative precision is now correctly passed toImplement LWG-231.sprintf.Fixes #1125
Fixes AB#847068