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

Remove non-interior mutability from generic types #587

Merged
merged 1 commit into from
Sep 4, 2024
Merged

Conversation

madsmtm
Copy link
Owner

@madsmtm madsmtm commented Mar 5, 2024

Part of #563.

Remove non-interior mutability from NSEnumerator, NSArray, NSDictionary, NSSet and the mutable variants.

I've run the new mut_while_iter fuzz target for using AFL++ ~8 hours with 8 parallel processes, and using libFuzzer for ~7 hours with 6 parallel processes, to test whether Foundation's mutation detection is precise enough to catch all errors, such that we can declare it sound.

A concrete place where this will be useful is Winit, which stores a RefCell<Retained<NSMutableAttributedString>> inside of a subclass of NSView to track marked text, but the RefCell here should be completely unnecessary.

@madsmtm madsmtm added enhancement New feature or request A-framework Affects the framework crates and the translator for them labels Mar 5, 2024
@madsmtm madsmtm changed the base branch from master to rework-cargo-features March 5, 2024 21:18
@madsmtm madsmtm force-pushed the non-mut branch 2 times, most recently from b6563b7 to a0454fb Compare March 7, 2024 16:08
@madsmtm madsmtm force-pushed the rework-cargo-features branch 5 times, most recently from 3d26587 to 0ee1323 Compare March 14, 2024 13:25
Base automatically changed from rework-cargo-features to master March 14, 2024 13:34
@madsmtm madsmtm added this to the objc2 v0.6 milestone Jun 2, 2024
@madsmtm madsmtm changed the title Remove non-interior mutability in Foundation types Remove non-interior mutability from generic types Jun 20, 2024
@madsmtm madsmtm force-pushed the non-mut branch 6 times, most recently from 973b04b to 032b698 Compare September 4, 2024 15:08
Comment on lines -92 to 29
T: Message,
{
// SAFETY: The collection had mutable ownership over the object, and since
// the object will never again be accessed from the collection, we can
// convert it to `Retained<T>`.
pub(crate) fn retain<T: Message>(obj: &T) -> Retained<T> {
// SAFETY: TODO
unsafe { Retained::retain(obj as *const T as *mut T).unwrap_unchecked() }
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently unsound, but won't be after we're done removing mutability.

Affects NSEnumerator, NSArray, NSDictionary, NSSet and the mutable
variants.

This removes all `_mut` methods and `Mut` iterators, as these are now
both unsound and unnecessary.

Most methods are also converted to `_retained` variants, as returning
references is unsound if the collection is mutated. `_unchecked`
variants are provided as alternatives instead.

At the same time, we remove a bunch of helper methods that are now
unnecessary, including `from_vec`.

Finally, we add a fuzz target for ensuring that the safe methods do
catch mutation mistakes.
@madsmtm madsmtm marked this pull request as ready for review September 4, 2024 15:15
@madsmtm madsmtm merged commit d198247 into master Sep 4, 2024
19 checks passed
@madsmtm madsmtm deleted the non-mut branch September 4, 2024 19:38
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