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

Support parsing snan #11573

Open
tannergooding opened this issue Nov 29, 2018 · 6 comments
Open

Support parsing snan #11573

tannergooding opened this issue Nov 29, 2018 · 6 comments
Labels
area-System.Numerics backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity
Milestone

Comments

@tannergooding
Copy link
Member

The IEEE 754:2008 specification requires that we be able to recover a string matching "snan" (case-insensitive) with an optional preceding sign. We are not required to recover this to an actual snan and are permitted to silently convert it to a qnan (such as double.NaN or float.NaN) instead.

@jkotas raised some concern around this on https://github.com/dotnet/coreclr/issues/19802 given that it may require a new public surface and due to it not being unusual to have NaN checked for by string comparison

At a minimum, we should update the invariant culture to check for snan and return double.NaN or float.NaN. Optionally, and if it is determined to not be breaking, we can add a NumberFormatInfo.SignallingNaN and provide culture specific snan information.

I don't believe either of these should be considered breaking since we are only parsing new values. What could be breaking is if we start formatting snan to NumberFormatInfo.SignallingNaN, as that may break existing code that checks for NaN by string comparison (we currently don't, as IEEE allows us to silently convert a signalling NaN into the quiet nan string: "NaN").

@danmoseley
Copy link
Member

@tannergooding I marked this Future

@tannergooding
Copy link
Member Author

I think future is fine. The actual fix here is pretty trivial, we just need to decide on what to do.

I think, the easiest fix, would be to support this only for the invariant culture and only support it for parsing (that is, you can would never get SNaN as a result of calling ToString()).

  • The spec indicates that we need to support parsing snan, but we can return qnan as the result. It also specifies that we can return just "NaN" as the result of formatting
  • Only supporting it for the invariant culture means we don't need to worry about exposing a new SignallingNaN property on NumberFormatInfo, while still giving users a way to support the value

@jkotas, do you think the above would alleviate the concerns you raised?

@jkotas
Copy link
Member

jkotas commented Feb 1, 2019

I would wait for somebody to ask for this. It does not look important to me to fix.

What does Java or Python do for parsing / formatting snans?

@tannergooding
Copy link
Member Author

Both format quiet and signalling NaN to the same result NaN on Java and nan on Python. Neither look to support parsing snan in any casing and Java looks to be case sensitive when parsing NaN. Both support parsing the sign prefix of + or -.

I agree that it isn't particularly important to fix, but it would be nice to check the box off indicating that we support this particular IEEE requirement, especially since we are allowed to quietly convert it to a quiet NaN (and therefore don't need any special support).

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 26, 2020
@tannergooding tannergooding added area-System.Numerics and removed area-System.Runtime untriaged New issue has not been triaged by the area owner labels Jun 23, 2020
@ghost
Copy link

ghost commented Jun 23, 2020

Tagging subscribers to this area: @tannergooding
Notify danmosemsft if you want to be subscribed.

Copy link
Contributor

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

@dotnet-policy-service dotnet-policy-service bot added backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity labels Dec 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Numerics backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity
Projects
None yet
Development

No branches or pull requests

5 participants