Skip to content

Sema: Prefer class properties to protocol properties when ranking overloads #18206

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

Closed

Conversation

slavapestov
Copy link
Contributor

This was the old behavior in Swift 4.1, but it broke after some changes
to redeclaration checking and shadowing.

Fixes rdar://problem/42201583.

…rloads

This was the old behavior in Swift 4.1, but it broke after some changes
to redeclaration checking and shadowing.

Fixes <rdar://problem/42201583>.
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@hamishknight Please take a look.

// If both are properties and one is in a protocol, prefer the one not in
// a protocol.
//
// This is a Swift 4.1 compatibility hack. Changes elsewhere in the compiler
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we conditionalize it on language version?

Copy link
Contributor

Choose a reason for hiding this comment

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

Arguably this is in line with preferring concrete implementations over protocol implementations. It is a little funny when the concrete implementation is a superclass, but…

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 56d9c47

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 56d9c47

@hamishknight
Copy link
Contributor

hamishknight commented Jul 25, 2018

Thanks for addressing this! LGTM as a stopgap until we decide on a consistent rule to follow for all types of members (i.e not just properties) in this case.

@hamishknight
Copy link
Contributor

cc. @xedin who's assigned to SR-7425

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.

6 participants