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-1715] NSProxy can not be subclassed #44324

Open
swift-ci opened this issue Jun 9, 2016 · 5 comments
Open

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

swift-ci opened this issue Jun 9, 2016 · 5 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself diagnostics QoI Bug: Diagnostics Quality of Implementation good first issue Good for newcomers

Comments

@swift-ci
Copy link
Collaborator

swift-ci commented Jun 9, 2016

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
Copy link
Contributor

belkadan commented Jun 9, 2016

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
Copy link
Collaborator Author

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:

  • Additional AST error diagnostic definition in include/swift/AST/DiagnosticsSIL.def

  • Some amount of logic checking for public designated initializers public in lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp. This is the part I'm less sure about. What should I inspect to make sure this condition holds? And is there a natural place for this check to be added?

Thanks

@belkadan
Copy link
Contributor

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

Comment by Step Christopher (JIRA)

WIP PR is up: #14024

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

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself diagnostics QoI Bug: Diagnostics Quality of Implementation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants