[4.2][NameLookup] Swift 4 compatibility hack for cross-module var overloads in generic type extensions #19223
Conversation
…s in generic type extensions In Swift 4, we only gave custom overload types to properties defined in extensions of generic types, using the null type for any other var decl. This meant that a property defined in such an extension would never shadow a property not defined in such an extension. As a result, this permitted cross-module overloads of properties of different types on generic types in certain cases. This patch adds an exception to the shadowing rules for properties defined in generic type extensions under Swift 4 mode. Permitting cross-module property overloads in the general case would also be source breaking (causing ambiguity errors), so this can be handled in a follow-up Swift 5 mode PR if desired. Resolves SR-7341.
|
@slavapestov @DougGregor Could you please run CI tests? (assuming this is still something that's viable for 4.2.1) |
|
@swift-ci Please test |
|
Any news on this? I don't know when the cut off point for getting stuff into 4.2.1 is, so I don't know how urgent it is. |
|
Ah, according to @jrose-apple, 4.2.1 is trying to be kept minimal. Presumably that rules this out (and the others: #19288 & #19562)? I'll go ahead and close if so. |
|
Sorry I completely missed this. The release manager (me) is the one that needs to approve merging these. From the 4.2 release process:
The problem is that I didn't see these as my review wasn't requested nor was I CC'ed. It is true that the kind of change to take for 4.2.1 is very small; I'll take a look at these to see if they qualify, but I suspect they won't. In the future, we'll look at using the CODEOWNERS feature that GitHub provides to see if we can get the release manager notified of these PRs (and have their review required) so that the tooling is supporting the process. |
|
@tkremenek Apologies for not cc'ing you. I saw other 4.2 PRs using |
|
No problem. I need to improve the processes in the tools to make sure the release manager sees these PRs in a more timely manner. I sincerely apologize for now seeing this sooner, and I appreciate you filing the PR. After consideration, I think we will not take this for 4.2.1. Swift 4.2.1 is a very focused release, and there are always a chance of regressions being introduced by changes like this. We should hold this fix to Swift 5. |
(4.2 cherry-pick of #18488 + expanded test-case for dynamic overload ranking from #18952)
Explanation: Across modules, a property declared in an extension of a generic type would shadow another property declared elsewhere, causing the one in the other module to be inaccessible.
Scope: Resolves a 4.2 source compatibility regression.
SR Issue: SR-7341.
Risk: Moderately low – the added logic is fairly narrow and matches existing compatibility logic in
swift::conflicting.Testing: Added tests for Swift 4 and 5 cross-module property overloading behaviour, Test suite, Compatibility suite.
Reviewers: @slavapestov, @DougGregor.