diff --git a/cpp-overload-test/src/stdcpp/forward_list.rs b/cpp-overload-test/src/stdcpp/forward_list.rs index da23970..f718d06 100644 --- a/cpp-overload-test/src/stdcpp/forward_list.rs +++ b/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` pointer. /// /// ### Limitations /// @@ -39,9 +42,77 @@ pub struct StdForwardList(*mut c_void, PhantomData); /// 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::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(*mut c_void, PhantomData); + +// 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::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::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::iterator"] -> *mut c_void as "std::forward_list::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::const_iterator", last as "std::forward_list::const_iterator"] -> *mut c_void as "std::forward_list*" { + return new std::forward_list(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) -> StdForwardList { let items: Vec = 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*"] -> *mut c_void as "const std::forward_list::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*"] -> *mut c_void as "std::forward_list::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*"] -> *mut c_void as "const std::forward_list::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*"] -> *mut c_void as "std::forward_list::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*", other as "std::forward_list*", 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*", other as "std::forward_list*", 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*", eq_predicate as "BinaryPredicate*"] -> c_size_t as "size_t" { + cpp!([slf as "std::forward_list*", 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()); diff --git a/cpp-overloads-in-rust.md b/cpp-overloads-in-rust.md index 20b449c..dd22d16 100644 --- a/cpp-overloads-in-rust.md +++ b/cpp-overloads-in-rust.md @@ -26,7 +26,7 @@ See the Limitations section below the table for more details. | *Non-Template Classes* | Constructors | Accessors | Modifiers | Transforms | | TODO | ？ | ？ | ？ | ？ | | *Template Classes* | Constructors | Accessors | Modifiers | Transforms | -| [forward_list] | ✅ | 🌗⚙️ | 🌗⚙️？[^1] | ✅ | +| [forward_list] | ✅ | 🌗⚙️[^1] | 🌗⚙️？[^2] | ✅ | | TODO | ？ | ？ | ？ | ？ | Constructors: Create an instance of the class, called as `Class::Class(...)`
@@ -36,7 +36,8 @@ Transforms: Rearrange the items in a list class, called as `instance->sort(...)` [forward_list]: https://github.com/rustfoundation/overloading-macros/blob/main/cpp-overload-test/src/stdcpp/forward_list.rs -[^1]: Mostly caused by the differences between Rust and C++ iterators, could be overcome with type wrappers. +[^1]: Rust and C++ iterators have different semantics, using adapter types is an ergonomic workaround. +[^2]: `insert_after` and `splice_after` still need to be analysed. ## Limitations @@ -97,8 +98,7 @@ These overloads have no equivalent in Rust: - assignment functions, assignment operators, and `swap`: there is no "type move method" in Rust (it uses `memcpy` for assignment/swaps) -- legacy iterator pointer-like methods for various positions - (`before_begin`/`begin`/`end`/`after_end`) +- legacy iterator pointer-like methods for various positions (`before_begin`/`begin`/`end`) - initializer list methods: these are also a common source of overload conflicts for C++ templated lists