Add C++ iterators to forward_list

cpp-overload-test/src/stdcpp/forward_list.rs

@@ -5,7 +5,7 @@
 //! [stdcpp-forward-list.rs]: https://github.com/rustfoundation/overloading-macros/blob/main/cpp-overload-test/examples/stdcpp-forward-list.rs
 //! [stdcpp-forward-list.stderr]: https://github.com/rustfoundation/overloading-macros/blob/main/cpp-overload-test/examples/stdcpp-forward-list.stderr
 
-#![allow(unused_braces)]
+#![allow(unused_braces, reason = "Generated by overload! macro")]
 
 use cpp::cpp;
 use splat_overload::overload;
@@ -24,10 +24,13 @@ cpp! {{
 
     // The `cpp!` macro can't handle C/C++ function type syntax, so we have to declare function
     // types here.
-    typedef bool BinaryPredicate(const int&, const int&);
+    typedef bool BinaryPredicateCpp(const int&, const int&);
 }}
 
-/// A wrapper struct to hold the returned C++ pointer.
+/// An ABI-compatible Rust function type for `BinaryPredicateCpp`.
+pub type BinaryPredicateRust = extern "C" fn(&T, &T) -> bool;
+
+/// A wrapper struct to hold the returned C++ `std::forward_list<int>` pointer.
 ///
 /// ### Limitations
 ///
@@ -39,9 +42,77 @@ pub struct StdForwardList(*mut c_void, PhantomData<T>);
 /// Workaround for missing generic support in the `overload!` macro.
 pub type T = std::ffi::c_int;
 
+/// A wrapper struct to hold C++ `std::forward_list<int>::iterator/const_iterator` objects.
+/// `ItemRef` is either `&'list T` or `&mut 'list T`, depending on the iterator type.
+///
+/// This generic type allows mutable iterators to be passed in place of constant iterators.
+#[cfg_attr(not(test), expect(dead_code, reason = "Only used in tests"))]
+pub struct StdForwardListIter<ItemRef>(*mut c_void, PhantomData<ItemRef>);
+
+// These overloads do not need the `overload!` macro.
+// TODO: manually rewrite the other `&`/`&mut` overloads in this way, or make the macro do it.
+#[cfg_attr(not(test), expect(dead_code, reason = "Only used in tests"))]
+impl StdForwardListIter<&T> {
+    /// Access the element at the current iterator position as a constant reference.
+    ///
+    /// ### Safety
+    ///
+    /// Accessing `end` iterators, or any invalid iterator, is undefined behaviour.
+    /// FIXME: check for end iterators and other invalid iterators, or mark this method as unsafe.
+    fn access(&self) -> Option<&T> {
+        let iter = self.0 as *const c_void;
+        unsafe {
+            cpp!([iter as "std::forward_list<int>::const_iterator"] -> Option<&T> as "const int*" {
+                return &*iter;
+            })
+        }
+    }
+}
+
+#[cfg_attr(not(test), expect(dead_code, reason = "Only used in tests"))]
+impl StdForwardListIter<&mut T> {
+    /// Access the element at the current iterator position as a mutable reference.
+    ///
+    /// ### Safety
+    ///
+    /// Accessing `end` iterators, or any invalid iterator, is undefined behaviour.
+    /// FIXME: check for end iterators and other invalid iterators, or mark this method as unsafe.
+    fn access(&mut self) -> Option<&mut T> {
+        let iter: *mut c_void = self.0;
+        unsafe {
+            cpp!([iter as "std::forward_list<int>::iterator"] -> Option<&mut T> as "int*" {
+                return &*iter;
+            })
+        }
+    }
+}
+
+#[cfg_attr(not(test), expect(dead_code, reason = "Only used in tests"))]
+impl<'list> StdForwardListIter<&'list mut T> {
+    /// Downgrade this mutable iterator to a constant iterator.
+    ///
+    /// ### Safety
+    ///
+    /// This is safe because we already hold a mutable iterator, which was derived from a Rust
+    /// exclusive reference to the list.
+    fn downgrade(self) -> StdForwardListIter<&'list T> {
+        let iter: *mut c_void = self.0;
+        let iter = unsafe {
+            cpp!([iter as "std::forward_list<int>::iterator"] -> *mut c_void as "std::forward_list<int>::const_iterator" {
+                return iter;
+            })
+        };
+        StdForwardListIter(iter, PhantomData)
+    }
+}
+
 // The cpp! macro doesn't work inside the overload! macro, so we extract C++ calls into separate methods.
 // A mature overload feature (or a production C++ project) wouldn't need this impl block.
 #[cfg_attr(not(test), expect(dead_code, reason = "Only used in tests"))]
+#[allow(
+    non_upper_case_globals,
+    reason = "Triggered by cpp! macro in rust-analyzer"
+)]
 impl StdForwardList {
     // Constructors: internal C++ implementations for `new(...)` overloads
     fn default() -> StdForwardList {
@@ -80,7 +151,6 @@ impl StdForwardList {
     ///
     /// The slice satisfies C++ `ContiguousIterator`, even though it's not a requirement for this
     /// C++ iterator overload.
-    /// FIXME: allow non-contiguous iterators, if that makes sense in Rust.
     fn from_slice(items: &[T]) -> StdForwardList {
         let len = items.len();
         let items: *const T = items.as_ptr();
@@ -93,6 +163,21 @@ impl StdForwardList {
         StdForwardList(list, PhantomData)
     }
 
+    /// Construct a list from a C++ iterator range `[first, last)`, excluding the element at `last`.
+    fn from_cpp_iter(
+        first: &StdForwardListIter<&T>,
+        last: &StdForwardListIter<&T>,
+    ) -> StdForwardList {
+        let first = first.0 as *const c_void;
+        let last = last.0 as *const c_void;
+        let list = unsafe {
+            cpp!([first as "std::forward_list<int>::const_iterator", last as "std::forward_list<int>::const_iterator"] -> *mut c_void as "std::forward_list<int>*" {
+                return new std::forward_list<int>(first, last);
+            })
+        };
+        StdForwardList(list, PhantomData)
+    }
+
     /// ### Limitations
     ///
     /// `std::from_range` is used as a tag to disambiguate this constructor, it is part of the C++
@@ -101,6 +186,10 @@ impl StdForwardList {
     ///
     /// This implementation builds a Rust `Vec` and C++ `std::vector` from the iterator for simplicity.
     /// A production implementation could use a Rust-to-C++ iterator-to-range adapter.
+    ///
+    /// ### Workarounds
+    ///
+    /// This could be done using C++ iterators, see `from_cpp_iter` above.
     fn from_iter(iter: &mut dyn Iterator<Item = T>) -> StdForwardList {
         let items: Vec<T> = iter.collect();
         let len = items.len();
@@ -173,7 +262,7 @@ impl StdForwardList {
         StdForwardList(list, PhantomData)
     }
 
-    // Accessors: internal C++ implementations for `front(...)` overloads
+    // Accessors: internal C++ implementations for `front(...)`, `begin(...)`, `end(...)` overloads
     fn front_const(&self) -> Option<&T>
     where
         T: Sized,
@@ -208,6 +297,63 @@ impl StdForwardList {
         }
     }
 
+    /// Returns a constant iterator to the first element of the list.
+    fn begin_const(&self) -> StdForwardListIter<&T> {
+        let slf = self.0 as *const c_void;
+        let iter = unsafe {
+            // We deliberately cast away the const here, to account for C++ interior mutability.
+            // FIXME: is the returned type always a pointer type?
+            cpp!([slf as "const std::forward_list<int>*"] -> *mut c_void as "const std::forward_list<int>::const_iterator" {
+                // Or cbegin()
+                return slf->begin();
+            })
+        };
+        StdForwardListIter(iter, PhantomData)
+    }
+
+    /// Returns a mutable iterator to the first element of the list.
+    fn begin_mut(&mut self) -> StdForwardListIter<&mut T> {
+        let slf: *mut c_void = self.0;
+        let iter = unsafe {
+            // FIXME: is the returned type always a pointer type?
+            cpp!([slf as "std::forward_list<int>*"] -> *mut c_void as "std::forward_list<int>::iterator" {
+                return slf->begin();
+            })
+        };
+        StdForwardListIter(iter, PhantomData)
+    }
+
+    /// Returns a constant iterator to a placeholder "element" *after* the end of the list.
+    /// Accessing this placeholder element is undefined behaviour.
+    fn end_const(&self) -> StdForwardListIter<&T> {
+        let slf = self.0 as *const c_void;
+        let iter = unsafe {
+            // We deliberately cast away the const here, to account for C++ interior mutability.
+            cpp!([slf as "const std::forward_list<int>*"] -> *mut c_void as "const std::forward_list<int>::const_iterator" {
+                // Or cend()
+                return slf->end();
+            })
+        };
+        StdForwardListIter(iter, PhantomData)
+    }
+
+    /// Returns a mutable iterator to a placeholder "element" *after* the end of the list.
+    /// Accessing this placeholder element is undefined behaviour.
+    ///
+    /// ### Limitations
+    ///
+    /// Having mutable access to a placeholder element makes no sense in Rust, and the ergonomics
+    /// of holding two mutable references into the same list are poor.
+    fn end_mut(&mut self) -> StdForwardListIter<&mut T> {
+        let slf: *mut c_void = self.0;
+        let iter = unsafe {
+            cpp!([slf as "std::forward_list<int>*"] -> *mut c_void as "std::forward_list<int>::iterator" {
+                return slf->end();
+            })
+        };
+        StdForwardListIter(iter, PhantomData)
+    }
+
     // Modifiers: internal implementations for `merge(...)`
     fn merge_from(self: &mut StdForwardList, other: &mut StdForwardList) {
         let slf: *mut c_void = self.0;
@@ -222,12 +368,14 @@ impl StdForwardList {
     fn merge_with(
         self: &mut StdForwardList,
         other: &mut StdForwardList,
-        less_than_predicate: extern "C" fn(&T, &T) -> bool,
+        less_than_predicate: BinaryPredicateRust,
     ) {
         let slf: *mut c_void = self.0;
         let other: *mut c_void = other.0;
         unsafe {
-            cpp!([slf as "std::forward_list<int>*", other as "std::forward_list<int>*", less_than_predicate as "BinaryPredicate*"] {
+            // Function types are function pointers in Rust, but in C++ function types are objects,
+            // so we need to convert the Rust type to a C++ pointer to function type.
+            cpp!([slf as "std::forward_list<int>*", other as "std::forward_list<int>*", less_than_predicate as "BinaryPredicateCpp*"] {
                 slf->merge(*other, less_than_predicate);
             })
         }
@@ -261,12 +409,12 @@ impl StdForwardList {
         }
     }
 
-    fn unique_with(&mut self, eq_predicate: extern "C" fn(&T, &T) -> bool) -> c_size_t {
+    fn unique_with(&mut self, eq_predicate: BinaryPredicateRust) -> c_size_t {
         let slf: *mut c_void = self.0;
         unsafe {
             // The `cpp!` macro can't handle C/C++ function type syntax, so we have to declare the
             // type separately above.
-            cpp!([slf as "std::forward_list<int>*", eq_predicate as "BinaryPredicate*"] -> c_size_t as "size_t" {
+            cpp!([slf as "std::forward_list<int>*", eq_predicate as "BinaryPredicateCpp*"] -> c_size_t as "size_t" {
                 return slf->unique(eq_predicate);
             })
         }
@@ -313,6 +461,11 @@ overload! {
             StdForwardList::from_slice(items)
         }
 
+        /// Construct a list from a C++ iterator range `[first, last)`, excluding the element at `last`.
+        pub fn new(first: &StdForwardListIter<&T>, last: &StdForwardListIter<&T>) -> StdForwardList {
+            StdForwardList::from_cpp_iter(first, last)
+        }
+
         /// Construct a list from an iterator.
         ///
         /// ### Limitations
@@ -368,7 +521,7 @@ overload! {
     }
 }
 
-// Accessors: `front(...)`
+// Accessors: `front(...)`, `begin(...)`, `end(...)`
 overload! {
     impl StdForwardList {
         /// Get the first element of the list as a constant reference.
@@ -416,6 +569,44 @@ overload! {
     }
 }
 
+overload! {
+    impl StdForwardList {
+        /// Get a constant iterator to the first element of the list.
+        ///
+        /// ### Limitations
+        ///
+        /// Same as `front(...)`.
+        pub fn begin(this: &'static StdForwardList) -> StdForwardListIter<&'static T> {
+            StdForwardList::begin_const(this)
+        }
+
+        /// Get a mutable iterator to the first element of the list.
+        pub fn begin(this: &'static mut StdForwardList) -> StdForwardListIter<&'static mut T> {
+            StdForwardList::begin_mut(this)
+        }
+    }
+}
+
+overload! {
+    impl StdForwardList {
+        /// Get a constant iterator to a placeholder "element" *after* the end of the list.
+        /// Accessing this placeholder element is undefined behaviour.
+        ///
+        /// ### Limitations
+        ///
+        /// Same as `front(...)`, with an added risk of undefined behaviour.
+        pub fn end(this: &'static StdForwardList) -> StdForwardListIter<&'static T> {
+            StdForwardList::end_const(this)
+        }
+
+        /// Get a mutable iterator to a placeholder "element" *after* the end of the list.
+        /// Accessing this placeholder element is undefined behaviour.
+        pub fn end(this: &'static mut StdForwardList) -> StdForwardListIter<&'static mut T> {
+            StdForwardList::end_mut(this)
+        }
+    }
+}
+
 // Modifiers: `merge(...)`
 overload! {
     impl StdForwardList {
@@ -427,7 +618,7 @@ overload! {
 
         /// Move elements from `other` into this list, preserving sort order with the given
         /// predicate. Both lists must be sorted according to that predicate.
-        pub fn merge(&mut self, other: &mut StdForwardList, less_than_predicate: extern "C" fn(&T, &T) -> bool) {
+        pub fn merge(&mut self, other: &mut StdForwardList, less_than_predicate: BinaryPredicateRust) {
             self.merge_with(other, less_than_predicate)
         }
     }
@@ -460,12 +651,13 @@ overload! {
         /// Remove all consecutive duplicate elements from the list, using the given predicate to
         /// compare elements as equal.
         /// Returns the number of elements removed.
-        pub fn unique(&mut self, eq_predicate: extern "C" fn(&T, &T) -> bool) -> c_size_t {
+        pub fn unique(&mut self, eq_predicate: BinaryPredicateRust) -> c_size_t {
             self.unique_with(eq_predicate)
         }
     }
 }
 
+/// These tests show that the correct Rust and C++ overloads are being called.
 #[test]
 pub fn test_forward_list() {
     // Constructors: `new(...)`
@@ -512,7 +704,7 @@ pub fn test_forward_list() {
     let from_initializer_list = StdForwardList::new(3, 2, 1);
     assert_eq!(from_initializer_list.front_const(), Some(&3));
 
-    // Accessors: `front(...)`
+    // Accessors: `front(...)`, `begin(...)`, `end(...)`
     //
     // ### Workaround
     //
@@ -530,6 +722,32 @@ pub fn test_forward_list() {
         Some(&mut 42)
     );
 
+    let begin_list = StdForwardList::new([1, 2, 3].as_slice());
+    assert_eq!(
+        StdForwardList::begin(static_ref(begin_list)).access(),
+        Some(&1)
+    );
+    let begin_list = StdForwardList::new([1, 2, 3].as_slice());
+    assert_eq!(
+        StdForwardList::begin(static_mut(begin_list)).access(),
+        Some(&mut 1)
+    );
+    let begin_list = StdForwardList::new([1, 2, 3].as_slice());
+    assert_eq!(
+        StdForwardList::begin(static_mut(begin_list))
+            .downgrade()
+            .access(),
+        Some(&1)
+    );
+
+    let from_cpp_iter_list = StdForwardList::new([5, 6, 7, 8].as_slice());
+    let workaround_static_ref = static_ref(from_cpp_iter_list);
+    let from_cpp_iter = StdForwardList::new(
+        &StdForwardList::begin(workaround_static_ref),
+        &StdForwardList::end(workaround_static_ref),
+    );
+    assert_eq!(from_cpp_iter.front_const(), Some(&5));
+
     // Modifiers: `merge(...)`
     let mut merge_into_list = StdForwardList::new([1, 3, 5].as_slice());
     let mut merge_from_list = StdForwardList::new([0, 2, 4, 6].as_slice());