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-584] Unexpected behavior when overriding a superclass’ method in an extension #43201

Closed
marcomasser opened this issue Jan 19, 2016 · 4 comments

Comments

@marcomasser
Copy link

@marcomasser marcomasser commented Jan 19, 2016

Previous ID SR-584
Radar None
Original Reporter @marcomasser
Type Bug
Additional Detail from JIRA
Votes 3
Component/s Compiler
Labels Bug
Assignee None
Priority Medium

md5: 251836798d8c1e10c3054964af248ffc

is duplicated by:

  • SR-923 override a property of a NSObject subclass generates inconsistent behavior

Issue Description:

There’s a case where a property (or function) override in a subclass’ extension leads to unexpected behavior:

class Base: NSObject {
    var directProperty: String { return "This is Base" }
    var indirectProperty: String { return directProperty }
}

class Sub: Base { }

extension Sub {
    override var directProperty: String { return "This is Sub" }
}

Accessing directProperty works as expected:

Base().directProperty // “This is Base”
Sub().directProperty // “This is Sub”

Accessing “indirectProperty”, shows the unexpected behavior:

Base().indirectProperty // “This is Base”
Sub().indirectProperty // “This is Base” <- Unexpected!

Adding the dynamic keyword to Base.directProperty resolves the issue, so it seems like the implementation of indirectProperty always accesses directProperty of the Base class directly instead of dispatching dynamically.

I think this is a bug, either because this doesn’t work as expected at runtime, or because it compiles at all. Because the Language Guide for Swift 2.1, chapter “Extensions” states in a note near the beginning:

“Extensions can add new functionality to a type, but they cannot override existing functionality.”

https://developer.apple.com/library/ios/documentation/Swift/Conceptual/Swift_Programming_Language/Extensions.html#//apple_ref/doc/uid/TP40014097-CH24-ID151

That sounds to me like the override keyword must not appear in an extension. Curiously enough, the above code compiles and some parts of it even run without issues despite violating the Language Guide. If Base does not inherit from NSObject but becomes a root class instead, the compiler produces the following error in the extension:

Declarations in extensions cannot override yet

Note the “yet”. Seems like this is just not done yet. Also, I don’t understand how this feature could work except if overriding properties and methods in a subclass’ extension will only be supported if the original property or method was declared as dynamic. Otherwise the compiler could inline their implementation and the extension’s implementation would never be called – assuming the extension isn’t known at compile time, as could be the case for extensions to classes of system frameworks. Everything’s probably fine with Whole Module Optimization if the module contains all the classes and extensions.

Anyways, if this will be supported for non-Objective-C classes in the future, the above bug could point to something that doesn’t work correctly.

Also note that this could be related to (but not the same as) a bug reported already where methods implemented in a protocol extension cannot be overridden by a conforming class’ subclass:
https://bugs.swift.org/browse/SR-103

I posted this to the Swift Users mailing list but got no response:
https://lists.swift.org/pipermail/swift-users/Week-of-Mon-20151228/000670.html

@belkadan
Copy link
Contributor

@belkadan belkadan commented Jan 19, 2016

Yep, extension behavior isn't quite right yet: in some cases it should be more restrictive, but in this case (all one module) it should be allowed.

(I'm guessing we missed your e-mail because (a) traffic's been very high, and (b) it was sent between Christmas and New Year's when most Apple engineers were on break.)

@marcomasser
Copy link
Author

@marcomasser marcomasser commented Jan 20, 2016

Don’t worry, I suspected that the timing of my message on the mailing list was what caused it to go unnoticed!

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Dec 15, 2016

Comment by Brian (JIRA)

Hey – I was interested in investigating this. I think I could generate a warning in `DeclChecker::recordOverride` if this happened. I'm not sure what approach I could take to make it work if it is the declaring module. I'm guessing that I could call `setDynamic` on the FuncDecl, but I'm not sure. It may be that there's a later phase where the overridden function would be marked as dynamic. But I think generating a warning regardless would be a worthy PR, since it would be needed for the multi-module case anyway.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@AnthonyLatsis
Copy link
Collaborator

@AnthonyLatsis AnthonyLatsis commented Jun 24, 2022

The example expectedly errors out with non-@objc property 'directProperty' declared in 'Base' cannot be overridden from extension now—closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants