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

Decimal parsing ignores significant digits below 1e-49 #375

Closed
gafter opened this issue Feb 10, 2015 · 6 comments · Fixed by #45438
Closed

Decimal parsing ignores significant digits below 1e-49 #375

gafter opened this issue Feb 10, 2015 · 6 comments · Fixed by #45438
Assignees
Labels
4 - In Review A fix for the issue is submitted for review. Area-Compilers Bug Concept-Determinism The issue involves our ability to support determinism in binaries and PDBs created at build time. Language-C# Tenet-Compatibility Violation of forwards/backwards compatibility in a design-time piece.
Milestone

Comments

@gafter
Copy link
Member

gafter commented Feb 10, 2015

See CodeGenTests.DecimalLiteral_BreakingChange unit test.

Compile and Run:

using System;
class C
{
    static void Main()
    {
        Console.WriteLine(5.00000000000000000001e-29m); // 5.0e-29m + 1e-49m [Dev11/Roslyn rounds]
        Console.WriteLine(5.0000000000000000000000000000001e-29m); // 5.0e-29m + 1e-60m [Dev11 rounds, Roslyn does not]
    }
}

Expected:
0.0000000000000000000000000001
0.0000000000000000000000000001

Actual:
0.0000000000000000000000000001
0.0000000000000000000000000000

[ported from TFS DevDiv 568494]
@cston @VladimirReshetnikov

@gafter gafter added Bug Language-C# Area-Compilers 1 - Planning Tenet-Compatibility Violation of forwards/backwards compatibility in a design-time piece. labels Feb 10, 2015
@VladimirReshetnikov
Copy link

This is indeed a bug. Roslyn uses the function decimal.TryParse from BCL to parse decimal literals. There is a bug in that function (BCL team is aware of it, but AFAIK their current plan is not to fix it for back compat reasons). If we stop using decimal.TryParse and implement correct parsing of decimal literals in Roslyn, it would cause behavior in compile-time and runtime to be different.

@theoy theoy added this to the Unknown milestone Feb 12, 2015
@gafter gafter added the help wanted The issue is "up for grabs" - add a comment if you are interested in working on it label Feb 17, 2015
@codespare
Copy link
Contributor

That is not probably sufficient by itself to justify implementing IEEE 754-2008 and introducing decimal32, decimal64 and decimal128, but perhaps another small reason to start looking at the successor of decimal? Those can now be found in Intel C++ compiler, gcc, SAP, and progressing on the C and C++ language tracks for instance. Independently of the added 5-6 significant digits of decimal128 and the optimizations possible with the 64 and 32 bit formats, which have some applications, using a standard representation will eventually avoid a lot of cross-platform headaches.

@gafter
Copy link
Member Author

gafter commented Jun 23, 2015

@codespare You're welcome to create a new issue proposing that if you think it is worth our effort.

@codespare
Copy link
Contributor

It undoubtedly would be at some point after 1.0 in my opinion, so I will certainly give a try at starting to document it as it deserves. I also recently came across a related issue, with the BCL project if my memory is correct, proposing to add some of the additional functions defined in IEEE 754-2008 for float and double.

Regarding the issue at hand, I believe it could be tracing back to https://github.com/dotnet/coreclr/blob/master/src/classlibnative/bcltype/decimal.cpp#L350 , although it is more an inference at this stage as I haven't gone through BCL ParseDecimal.
Another outstanding question is where does 5.0000000000000000000000000000001e-29m comes from and why not 5.00000000000000000000000000000001e-29m? I will check what the C# specification says on the subject.

@gafter gafter added the Concept-Determinism The issue involves our ability to support determinism in binaries and PDBs created at build time. label Mar 30, 2019
@gafter gafter self-assigned this Mar 31, 2019
@gafter gafter modified the milestones: Backlog, 16.2 Mar 31, 2019
@gafter
Copy link
Member Author

gafter commented Apr 1, 2019

See #372 (comment) ; this is now fixed in the BCL.

@gafter gafter removed their assignment Jun 3, 2019
@gafter gafter modified the milestones: 16.2, Backlog Jun 3, 2019
@gafter gafter self-assigned this Dec 21, 2019
@gafter gafter modified the milestones: Backlog, 16.5 Dec 21, 2019
@gafter gafter removed the help wanted The issue is "up for grabs" - add a comment if you are interested in working on it label Dec 21, 2019
@gafter
Copy link
Member Author

gafter commented Dec 21, 2019

Assigning to myself to add a test confirming this is fixed in the BCL that Roslyn runs against.

gafter pushed a commit to gafter/roslyn that referenced this issue Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - In Review A fix for the issue is submitted for review. Area-Compilers Bug Concept-Determinism The issue involves our ability to support determinism in binaries and PDBs created at build time. Language-C# Tenet-Compatibility Violation of forwards/backwards compatibility in a design-time piece.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants