Skip to content

Sema: Go back to synthesizing hashValue on _StoredBridgedNSError conformers #25349

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 11, 2019

Commit e0bba70 added a default implementation, however this is wrong
for non-imported types.

Instead, synthesize the body as before. Since this is one of the few
derived methods that can appear on an imported type, make sure to
build fully type-checked AST.

Fixes rdar://problem/51322302.

@slavapestov slavapestov requested a review from lorentey June 11, 2019 09:45
…ormers

Commit e0bba70 added a default implementation, however this is wrong
for non-imported types.

Instead, synthesize the body as before. Since this is one of the few
derived methods that can appear on an imported type, make sure to
build fully type-checked AST.

Fixes <rdar://problem/51322302>.
@slavapestov slavapestov force-pushed the synthesized-hashValue-abi-break branch from 3f3bcf0 to 712927c Compare June 11, 2019 09:46
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@jrose-apple
Copy link
Contributor

You're not supposed to use that protocol with non-imported types.

@lorentey
Copy link
Member

We have CocoaError, URLError, POSIXError and MachError conforming to it in Foundation.

@jrose-apple
Copy link
Contributor

Okay, but that's because those are emulating imported types, and I wouldn't expect their hash(into:) or == to be out of sync anyway. Can we just add manual implementations to those?

@lorentey
Copy link
Member

Yep, we can do that. I'm perfectly okay with either solution.

@slavapestov slavapestov merged commit 6ee5774 into swiftlang:master Jun 11, 2019
@jrose-apple
Copy link
Contributor

Please revert this, then, @slavapestov.

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