swiftlang / swift

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

[SR-103] Protocol Extension: function's implementation cannot be overridden by a subclass #42725

Open swift-ci opened 8 years ago

swift-ci commented 8 years ago
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:

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 commented 7 years ago

Comment by Rex Fenley (JIRA)

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

belkadan commented 7 years ago

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 commented 7 years ago

Comment by Dale Buckley (JIRA)

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

belkadan commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

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 commented 5 years ago

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 commented 5 years ago

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

allevato commented 5 years ago

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 commented 5 years ago

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