[SR-695] Loophole in Self-requirement leads to crash

[swift-ci]

Previous ID	SR-695
Radar	None
Original Reporter	DarkDust (JIRA User)
Type	Bug
Status	Resolved
Resolution	Done

Environment

Swift version 2.2-dev (LLVM 3ebdbb2c7e, Clang f66c5bb67b, Swift 42591f7)
Apple Swift version 2.1.1 (swiftlang-700.1.101.15 clang-700.1.81)

Additional Detail from JIRA

Votes	1
Component/s	Compiler
Labels	Bug
Assignee	@gregomni
Priority

md5: 1b9fffb15b9c311a34f50726c4f182f5

is duplicated by:

• SR-988 Undiagnosed type error in subclass function with Self return type

Issue Description:

A StackOverflow question pointed out that there is a problem with Self-requirement in protocols that can lead to wrong results or crashes. Consider this example from the question:

// The protocol with Self requirement
protocol Narcissistic {
    func getFriend() -> Self
}

// Base class that adopts the protocol
class Mario : Narcissistic  {
    func getFriend() -> Self {
        print("Mario.getFriend()")
        return self;
    }
}

// Intermediate class that eliminates the
// Self requirement by specifying an explicit type
// (Why does the compiler allow this?)
class SuperMario : Mario {
    override func getFriend() -> SuperMario {
        print("SuperMario.getFriend()")
        return SuperMario();
    }
}

// Most specific class that defines a field whose
// (polymorphic) access will cause the world to explode
class FireFlowerMario : SuperMario {
    let fireballCount = 42

    func throwFireballs() {
        print("Throwing " + String(fireballCount) + " fireballs!")
    }
}

// Global generic function restricted to the protocol
func queryFriend<T : Narcissistic>(narcissistic: T) -> T {
    return narcissistic.getFriend()
}

// Sample client code

// Instantiate the most specific class
let m = FireFlowerMario()

// The call to the generic function is verified to return
// the same type that went in -- 'FireFlowerMario' in this case.
// But in reality, the method returns a 'SuperMario' and the
// call to 'throwFireballs' will cause arbitrary
// things to happen at runtime.
queryFriend(m).throwFireballs()

The problem here is that SuperMario.getFriend always returns a new instance of SuperMario which is correct at that point.

But when FireFlowerMario derived from SuperMario it inherited the implementation of getFriend that returns a SuperMario instance! The compiler treats the instance as a FireFlowerMario instead and tries to access the nonexistent fireballCount. This either leads to garbage output or crashes.

The compiler should be able to detect that SuperMario.getFriend does not behave as expected when subclassing and issue a warning about this.

[slavapestov]

I think the problem is not even specific to protocols, we simply should not allow non-dynamic Self returns to override dynamic Self returns.

Should be an easy fix if someone wants to take a look ;-)

[lilyball]

I think it's reasonable to allow override func getFriend() -> SuperMario in the case where the class is marked final. The issue here isn't that the declaration specifies SuperMario where Self is expected, it's that the class can be subclassed and the method, when inherited by a subclass, will no longer work.

The benefit of allowing this is final classes can quite legitimately return a new instance of themselves instead of being forced to return self (or of having a required initializer that they use to create the new instance).

[gregomni]

@slavapestov Do you still feel the same way about overriding dynamic Self with non-dynamic (in a non-final class)? It is, indeed, an easy fix to give a diagnostic in that situation.

The reason why I ask is that it makes some previously legal code into a compiler error, and the importance of source compatibility has grown some since early 2016. Is this an ok thing to just PR, or is it now a language change worthy of (shudder) swift-evolution and a proposal?

[slavapestov]

@gregomni we can make the more correct semantics conditional on -swift-version 5. Use ASTContext::isSwiftVersionAtLeast(5) to check. And yeah, we should not allow a non-final class to override a method with non-dynamic Self.

[gregomni]

#12495

[gregomni]

Done for swift 5