Always try to call the most-derived ObjC protocol method #9716
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, when the Clang importer sees that an imported ObjC class conforms to an ObjC protocol, it creates concrete methods on the class for every method in the protocol hierarchy. These are called "mirror imports". It's a hack that we want to eliminate long-term, but in 4.0, at least, we have to live with it.
Now, a method can be declared multiple times in an ObjC protocol hierarchy, and the signatures don't actually have to line up. ObjC has complex but ultimately somewhat arbitrary rules for picking which method to use for type-checking a message send. Swift, apparently, picks the first one it sees, according to some unspecified walk of the protocol hierarchy, which is even more arbitrary than ObjC. That walk appears to correctly prioritize more-derived methods when the protocol hierarchy is a tree, and probably when the more-derived method dominates the less-derived one, but when there are multiple paths to a protocol it doesn't seem to always honor overriders. Worse, the walk order appears to have changed over time; we have a test case which fails in master but succeeds in previous releases because it now finds the less-derived protocol method instead of the more-derived one.
This patch implements more-principled behavior: it drops methods from the candidate set if they are overridden by another method in the set, then picks the first method remaining.
This is the swift-4.0-branch version of #9681.
This patch can change what method we use to type-check ObjC method calls when the method is not redeclared on the class, which is quite common. Using a different method can have a drastic impact on the type-checking of that call. However, it only actually matters if there are multiple declarations of the method in the protocol hierarchy, and those declarations have different signatures, and we were previously using a less-derived one (which should only arise in complex protocol hierarchies).
This patch fixes rdar://31471034.
The risk of this change is moderate: it affects what types are used in type-checking, and it's certainly possible that there is longstanding code which is somehow relying on our previous behavior. However, this should be largely mitigated by the considerations above. Furthermore, overrides are usually type-theoretically sound, so using a more-derived method instead of a less-derived method should always be more permissive about arguments and more informative about results. Moreover, we know that there are projects which are relying on the previous behavior of the compiler, which at least in some cases used the more-derived method; those projects will see not taking this patch as a source-compatibility regression. But I can't pretend that there's no risk from the patch working as intended.
This has been tested with ordinary regression tests and by verifying that it fixes a source-compatibility regression with a particular project.