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 requirements on values in NSSet and keys in NSDictionary #505

Merged
merged 5 commits into from
Sep 10, 2023

Conversation

madsmtm
Copy link
Owner

@madsmtm madsmtm commented Sep 7, 2023

Introduce HasStableHash and use that to restrict the keys in NSSet and NSMutableDictionary, such that you cannot use e.g. NSView as a key (since the hash and isEqual: implementations of that may change if the view changes).

Addtionally, refactor and clean up the NSCopying/NSMutableCopying trait definitions.

Fixes #306 and fixes #337 - the remaining parts are moved to #507.

@madsmtm madsmtm added enhancement New feature or request A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates A-framework Affects the framework crates and the translator for them I-unsound A soundness hole labels Sep 7, 2023
@madsmtm madsmtm added this to the Usable icrate milestone Sep 7, 2023
@madsmtm madsmtm force-pushed the ensure-stable-hash branch 8 times, most recently from c518730 to 7508af1 Compare September 7, 2023 09:21
@madsmtm madsmtm marked this pull request as ready for review September 7, 2023 09:21
Add CounterpartOrSelf instead of CopyHelper, which lives in objc2 and is much nicer to use.

This also allows us to move NSCopying and NSMutableCopying back to reside in Foundation (note: We might have to at some point implement a small hack for these traits acting as if they contain the `copy` method, while it's actually `NSObject` that does).
Almost at least, some generic types like `NSArray<NSView>` is still incorrectly allowed as a key
@madsmtm madsmtm merged commit ed7eb95 into master Sep 10, 2023
19 checks passed
@madsmtm madsmtm deleted the ensure-stable-hash branch September 10, 2023 15:46
madsmtm added a commit that referenced this pull request Jun 4, 2024
This was added in an abundance of caution in #505 to fix #337, but
prevents real-world usage of these types, and isn't actually needed for
soundness (the documentation mentions the collection being "corrupt" if
the hash is changed, but that's not the same as it being UB).
madsmtm added a commit that referenced this pull request Jun 4, 2024
This was added in an abundance of caution in #505 to fix #337, but
prevents real-world usage of these types, and isn't actually needed for
soundness (the documentation mentions the collection being "corrupt" if
the hash is changed, but that's not the same as it being UB).
madsmtm added a commit that referenced this pull request Jun 4, 2024
This was added in an abundance of caution in #505 to fix #337, but
prevents real-world usage of these types, and isn't actually needed for
soundness (the documentation mentions the collection being "corrupt" if
the hash is changed, but that's not the same as it being UB).
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 A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates enhancement New feature or request I-unsound A soundness hole
Projects
None yet
1 participant