Merge pull request #443 from madsmtm/fix-enumeration

Fix enumeration

Author: madsmtm

Cargo.lock

@@ -12,6 +12,28 @@ data! {
         unsafe -removeAllObjects;
     }
 
+    // SAFETY: `NSEnumerator` and subclasses are safe as mutable because even
+    // though the items it contains are not mutable, the enumerator itself is
+    // (and it is important that the methods below are marked `&mut` as well).
+    //
+    // However, instances of this are only safe for others to create if
+    // they're ready to pass ownership to the enumerator, or if they somehow
+    // add a lifetime parameter (to prevent the original collection from
+    // being modified).
+    //
+    // So e.g. `Id<NSMutableArray<T>> -> Id<NSEnumerator<T>>` is safe, as is
+    // `&Id<NSArray<T: IsCloneable>> -> Id<NSEnumerator<T>>`, and so is
+    // `&'a NSArray<T: IsCloneable> -> Id<NSEnumerator<T>> + 'a`.
+    class NSEnumerator: Mutable {
+        // SAFETY: This removes the object from the internal collection, so it
+        // may safely return `Id<T>`.
+        unsafe -nextObject;
+        // SAFETY: The objects are removed from the internal collection and as
+        // such are safe to give ownership over.
+        unsafe -allObjects;
+    }
+    class NSDirectoryEnumerator: Mutable {}
+
     class NSString: ImmutableWithMutableSubclass<Foundation::NSMutableString> {
         unsafe -init;
         unsafe -compare;

crates/header-translator/src/data/Foundation.rs

@@ -574,6 +574,10 @@ skipped = true
 [struct.NSDecimal]
 skipped = true
 
+# Uses `c_ulong` which means we need to specify the encoding manually.
+[struct.NSFastEnumerationState]
+skipped = true
+
 # Uses stuff from core Darwin libraries which we have not yet mapped
 [class.NSAppleEventDescriptor.methods]
 descriptorWithDescriptorType_bytes_length = { skipped = true }
@@ -1551,6 +1555,8 @@ definition-skipped = true
 definition-skipped = true
 [class.NSMutableOrderedSet]
 definition-skipped = true
+[class.NSEnumerator]
+definition-skipped = true
 
 # These protocol impls would return the wrong types
 [class.NSSimpleCString]

crates/header-translator/translation-config.toml

@@ -31,6 +31,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * Added `MainThreadMarker::alloc` for allocating objects that need to be so on
   the main thread.
 * Added automatically generated `new`/`init` methods for all types.
+* Added `FromIterator` impls for various collection types.
 
 ### Changed
 * **BREAKING**: Renamed the `from_slice` method on `NSArray`, `NSSet`,
@@ -45,6 +46,46 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * **BREAKING**: Renamed `NSMutableCopying::mutable_copy` to `::mutableCopy`.
 * **BREAKING**: The default value for `NSUUID` was changed from a nil UUID to
   a new random UUID.
+* **BREAKING**: Changed how iteration works.
+
+  Instead of the single `NSFastEnumerator`, we now have concrete types
+  `array::Iter`, `array::IterMut`, `array::IterRetained` and
+  `array::IntoIter`, which allows iterating over `NSArray` in different ways.
+
+  Combined with proper `IntoIterator` implementations for collection types,
+  you can now do:
+  ```rust
+  let mut array: Id<NSMutableArray<T>> = ...;
+
+  for item in &array {
+      // item: &T
+  }
+
+  // If T: IsMutable
+  for item in &mut array {
+      // item: &mut T
+  }
+
+  // If T: IsIdCloneable
+  for item in array.iter_retained() {
+      // item: Id<T>
+  }
+
+  for item in array {
+      // item: Id<T>
+  }
+  ```
+
+  (similar functionality exist for `NSSet` and `NSDictionary`).
+* **BREAKING**: Renamed `NSDictionary` methods:
+  - `keys` -> `keys_vec`.
+  - `values` -> `values_vec`.
+  - `values_mut` -> `values_vec_mut`.
+  - `keys_and_objects` -> `to_vecs`.
+  - `iter_keys` -> `keys`.
+  - `iter_values` -> `values`.
+* **BREAKING**: `NSDictionary::keys_retained` and
+  `NSDictionary::values_retained` now return an iterator instead.
 
 ### Removed
 * **BREAKING**: Removed various redundant `NSProxy` methods.
@@ -56,6 +97,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * **BREAKING**: Removed a few `init` methods on subclasses that were declared
   on categories on their superclass. These should be re-added at some point.
 
+### Fixed
+* Soundness issues with enumeration / iteration over collection types.
+
 
 ## icrate 0.0.2 - 2023-02-07
 

crates/icrate/CHANGELOG.md

@@ -1,13 +1,15 @@
+//! Utilities for the `NSArray` and `NSMutableArray` classes.
 #![cfg(feature = "Foundation_NSArray")]
 use alloc::vec::Vec;
 use core::fmt;
 use core::mem;
 use core::ops::{Index, IndexMut, Range};
 use core::panic::{RefUnwindSafe, UnwindSafe};
 
-use objc2::msg_send;
 use objc2::mutability::{IsMutable, IsRetainable};
+use objc2::rc::IdFromIterator;
 
+use super::iter;
 use super::util;
 use crate::common::*;
 #[cfg(feature = "Foundation_NSMutableArray")]
@@ -220,15 +222,6 @@ extern_methods!(
 );
 
 impl<T: Message> NSArray<T> {
-    #[doc(alias = "objectEnumerator")]
-    #[cfg(feature = "Foundation_NSEnumerator")]
-    pub fn iter(&self) -> Foundation::NSEnumerator2<'_, T> {
-        unsafe {
-            let result: *mut Object = msg_send![self, objectEnumerator];
-            Foundation::NSEnumerator2::from_ptr(result)
-        }
-    }
-
     unsafe fn objects_in_range_unchecked(&self, range: Range<usize>) -> Vec<&T> {
         let range = Foundation::NSRange::from(range);
         let mut vec: Vec<NonNull<T>> = Vec::with_capacity(range.length);
@@ -340,33 +333,109 @@ impl<T: Message> NSMutableArray<T> {
     }
 }
 
-unsafe impl<T: Message> Foundation::NSFastEnumeration2 for NSArray<T> {
+impl<T: Message> NSArray<T> {
+    #[doc(alias = "objectEnumerator")]
+    #[inline]
+    pub fn iter(&self) -> Iter<'_, T> {
+        Iter(super::iter::Iter::new(self))
+    }
+
+    #[doc(alias = "objectEnumerator")]
+    #[inline]
+    pub fn iter_mut(&mut self) -> IterMut<'_, T>
+    where
+        T: IsMutable,
+    {
+        IterMut(super::iter::IterMut::new(self))
+    }
+
+    #[doc(alias = "objectEnumerator")]
+    #[inline]
+    pub fn iter_retained(&self) -> IterRetained<'_, T>
+    where
+        T: IsIdCloneable,
+    {
+        IterRetained(super::iter::IterRetained::new(self))
+    }
+}
+
+unsafe impl<T: Message> iter::FastEnumerationHelper for NSArray<T> {
     type Item = T;
+
+    #[inline]
+    fn maybe_len(&self) -> Option<usize> {
+        Some(self.len())
+    }
 }
 
 #[cfg(feature = "Foundation_NSMutableArray")]
-unsafe impl<T: Message> Foundation::NSFastEnumeration2 for NSMutableArray<T> {
+unsafe impl<T: Message> iter::FastEnumerationHelper for NSMutableArray<T> {
     type Item = T;
+
+    #[inline]
+    fn maybe_len(&self) -> Option<usize> {
+        Some(self.len())
+    }
 }
 
-impl<'a, T: Message> IntoIterator for &'a NSArray<T> {
-    type Item = &'a T;
-    type IntoIter = Foundation::NSFastEnumerator2<'a, NSArray<T>>;
+/// An iterator over the items of a `NSArray`.
+#[derive(Debug)]
+pub struct Iter<'a, T: Message>(iter::Iter<'a, NSArray<T>>);
 
-    fn into_iter(self) -> Self::IntoIter {
-        use Foundation::NSFastEnumeration2;
-        self.iter_fast()
-    }
+__impl_iter! {
+    impl<'a, T: Message> Iterator<Item = &'a T> for Iter<'a, T> { ... }
 }
 
-#[cfg(feature = "Foundation_NSMutableArray")]
-impl<'a, T: Message> IntoIterator for &'a NSMutableArray<T> {
-    type Item = &'a T;
-    type IntoIter = Foundation::NSFastEnumerator2<'a, NSMutableArray<T>>;
+/// A mutable iterator over the items of a `NSArray`.
+#[derive(Debug)]
+pub struct IterMut<'a, T: Message>(iter::IterMut<'a, NSArray<T>>);
 
-    fn into_iter(self) -> Self::IntoIter {
-        use Foundation::NSFastEnumeration2;
-        self.iter_fast()
+__impl_iter! {
+    impl<'a, T: IsMutable> Iterator<Item = &'a mut T> for IterMut<'a, T> { ... }
+}
+
+/// An iterator that retains the items of a `NSArray`.
+#[derive(Debug)]
+pub struct IterRetained<'a, T: Message>(iter::IterRetained<'a, NSArray<T>>);
+
+__impl_iter! {
+    impl<'a, T: IsIdCloneable> Iterator<Item = Id<T>> for IterRetained<'a, T> { ... }
+}
+
+/// A consuming iterator over the items of a `NSArray`.
+#[derive(Debug)]
+pub struct IntoIter<T: Message>(iter::IntoIter<NSArray<T>>);
+
+__impl_iter! {
+    impl<'a, T: Message> Iterator<Item = Id<T>> for IntoIter<T> { ... }
+}
+
+__impl_into_iter! {
+    impl<T: Message> IntoIterator for &NSArray<T> {
+        type IntoIter = Iter<'_, T>;
+    }
+
+    #[cfg(feature = "Foundation_NSMutableArray")]
+    impl<T: Message> IntoIterator for &NSMutableArray<T> {
+        type IntoIter = Iter<'_, T>;
+    }
+
+    impl<T: IsMutable> IntoIterator for &mut NSArray<T> {
+        type IntoIter = IterMut<'_, T>;
+    }
+
+    #[cfg(feature = "Foundation_NSMutableArray")]
+    impl<T: IsMutable> IntoIterator for &mut NSMutableArray<T> {
+        type IntoIter = IterMut<'_, T>;
+    }
+
+    impl<T: IsIdCloneable> IntoIterator for Id<NSArray<T>> {
+        type IntoIter = IntoIter<T>;
+    }
+
+    #[cfg(feature = "Foundation_NSMutableArray")]
+    impl<T: Message> IntoIterator for Id<NSMutableArray<T>> {
+        type IntoIter = IntoIter<T>;
     }
 }
 
@@ -403,8 +472,7 @@ impl<T: IsMutable> IndexMut<usize> for NSMutableArray<T> {
 impl<T: fmt::Debug + Message> fmt::Debug for NSArray<T> {
     #[inline]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        use Foundation::NSFastEnumeration2;
-        f.debug_list().entries(self.iter_fast()).finish()
+        f.debug_list().entries(self).finish()
     }
 }
 
@@ -424,3 +492,33 @@ impl<'a, T: IsRetainable> Extend<&'a T> for NSMutableArray<T> {
             .for_each(move |item| unsafe { self.addObject(item) })
     }
 }
+
+impl<'a, T: IsRetainable + 'a> IdFromIterator<&'a T> for NSArray<T> {
+    fn id_from_iter<I: IntoIterator<Item = &'a T>>(iter: I) -> Id<Self> {
+        let vec = Vec::from_iter(iter);
+        Self::from_slice(&vec)
+    }
+}
+
+impl<T: Message> IdFromIterator<Id<T>> for NSArray<T> {
+    fn id_from_iter<I: IntoIterator<Item = Id<T>>>(iter: I) -> Id<Self> {
+        let vec = Vec::from_iter(iter);
+        Self::from_vec(vec)
+    }
+}
+
+#[cfg(feature = "Foundation_NSMutableArray")]
+impl<'a, T: IsRetainable + 'a> IdFromIterator<&'a T> for NSMutableArray<T> {
+    fn id_from_iter<I: IntoIterator<Item = &'a T>>(iter: I) -> Id<Self> {
+        let vec = Vec::from_iter(iter);
+        Self::from_slice(&vec)
+    }
+}
+
+#[cfg(feature = "Foundation_NSMutableArray")]
+impl<T: Message> IdFromIterator<Id<T>> for NSMutableArray<T> {
+    fn id_from_iter<I: IntoIterator<Item = Id<T>>>(iter: I) -> Id<Self> {
+        let vec = Vec::from_iter(iter);
+        Self::from_vec(vec)
+    }
+}

