swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.49k stars 10.35k forks source link

[SR-3022] SWIFT Optionals + Timer Class #45612

Open swift-ci opened 8 years ago

swift-ci commented 8 years ago
Previous ID SR-3022
Radar rdar://problem/28924332
Original Reporter antony.newman (JIRA User)
Type Bug
Environment Latest Xcode on MacPro
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 3c09f7c5aaf86b05d193736f38542036

Issue Description:

//Code that crashes in Playground : Comment out 'Timer' and it does not Crash.

import UIKit

class STRING_TEST : Timer
{
    var null_string  : String? = "TEST"

    func crash_Swift()
    {
        print("THIS CRASHES SWIFT : \(null_string!)")
    }
}

let timer = STRING_TEST()

timer.crash_Swift()
belkadan commented 7 years ago

I don't think (NS)Timer is intended to be subclassed, which might explain this behavior. @phausler?

belkadan commented 7 years ago

(To be clear, the "crash" here is that null_string is not being properly initialized, and the program traps trying to unwrap it.)

phausler commented 7 years ago

NSTimer can certainly be subclasses in Objective-C, however it's DI has a fatal error.

- (id)initWithFireDate:(NSDate *)date interval:(NSTimeInterval)ti target:(id)t selector:(SEL)s userInfo:(id)ui repeats:(BOOL)rep {
    // effectively
   abort()
    return nil;
}

so the subclasser would normally just override this as such in objc

- (id)initWithFireDate:(NSDate *)date interval:(NSTimeInterval)ti target:(id)t selector:(SEL)s userInfo:(id)ui repeats:(BOOL)rep {
    self = [super init];
    return self;
}

This does not seem to be possible with the current annotations in swift.

belkadan commented 7 years ago

That's, um, not a valid DI. The definition of a DI is "an initializer you may call from your subclass initializer, and which you must override if you want convenience initializers". Is init() the real DI for NSTimer?

(Swift's ability to correctly inherit initializers does indeed depend on the designated initializer annotations being correct in header files.)

The documentation page for Timer says "You should not attempt to subclass NSTimer", so I think it's probably fine to say this is NTBF, but otherwise it might be a Foundation bug.

phausler commented 7 years ago

Also it does not really make sense to initialize a timer with out any contextual information. I would claim that -[NSTimer init] should be marked as an invalid initializer. (but that would need to be paired with a more appropriate initWithFireDate:interval:target:selector:userInfo:repeats: method body)

phausler commented 7 years ago

I think the documentation warning is a warning in the regards that it is probably nearly impossible to actually get a subclass to do anything useful; so likely it hasn't gotten any audits for DIs. However that being said - given enough overrides almost every Cocoa class can be subclassed. Some of which, in the case of Timer, would require compatible ivar layout from private APIs to schedule it into the run loop. So in practice it probably is effectively final.

Just so I have the proper fodder to the API owners, do we have an ability to make classes final in swift by annotation or apinotes?

belkadan commented 7 years ago

I don't think so at the moment. We've actually needed something like that in Objective-C when we want to mark a class as having no public designated initializers, or possibly no initializers at all.

(At the risk of bringing the "free subclassing" crowd down on our heads, I think we would implement it by just importing those classes as public instead of open. That's not technically correct because there may be subclasses in other Objective-C modules, but it's probably close enough.)

swift-ci commented 7 years ago

Comment by ANTONY NEWMAN (JIRA)

Here's another example where people unfamiliar with this might be surprised ... the following will write "False" to the screen.

=============================================
import UIKit

class test_class : Timer
{
var true_var : Bool = true
}

let c = test_class()
print("true_var = (c.true_var)")