search

bignerdranch / Freddy

A reusable framework for parsing JSON in Swift.
MIT License
1.09k stars 120 forks source link

Compile under Swift 4 #252

Closed volodg closed 7 years ago

volodg commented 7 years ago

This applies minimal changes to allow Freddy to compile under Swift 4. It polyfills the Swift 4 overflowing Int add/multiply API for Swift 3.

It also tweaks some code to treat Dictionary.map's tuple argument as a single argument under both Swift 3 and 4, since Swift 4 won't automatically "splat" it into two separate arguments in the transform block.

jeremy-w commented 7 years ago

Thanks!

The fewer version-conditional blocks we can have, the better. There already aren't a ton, but it looks like we can eliminate most of them by using the version you proposed for Swift 4 even under Swift 3, by keying into the tuple rather than relying on automatic tuple unpacking.

The overflow-related code is a significant API shift. I'd prefer to reduce the amount of duplication across version branches for what's really just a method call, but I'd want to see what sort of impact, say, shimming Swift 3 to look like Swift 4, or indirecting through a multiply-and-gripe API we control, might have on performance.

jeremy-w commented 7 years ago

I'm updating this to track the latest master, and then it should be good to merge once Travis gives the A-OK. The review comments can be addressed later.

Thanks again! :)

jeremy-w commented 7 years ago

@volodg Getting compiler errors. Maybe the new IntExtensions shim file didn't get added to the targets?

❌  /Users/travis/build/bignerdranch/Freddy/Sources/JSONParser.swift:459:56: value of type 'Int' has no member 'multipliedReportingOverflow'

guard case let (exponent, .none) = 10.multipliedReportingOverflow(by: value) else {

                                                       ^~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~

❌  /Users/travis/build/bignerdranch/Freddy/Sources/JSONParser.swift:483:52: value of type 'Int' has no member 'multipliedReportingOverflow'

guard case let (signedValue, .none) = sign.rawValue.multipliedReportingOverflow(by: value) else {

                                              ~~~~~^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
volodg commented 7 years ago

@jeremy-w you are right, fixed