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

Don't require Eq + Hash for NSSet values / NSDictionary keys #641

Merged
merged 3 commits into from
Aug 30, 2024

Conversation

madsmtm
Copy link
Owner

@madsmtm madsmtm commented Jul 17, 2024

These are implied by Message, since (basically) all objects implement NSObjectProtocol - and if the isEqual:/hash methods aren't implemented, they'll simply throw an error or abort (so it's still sound).

Additionally, we remove the previous NSSet methods contains, is_subset, is_superset and is_disjoint, since they don't match the Objective-C naming scheme, and the underlying methods are now marked as safe.

Finally, note that this makes it possible to use AnyObject and NSValue in NSSet and as NSDictionary keys (this is likely already done in existing Objective-C code, so is likely the desired behaviour). Since the absence of the Eq implementation for NSValue is no longer valuable, this PR also implements Eq for NSValue.

TODO:

  • Add a test to check that we throw as expected.
  • Add changelog entry.

@madsmtm madsmtm added enhancement New feature or request A-framework Affects the framework crates and the translator for them labels Jul 17, 2024
@madsmtm madsmtm force-pushed the no-eq-hash branch 2 times, most recently from 3661389 to 8f0e325 Compare August 30, 2024 04:41
These are implied by `Message`, since (basically) all objects implement
`NSObjectProtocol` - and if the `isEqual:`/`hash` methods aren't
implemented, they'll simply throw an error (so it's still sound).
Additionally, we remove the previous aliases `contains`, `is_subset`,
`is_superset` and `is_disjoint`, since they don't match the Objective-C
naming scheme.
Note that `NSNumber` already implements `Eq` since 3661935, which was
the biggest blocker for adding this implementation.
@madsmtm madsmtm merged commit 2e45639 into master Aug 30, 2024
19 checks passed
@madsmtm madsmtm deleted the no-eq-hash branch August 30, 2024 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-framework Affects the framework crates and the translator for them enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant