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-103] Protocol Extension: function's implementation cannot be overridden by a subclass #42725

Open
swift-ci opened this issue Dec 7, 2015 · 11 comments
Assignees

Comments

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Dec 7, 2015

Previous ID SR-103
Radar rdar://problem/21141185
Original Reporter tarunon (JIRA User)
Type Bug
Environment

Reported: Mac OSX 10.11.1, Swift 2.2-dev

Tested & remains an an issue on:

  • macOS 10.14, Swift 4.2 - 25th September 2018

  • macOS 10.14.1, Swift 5.1 (Xcode 11 beta 5) - 19th August 2019

Additional Detail from JIRA
Votes 29
Component/s Compiler
Labels Bug
Assignee @slavapestov
Priority Medium

md5: 57cd9bdded34d37cb640b9c376cca165

is duplicated by:

  • SR-118 Protocol extensions cannot be overridden in subclasses
  • SR-1271 Protocol extension method being called over subclass implementation.
  • SR-2119 Protocol methods of superclass should be treated as superclass API in subclasses
  • SR-2859 Cannot override methods from a default protocol extension in a subclass of a class that relies on the default implementation
  • SR-3616 Default implementation being called when specialized one should be picked
  • SR-5654 Class vtables should have entries for default implementations
  • SR-6681 A subclass of a base class which conforms to a protocol with a requirement fulfilled by a default implementation cannot override the base class' method
  • SR-7129 Inconsistent dispatch with protocol extensions
  • SR-8473 Broken protocol/class interaction
  • SR-10443 Dynamic dispatch is sometimes ignored when passing a subclass object to a function that takes a protocol argument
  • SR-11398 Self type does not behave correctly in subclasses of a protcol implementation

Issue Description:

A function(x') is not called that we thought If there is following conditions.

  1. A protocol(A) is defined a function(x).

  2. A has implement of x in the protocol extension.

  3. A class(B) is implement A, but doesn't have implement x.

  4. A subclass(C) is inheritance B, and have implement x(x').

  5. A instance of C typed A, and call x.

// Defined protocol.
protocol A {
    func a() -> Int
}
extension A {
    func a() -> Int {
        return 0
    }
}

// A class doesn't have implement of the function.
class B: A {}

class C: B {
    func a() -> Int {
        return 1
    }
}

// A class has implement of the function.
class D: A {
    func a() -> Int {
        return 1
    }
}

class E: D {
    override func a() -> Int {
        return 2
    }
}

// Failure cases.
B().a() // 0
C().a() // 1
(C() as A).a() // 0 # We thought return 1. 

// Success cases.
D().a() // 1
(D() as A).a() // 1
E().a() // 2
(E() as A).a() // 2
@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Mar 1, 2017

Comment by Rex Fenley (JIRA)

@belkadan Is this on the roadmap to get fixed in swift 3.1?

@belkadan
Copy link
Contributor

@belkadan belkadan commented Mar 1, 2017

At this point, probably not. It's not a regression and it can generally be worked around, and 3.1's already far enough along that we don't want to destabilize it.

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Mar 3, 2017

Comment by Dale Buckley (JIRA)

@belkadan Is this worth a proposal for this capability to be added to Swift 4?

@belkadan
Copy link
Contributor

@belkadan belkadan commented Mar 3, 2017

I don't think it needs a proposal. We already consider it a bug. It's just not something anyone's gotten around to.

(Double-checking with @DougGregor that we consider it a bug.)

@DougGregor
Copy link
Member

@DougGregor DougGregor commented Mar 3, 2017

I do feel like it's a bug, but it is a source-breaking change and a semantics change, so there are benefits to going through the swift-evolution process here.

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Mar 3, 2017

Comment by Dale Buckley (JIRA)

Ok, well I've started to put a proposal together but I'm out of the country for the next week. I'll complete it when I get back and drop it into the evolution mailing list for discussion.

Thanks for the feedback.

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Sep 18, 2018

Comment by Dale Buckley (JIRA)

After being prompted on the swift forums (https://forums.swift.org/t/create-sticky-to-remind-people-to-retest-bugs-with-new-swift-releases/11539) I decided to look at this issue again and found it still exists in Swift 4.2.

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Nov 20, 2018

Comment by Bryan Anderson (JIRA)

This is still an issue that I see as a bug. Very unpredictable and requires a lot of careful consideration. Couldn't we just use @objc to force dynamic dispatch?

@belkadan
Copy link
Contributor

@belkadan belkadan commented Nov 26, 2018

No, because not every protocol requirement has a signature compatible with @objc.

@allevato
Copy link
Collaborator

@allevato allevato commented Jun 5, 2019

This bug caused quite a bit of pain for swift-format due to otherwise innocent SwiftSyntax changes.

Old SwiftSyntax had `SyntaxVisitor` class with stubbed out `visit` methods. We subclassed it to create a common `LintRule` class (which overrode none of those methods), and specific rules were implemented by subclassing `LintRule` and overriding the appropriate methods. When SwiftSyntax changed `SyntaxVisitor` into a protocol with default implementations, all of those rules broke because the non-default implementations stopped getting called. Unfortunately there's not a great workaround here because there are hundreds of `visit` methods that we'd need to re-stub-out in the common base class to fix the dispatching.

(This ended up having bigger implications for the design/performance of our rule pipeline since replacing a common base class with a protocol had some unfortunate interactions with an `inout` argument... but I'll spare everyone the gory details 🙂 )

Is there a chance this bug might be revisited in the future?

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Aug 20, 2019

Comment by Dale Buckley (JIRA)

I've highlighted this issue for discussion on the Swift evolution forums: https://forums.swift.org/t/default-protocol-implementation-inheritance-behaviour-the-current-situation-and-what-if-anything-should-be-done-about-it/28049

@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
Projects
None yet
Development

No branches or pull requests

4 participants