swiftlang / swift

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

[SR-1715] NSProxy can not be subclassed #44324

Open swift-ci opened 8 years ago

swift-ci commented 8 years ago
Previous ID SR-1715
Radar None
Original Reporter manuyavuz (JIRA User)
Type Bug
Environment Xcode 7.3.1 default toolchain
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug, DiagnosticsQoI, StarterBug | |Assignee | step (JIRA) | |Priority | Medium | md5: 6ce9ad56a254833eb119945d8b995454

Issue Description:

Hi,

The following simple code

class MyProxy: NSProxy {
  init(value : Int) {
  } // Compiler error here
}

gives

Super.init isn't called on all paths before  returning from initializer

compiler error.

I can't call super.init() because NSProxy class does not have one.

This means a Swift type can not be subclasses from NSProxy class.

Is this intentional? If so, I think this class should be unavailable while writing Swift code, or at least have a warning in documentation or code reference.

belkadan commented 8 years ago

It is intentional, since Swift does not really use the Objective-C runtime, but I'm not sure what we can do to warn about it. You can still (barely) use NSProxy instances that come from Objective-C; there's just no effective way to subclass them.

That said, NSProxy isn't the only type that doesn't have any public designated initializers, though we don't have a good way to mark that for subclasses in Objective-C. So maybe that's the diagnostic we need: "cannot subclass 'NSProxy' because it has no designated initializers".

swift-ci commented 7 years ago

Comment by Matt Gambogi (JIRA)

Hi, I'd like to take a pass at this, but I wanted to make sure I take the right approach.
I think this change requires:

Thanks

belkadan commented 7 years ago

I think we'd rather just do this ahead of time, in the Sema library. In TypeCheckDecl.cpp's visitClassDecl, there's already a section that checks for various problems with the superclass; it could also check that there exists at least one designated initializer. (You can do this simply by looking up members named init and seeing if any of them are ConstructorDecls that are designated.)

Alternately, now that we have a distinction between public and open classes, the ClangImporter library could do this check and mark the class as merely public if it has no methods that would be imported as initializers. That might be a little harder, though, since we'd want to do it without actually importing the methods to check.

swift-ci commented 6 years ago

Comment by Step Christopher (JIRA)

I've added a new Sema error, "inheritance_from_class_with_no_designated_initializers". In visitClassDecl, I'm checking for ConstructorDecls that are designated as Jordan suggested.

I have a couple places where tests might make sense. One test case is specific to NSProxy:

```class NotInitializable : NSProxy { // expected-error{{cannot inherit from class 'NSProxy' because it has no designated initializers}} ```

Not 100% sure of the right place for this test. I currently have it in attr_requires_stored_property_inits.swift for convenience of getting started, but am looking for other initialization tests that it would make sense to be near.

But I want to add at least one test that is more general. I have a test that makes a class with a private init, and a subclass with no super.init. This crosses into other territory (like errors for adding a super.init, or errors for superclass not having an available init, SR-2484). Should I just ... not add this test? Or is there a way to construct a non-Swift type with no designated init, import that to the test ... that sounds like it might not be worth it. Any feedback / suggestions? Or should I put up a PR and go from there?

swift-ci commented 6 years ago

Comment by Step Christopher (JIRA)

WIP PR is up: https://github.com/apple/swift/pull/14024

There are a couple open questions around the tests to wrap this up.