crates/icrate/src/Foundation/additions/array.rs

@@ -8,6 +8,9 @@ use core::ops::{IndexMut, Range};
 use core::panic::{RefUnwindSafe, UnwindSafe};
 use core::slice::{self, SliceIndex};
 
+#[cfg(feature = "block")]
+use objc2::rc::IdFromIterator;
+
 use crate::common::*;
 #[cfg(feature = "Foundation_NSMutableData")]
 use crate::Foundation::NSMutableData;
@@ -246,18 +249,22 @@ impl std::io::Write for NSMutableData {
     }
 }
 
-// #[cfg(feature = "Foundation_NSMutableData")]
-// impl FromIterator<u8> for Id<NSMutableData> {
-//     fn from_iter<T: IntoIterator<Item = u8>>(iter: T) -> Self {
-//         let iter = iter.into_iter();
-//         let (lower, _) = iter.size_hint();
-//         let data = Self::with_capacity(lower);
-//         for item in iter {
-//             data.push(item);
-//         }
-//         data
-//     }
-// }
+#[cfg(feature = "block")]
+impl IdFromIterator<u8> for NSData {
+    fn id_from_iter<I: IntoIterator<Item = u8>>(iter: I) -> Id<Self> {
+        let vec = Vec::from_iter(iter);
+        Self::from_vec(vec)
+    }
+}
+
+#[cfg(feature = "Foundation_NSMutableData")]
+#[cfg(feature = "block")]
+impl IdFromIterator<u8> for NSMutableData {
+    fn id_from_iter<I: IntoIterator<Item = u8>>(iter: I) -> Id<Self> {
+        let vec = Vec::from_iter(iter);
+        Self::from_vec(vec)
+    }
+}
 
 #[cfg(feature = "block")]
 unsafe fn with_vec<T: Message>(obj: Option<Allocated<T>>, bytes: Vec<u8>) -> Id<T> {