Skip to content

Sema: Warn about non-final classes conforming to protocols with a same-type requirement on 'Self' #41545

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

Conversation

slavapestov
Copy link
Contributor

This is unsound because it breaks covariance of protocol conformances
on classes. See the test case and changelog entry for details.

…e-type requirement on 'Self'

This is unsound because it breaks covariance of protocol conformances
on classes. See the test case and changelog entry for details.
@slavapestov slavapestov force-pushed the non-final-class-self-same-type-requirement-conformance branch from e02ad86 to c98ddb9 Compare February 24, 2022 19:23
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@hamishknight
Copy link
Contributor

hamishknight commented Feb 24, 2022

Nice! I guess this also catches SR-10713?

Edit: Ah looks like that's more or less test/Generics/rdar80503090.swift, so yes

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

1 similar comment
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test Linux

1 similar comment
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test Linux

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test macOS

@slavapestov
Copy link
Contributor Author

@hamishknight Ah indeed, looks like you discovered the issue 3 years ago :)

@slavapestov
Copy link
Contributor Author

@hamishknight Also I found another related radar I filed last year: rdar://74944514. My commit here doesn't actually catch that case, but it will after a small change. I'll generalize it in a follow-up PR.

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.

2 participants