Skip to content
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

Fix hashing issues for messages/groups in arrays. #755

Merged
merged 1 commit into from
Jun 14, 2018

Conversation

thomasvl
Copy link
Collaborator

The original code here landed in f997206,
but because of SR-7993 and not having yet landed a Package.swift that
enabled 4.2 support, we weren't hitting this code yet.

Since the visitor api uses Message, it doesn't know they messages are
Hashable, the maps work because they get more complete type info to
pick up the conformance declared on the Base used by for generated
messages.

So...

Going back to manually mixing in the for Arrays, adding a Todo
to revisit this. But this gets master back to working if someone
tries to pull the code into a compiling context that is 4.2+.

The original code here landed in f997206,
but because of SR-7993 and not having yet landed a Package.swift that
enabled 4.2 support, we weren't hitting this code yet.

Since the visitor api uses Message, it doesn't know they messages are
Hashable, the maps work because they get more complete type info to
pick up the conformance declared on the Base used by for generated
messages.

So...

Going back to manually mixing in the for Arrays, adding a Todo
to revisit this. But this gets master back to working if someone
tries to pull the code into a compiling context that is 4.2+.
@thomasvl thomasvl requested a review from allevato June 14, 2018 12:55
@thomasvl
Copy link
Collaborator Author

I plan on revisiting this with hash(into:) but this is just to fix what was landed in a broken state, but I didn't realized it because of the SwiftPM issue.

@thomasvl thomasvl merged commit 8e8fc10 into apple:master Jun 14, 2018
@thomasvl thomasvl deleted the fix_hash_changes branch June 14, 2018 14:32
ahmed-osama-saad pushed a commit to ahmed-osama-saad/swift-protobuf that referenced this pull request Oct 12, 2023
Co-authored-by: RutgerVromans <rutger.vromans@signify.com>
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