Skip to content

Fix a couple of silly problems with extensions [4.0] #10443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Jun 21, 2017

  • Description: A constrained extension of a typealias would result in a crash. Also, there was an old limitation that prevented us from being able to define a protocol extension with a typealias in the same module, probably as a workaround for some related problem, but it doesn't seem like this restriction is necessary anymore. Note that the crash I fixed occurs regardless of the module that the type is defined in, so maybe the restriction was bogus to begin with.

  • Origination: The crash was first reported against Swift 2 so this is an old problem. The related restriction has been there even longer.

  • Scope of the issue: Probably extensions of typealiases is not something people do intentionally very often, but happens when we rename a type and add a typealias for the old name.

  • Risk: Very low, we already desugar the type on this code path so we know this succeeds. Lifting the restriction might make some code that used to diagnose crash, but from my understanding of how extensions and name lookup works in today's Swift this doesn't seem likely. I strongly suspect it is vestigial, but @DougGregor can confirm.

  • Tested: New test case added, existing test for extension of protocol via typealias updated to reflect that it no longer produces a diagnostic.

  • Reviewed by: @DougGregor

  • Radar: rdar://problem/21607421

@slavapestov slavapestov force-pushed the fix-extension-sillyness-4.0 branch from 874e1d1 to 0fcd8fc Compare June 21, 2017 07:45
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We are still likely to have annoying order dependency issues here, but this is an improvement.

@tkremenek tkremenek merged commit 09bd4c3 into swiftlang:swift-4.0-branch Jun 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants