[swift-dev] [Swift 2.2] Request to merge pull request: Perform a dynamic method call if a class has objc ancestry

[Ted Kremenek]

Approved.  I’ll handle the merge with the pull request.

> On Feb 2, 2016, at 9:54 AM, Arnold Schwaighofer <aschwaighofer at apple.com> wrote:
> 
> https://github.com/apple/swift/pull/1159
> 
> Perform a dynamic method call if a class has objc ancestry in specula
> tive devirt as fallback.
> 
> If a class has an @objc ancestry this class can be dynamically overridden and
> therefore we don't know the default case even if we see the full class
> hierarchy.
> 
> rdar://23228386
> 
> Explanation:
> 
> Before this change we would devirtualize a method call to static calls of the potential call targets without a fallback to a class method lookup if we believed to have the full class hierarchy e.g in WMO mode. But during runtime this assumption can be violated because an objective-c class can be dynamically extended and so we would end up calling through the wrong method.
> 
> private class A : NSObject {
> func foo() {...}
> }
> private class B : A { 
> override foo() {...}
> }
> 
> Before:
> 
> callAnA(a : A) {
> if (a isa A) {
>  A.foo(a)
> } else {
>  B.foo(a)
> }
> }
> 
> After:
> 
> callAnA(a : A) {
> if (a isa A) {
>  A.foo(a)
> } else if (a isa B) {
>  B.foo(a)
> } else a.foo(a) // call through class method table.
> }
> 
> Scope:
> 
> The change only effects whether we emit a default case that calls through the class method table. Emitting the call through the class method table is always safe. This risk is low.
> 
> Testing:
> 
> There is a unit test testing the change, furthermore the change was tested in the project reported in rdar://23228386 and only with this change the test scenario in the project works.
> 
> Reviewed by:
> Roman, the author of the speculative virtualization pass, and Slava also took a look at it.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-dev/attachments/20160202/1cd94b90/attachment.html>