Fixing up various roundtripping/serialization tests that are impacted by the double/single tostring and parse changes #32268
Conversation
tannergooding
changed the title
|
Is this para in the number formatting doc still accurate "For Double values, the "R" format specifier in some cases fails to successfully round-trip the original value. For both Double and Single values, it also offers relatively poor performance. Instead, we recommend that you use the "G17" format specifier for Double values and the "G9" format specifier to successfully round-trip Single values." |
|
BTW is round tripping ever unstable, ie, round trip the output again and you get a third value |
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(1.1, "1.1000000000000001")] |
There was a problem hiding this comment.
So previously round-tripping "1.1" with "R" gave "1.1" and after this change it gives "1.1000...01"? And that is because we changed the algorithm to give better round tripping in other cases? Even if it is a strict improvement in more cases we will definitely get bug reports.. Cc @jkotas
There was a problem hiding this comment.
Yes, the IEEE spec requires you to return the full set of digits (9 for single and 17 for double) in order to guarantee the number can always round-trip successfully.
We are going to be getting bugs either way. We already had a number of issues related to parsing, formatting, and round-tripping values behaving incorrectly: dotnet/coreclr#1316, dotnet/coreclr#3313, dotnet/coreclr#13106, dotnet/coreclr#13615, dotnet/coreclr#16783, dotnet/coreclr#17467 (with even more issues on stackoverflow, uservoice, etc)
Many of these issues have caused people to write their own implementations (such as Roslyn) or provide their own workarounds (such as various bits of serialization code in CoreFX).
For round-tripping specifically, any effort to provide "prettier" numbers will slow down the implementation and is going to be prone to bugs (as was the case already). However, as part of the updates to make things more IEEE compliant, users will have expanded capabilities that will let them get correct results in the format they desire.
There was a problem hiding this comment.
For round-tripping specifically, any effort to provide "prettier" numbers will slow down the implementation
The "prettier" number are shorter strings. If you give me 6x longer string for 1.1 than required, I have to send 6x more bytes over the wire. Unclear whether there is anything actually saved at the end - depends on a lot of factors.
| { | ||
| var objs = new RectangleF[] | ||
| { | ||
| new RectangleF(1.50001f, -2.5000f, 1.5000f, -2.5000f) |
There was a problem hiding this comment.
Or maybe that's how VS does it
There was a problem hiding this comment.
Most of the badly formatted code here was just copied verbatim from the existing testcase that covers netfx.
A good bit of the code could do with cleanup/refactoring.
The docs will need updated in a few places. I indicated as such here: dotnet/coreclr#19905 (comment) |
No, roundtripping should be completely stable. |
|
Closing this for the time being. Will re-open after the corresponding CoreCLR PR gets cleaned up. |
This fixes up all the tests that are impacted by dotnet/coreclr#19905