Fixed TimeZone.init(secondsFromGMT:) failability

[bubski]

TimeZone.init(secondsFromGMT:) made not failable.

Aligns with https://developer.apple.com/documentation/foundation/nstimezone/1387199-init

Fixed TimeZone.init(secondsFromGMT:) failability

[ianpartridge]

I think this should be failable?

https://developer.apple.com/documentation/foundation/timezone/2293718-init

[bubski]

@ianpartridge Ah, you're right. Let's keep it consistent.

I don't understand why it should be failable though. Do you?

[ianpartridge]

No I don't know, sorry. Possibly there's a difference in the implementation on Darwin which means it might fail.

[parkera]

let tz = TimeZone(secondsFromGMT: 99999999) -> nil

[parkera]

Here's the relevant line from the Darwin implementation:

if (seconds < -18 * 3600 || 18 * 3600 < seconds) return nil;

[bubski]

Ah, interesting.
@parkera Thanks for the context!

Now I see that NSTimeZone(forSecondsFromGMT: 99999999) throws an exception on Darwin's Foundation.
It doesn't on swift-corelibs-foundation, but returns an invalid instance.

also

let tz = TimeZone(secondsFromGMT: 99999999)
tz == nil // false
// And subsequent usage of tz throws exceptions

I assume that's something that needs to be resolved, right?

[bubski]

I'm reopening this with a half-way solution.

I'm not sure if it's possible to work around not failable NSTimeZone.init(forSecondsFromGMT:) in Swift, and reproduce Darwin's behavior, which comes from Objective-C interop.

The least we can do I guess, is making TimeZone.init(secondsFromGMT:) fail when expected.

@ianpartridge @parkera please let me know if my reasoning is correct here.

[ianpartridge]

@parkera should we open a Radar if TimeZone(secondsFromGMT:) should be marked failable on Darwin?

[parkera]

So basically, NSTimeZone has the wrong nullability annotation on Darwin. It does not throw an exception (in Foundation terms); it just returns nil then trying to use it causes a crash as Swift aborts due to using a nil but non-nil result.

This is something that I think I realized when writing the struct TimeZone API, which is why that one is correctly nullable (https://github.com/apple/swift/blob/master/stdlib/public/SDK/Foundation/TimeZone.swift#L71).

For now, for source compatibility, the proposed patch makes sense to me.

[parkera]

And yah, if a radar is filed I can track it -- although frankly I want people to use the struct type here and not the ref type. :)

[ianpartridge]

@parkera Radar 33650342 raised for the incorrect nullable annotation on NSTimeZone.init(forSecondsFromGMT:) on Darwin.

[ianpartridge]

@swift-ci please test and merge