Skip to content

Comments

Fix requirements on values in NSSet and keys in NSDictionary#505

Merged
madsmtm merged 5 commits intomasterfrom
ensure-stable-hash
Sep 10, 2023
Merged

Fix requirements on values in NSSet and keys in NSDictionary#505
madsmtm merged 5 commits intomasterfrom
ensure-stable-hash

Conversation

@madsmtm
Copy link
Owner

@madsmtm madsmtm commented Sep 7, 2023 •
edited
Loading

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, or affecting soundness 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
@madsmtm madsmtm mentioned this pull request Sep 10, 2023
Closed
@madsmtm
Refactor CopyHelper and NS[Mutable]Copying setup
d5caed7 
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).
@madsmtm
Implement Eq and Ord for NSNumber
3661935
@madsmtm
Fix unsoundness in NSDictionary and NSSet creation/mutation
b4cb59e 
Almost at least, some generic types like `NSArray<NSView>` is still incorrectly allowed as a key
@madsmtm
Make a few more types immutable, and hence allowed in NSSet/NSDictionary
b5b79aa
@madsmtm
Add test for broken behaviour
7c9ff46
@madsmtm madsmtm merged commit ed7eb95 into master Sep 10, 2023
@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
@madsmtm
Remove HasStableHash requirement from NSSet and NSDictionary
72702d2 
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 madsmtm mentioned this pull request Jun 4, 2024
Merged
madsmtm added a commit that referenced this pull request Jun 4, 2024
@madsmtm
Remove HasStableHash requirement from NSSet and NSDictionary
eb13e02 
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
@madsmtm
Remove HasStableHash requirement from NSSet and NSDictionary
7e6d691 
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

Reviewers

No reviews

Assignees

No one assigned

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, or affecting soundness

Projects

None yet

Milestone

Usable icrate

Development

Successfully merging this pull request may close these issues.

Safety of storing mutable objects in collections? Can't have NSString and NSNumber as values in an NSDictionary

1 participant

@madsmtm