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

[SR-5961] JSONSerialization does not properly convert 1.1 #4412

Open
swift-ci opened this issue Sep 22, 2017 · 8 comments
Open

[SR-5961] JSONSerialization does not properly convert 1.1 #4412

swift-ci opened this issue Sep 22, 2017 · 8 comments

Comments

@swift-ci
Copy link
Contributor

Previous ID SR-5961
Radar None
Original Reporter rexmas (JIRA User)
Type Bug

Attachment: Download

Environment

Mac OS 10.12.6
MacBook Pro (Retina, 15-inch, Late 2013)

iOS 11

Xcode 9

Swift 4 and Swift 3.2

Additional Detail from JIRA
Votes 1
Component/s Foundation, Standard Library
Labels Bug
Assignee None
Priority Medium

md5: fb6921cbf1fa090db8d7f39e528d14b4

relates to:

  • SR-106 description returns a rounded value of Double
  • SR-454 Reimplement float -> string with better algorithms
  • SR-7195 JSONSerialization encodes Double differently on Darwin and Linux

Issue Description:

type this in a Playground, see "[1.1000000000000001]"

let options = JSONSerialization.WritingOptions(rawValue: 0)
let data = try JSONSerialization.data(withJSONObject: [1.1], options: options)
let string = String(data: data, encoding: .utf8)
@belkadan
Copy link

Floating-point numbers can't represent 1.1 exactly, and that's what you're seeing. However, we could always consider switching our printed representation to "shortest equivalent float" instead of "X digits of precision", either for just JSONEncoder or for all printing of floats. @moiseev, @itaiferber, what do you think?

(There's a bit of a danger here because "shortest equivalent float" is different for Float and Double.)

@itaiferber
Copy link
Contributor

We can do that, though folks are still going to get numbers which are "unexpected" because floating-point numbers are non-intuitive. If we decide on a solution, I think we should apply it to numbers pretty much across the board — we just discovered a ton of unit tests in swift-corelibs-foundation for floating-point round-tripping that are straight up incorrect because the default implementation for numbers hard-codes 15 digits of precision which causes a lot of numbers to print incorrectly. The behavior on Linux is different than on Darwin and "looks" correct, but for the wrong reason...

@itaiferber
Copy link
Contributor

Regardless, this isn't a Swift issue, but an `NSJSONSerialization` issue.

@belkadan
Copy link

Well, Swift's default printing for Float and Double could do this too. Maybe that's a separate issue, though.

@itaiferber
Copy link
Contributor

@belkadan Agreed — I think we can do this across the board in a consistent way; worth following up on separately though.

@tbkka
Copy link
Contributor

tbkka commented Mar 8, 2018

To solve SR-106, I'm preparing a new implementation of Double.description (and Float and Float80 as well) that produces much better quality in less time.

That won't solve the NSJSONSerialization issue, of course. Someone else will have to do the work to adopt similar algorithms in that layer.

@tbkka
Copy link
Contributor

tbkka commented Mar 24, 2018

Here's the Pull Request for the new Double.description. Besides being a lot faster, this fully achieves the "shortest equivalent float" behavior that Jordan described.

apple/swift#15474

`NSJSONSerialization` should adopt this code (or code like it).

@tbkka
Copy link
Contributor

tbkka commented Apr 2, 2018

I disagree that this is actually a bug. The only critical requirement for JSON serialization is round-trip correctness. That is, parsing back should produce exactly the same floating-point value (according to `==`). Extra digits that don't affect round-trip correctness are at worst a minor performance concern (because the output is larger than necessary).

However, the new `Double.description` implementation does now generate the "shortest equivalent form" and could be used to address the concern here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants