From b391c01f23b173de5a25710407b7f11b1eaf089a Mon Sep 17 00:00:00 2001 From: Andrew Hayzen Date: Thu, 22 Jun 2023 11:12:15 +0100 Subject: [PATCH] cxx-qt-gen: remove wrapper method for C++ -> Rust invokables This then avoids us needing to generate Rust methods with fully qualified types on the Rust side and removes a load of generation. Related to #404 --- .../cxx-qt-gen/src/generator/cpp/invokable.rs | 94 +++++++++++++--- .../src/generator/rust/invokable.rs | 106 ++++-------------- crates/cxx-qt-gen/test_outputs/invokables.cpp | 16 +-- crates/cxx-qt-gen/test_outputs/invokables.h | 10 ++ crates/cxx-qt-gen/test_outputs/invokables.rs | 93 +++------------ 5 files changed, 137 insertions(+), 182 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/invokable.rs b/crates/cxx-qt-gen/src/generator/cpp/invokable.rs index cc181a4aa..a07964e95 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/invokable.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/invokable.rs @@ -67,11 +67,11 @@ pub fn generate_cpp_invokables( .collect::>>()?; let body = format!( - "m_rustObj->{ident}({parameter_names})", + "{ident}({parameter_names})", ident = idents.wrapper.cpp, - parameter_names = vec!["*this"] - .into_iter() - .chain(parameters.iter().map(|parameter| parameter.ident.as_str())) + parameter_names = parameters + .iter() + .map(|parameter| parameter.ident.as_str()) .collect::>() .join(", "), ); @@ -140,6 +140,25 @@ pub fn generate_cpp_invokables( }, }, }); + + // Note that we are generating a header to match the extern "Rust" method + // in Rust for our invokable. + // + // CXX generates the source and we just need the matching header. + // + // TODO: this could be private ? + // + // TODO: will this always be noexcept ? If the return type is Result is this then removed? + generated.methods.push(CppFragment::Header(format!( + "{return_cxx_ty} {ident}({parameter_types}){is_const} noexcept;", + return_cxx_ty = if let Some(return_cxx_ty) = &return_cxx_ty { + return_cxx_ty.as_cxx_ty() + } else { + "void" + }, + ident = idents.wrapper.cpp, + parameter_types = parameter_types, + ))); } Ok(generated) @@ -219,7 +238,7 @@ mod tests { .unwrap(); // methods - assert_eq!(generated.methods.len(), 4); + assert_eq!(generated.methods.len(), 8); let (header, source) = if let CppFragment::Pair { header, source } = &generated.methods[0] { (header, source) @@ -234,12 +253,19 @@ mod tests { MyObject::voidInvokable() const { // ::std::lock_guard - m_rustObj->voidInvokableWrapper(*this); + voidInvokableWrapper(); } "#} ); - let (header, source) = if let CppFragment::Pair { header, source } = &generated.methods[1] { + let header = if let CppFragment::Header(header) = &generated.methods[1] { + header + } else { + panic!("Expected header") + }; + assert_str_eq!(header, "void voidInvokableWrapper() const noexcept;"); + + let (header, source) = if let CppFragment::Pair { header, source } = &generated.methods[2] { (header, source) } else { panic!("Expected pair") @@ -255,12 +281,22 @@ mod tests { MyObject::trivialInvokable(::std::int32_t param) const { // ::std::lock_guard - return m_rustObj->trivialInvokableWrapper(*this, param); + return trivialInvokableWrapper(param); } "#} ); - let (header, source) = if let CppFragment::Pair { header, source } = &generated.methods[2] { + let header = if let CppFragment::Header(header) = &generated.methods[3] { + header + } else { + panic!("Expected header") + }; + assert_str_eq!( + header, + "::std::int32_t trivialInvokableWrapper(::std::int32_t param) const noexcept;" + ); + + let (header, source) = if let CppFragment::Pair { header, source } = &generated.methods[4] { (header, source) } else { panic!("Expected pair") @@ -276,12 +312,22 @@ mod tests { MyObject::opaqueInvokable(QColor const& param) { // ::std::lock_guard - return m_rustObj->opaqueInvokableWrapper(*this, param); + return opaqueInvokableWrapper(param); } "#} ); - let (header, source) = if let CppFragment::Pair { header, source } = &generated.methods[3] { + let header = if let CppFragment::Header(header) = &generated.methods[5] { + header + } else { + panic!("Expected header") + }; + assert_str_eq!( + header, + "::std::unique_ptr opaqueInvokableWrapper(QColor const& param) noexcept;" + ); + + let (header, source) = if let CppFragment::Pair { header, source } = &generated.methods[6] { (header, source) } else { panic!("Expected pair") @@ -297,10 +343,20 @@ mod tests { MyObject::specifiersInvokable(::std::int32_t param) const { // ::std::lock_guard - return m_rustObj->specifiersInvokableWrapper(*this, param); + return specifiersInvokableWrapper(param); } "#} ); + + let header = if let CppFragment::Header(header) = &generated.methods[7] { + header + } else { + panic!("Expected header") + }; + assert_str_eq!( + header, + "::std::int32_t specifiersInvokableWrapper(::std::int32_t param) const noexcept;" + ); } #[test] @@ -335,7 +391,7 @@ mod tests { .unwrap(); // methods - assert_eq!(generated.methods.len(), 1); + assert_eq!(generated.methods.len(), 2); let (header, source) = if let CppFragment::Pair { header, source } = &generated.methods[0] { (header, source) @@ -350,9 +406,19 @@ mod tests { MyObject::trivialInvokable(A1 param) const { // ::std::lock_guard - return m_rustObj->trivialInvokableWrapper(*this, param); + return trivialInvokableWrapper(param); } "#} ); + + let header = if let CppFragment::Header(header) = &generated.methods[1] { + header + } else { + panic!("Expected header") + }; + assert_str_eq!( + header, + "B2 trivialInvokableWrapper(A1 param) const noexcept;" + ); } } diff --git a/crates/cxx-qt-gen/src/generator/rust/invokable.rs b/crates/cxx-qt-gen/src/generator/rust/invokable.rs index ad53a7654..12bb3677e 100644 --- a/crates/cxx-qt-gen/src/generator/rust/invokable.rs +++ b/crates/cxx-qt-gen/src/generator/rust/invokable.rs @@ -12,7 +12,7 @@ use crate::{ }; use proc_macro2::TokenStream; use quote::quote; -use syn::{Ident, Result, ReturnType}; +use syn::Result; pub fn generate_rust_invokables( invokables: &Vec, @@ -20,26 +20,21 @@ pub fn generate_rust_invokables( ) -> Result { let mut generated = GeneratedRustQObjectBlocks::default(); let cpp_class_name_rust = &qobject_idents.cpp_class.rust; - let rust_struct_name_rust = &qobject_idents.rust_struct.rust; for invokable in invokables { let idents = QInvokableName::from(invokable); let wrapper_ident_cpp = idents.wrapper.cpp.to_string(); - let wrapper_ident_rust = &idents.wrapper.rust; let invokable_ident_rust = &idents.name.rust; + // TODO: once we aren't using qobject::T in the extern "RustQt" + // we can just pass through the original ExternFn block and add the attribute? let cpp_struct = if invokable.mutable { - quote! { Pin<&mut #cpp_class_name_rust> } + quote! { Pin<&mut #cpp_class_name_rust> } } else { quote! { &#cpp_class_name_rust } }; - let rust_struct = if invokable.mutable { - quote! { &mut #rust_struct_name_rust } - } else { - quote! { &#rust_struct_name_rust } - }; let parameter_signatures = if invokable.parameters.is_empty() { - quote! { self: #rust_struct, cpp: #cpp_struct } + quote! { self: #cpp_struct } } else { let parameters = invokable .parameters @@ -50,14 +45,9 @@ pub fn generate_rust_invokables( quote! { #ident: #ty } }) .collect::>(); - quote! { self: #rust_struct, cpp: #cpp_struct, #(#parameters),* } + quote! { self: #cpp_struct, #(#parameters),* } }; let return_type = &invokable.method.sig.output; - let has_return = if matches!(invokable.method.sig.output, ReturnType::Default) { - quote! {} - } else { - quote! { return } - }; let mut unsafe_block = None; let mut unsafe_call = Some(quote! { unsafe }); @@ -65,31 +55,19 @@ pub fn generate_rust_invokables( std::mem::swap(&mut unsafe_call, &mut unsafe_block); } - let parameter_names = invokable - .parameters - .iter() - .map(|parameter| parameter.ident.clone()) - .collect::>(); - let fragment = RustFragmentPair { cxx_bridge: vec![quote! { - // TODO: is an unsafe block valid? + // Note: extern "Rust" block does not need to be unsafe extern "Rust" { + // Note that we are exposing a Rust method on the C++ type to C++ + // + // CXX ends up generating the source, then we generate the matching header. + #[doc(hidden)] #[cxx_name = #wrapper_ident_cpp] - #unsafe_call fn #wrapper_ident_rust(#parameter_signatures) #return_type; + #unsafe_call fn #invokable_ident_rust(#parameter_signatures) #return_type; } }], - implementation: vec![ - // TODO: not all methods have a wrapper - quote! { - impl #rust_struct_name_rust { - #[doc(hidden)] - pub #unsafe_call fn #wrapper_ident_rust(#parameter_signatures) #return_type { - #has_return cpp.#invokable_ident_rust(#(#parameter_names),*); - } - } - }, - ], + implementation: vec![], }; generated @@ -164,26 +142,16 @@ mod tests { let generated = generate_rust_invokables(&invokables, &qobject_idents).unwrap(); assert_eq!(generated.cxx_mod_contents.len(), 4); - assert_eq!(generated.cxx_qt_mod_contents.len(), 4); + assert_eq!(generated.cxx_qt_mod_contents.len(), 0); // void_invokable assert_tokens_eq( &generated.cxx_mod_contents[0], quote! { extern "Rust" { - #[cxx_name = "voidInvokableWrapper"] - fn void_invokable_wrapper(self: &MyObject, cpp: &MyObjectQt); - } - }, - ); - assert_tokens_eq( - &generated.cxx_qt_mod_contents[0], - quote! { - impl MyObject { #[doc(hidden)] - pub fn void_invokable_wrapper(self: &MyObject, cpp: &MyObjectQt) { - cpp.void_invokable(); - } + #[cxx_name = "voidInvokableWrapper"] + fn void_invokable(self: &MyObjectQt); } }, ); @@ -193,19 +161,9 @@ mod tests { &generated.cxx_mod_contents[1], quote! { extern "Rust" { - #[cxx_name = "trivialInvokableWrapper"] - fn trivial_invokable_wrapper(self: &MyObject, cpp: &MyObjectQt, param: i32) -> i32; - } - }, - ); - assert_tokens_eq( - &generated.cxx_qt_mod_contents[1], - quote! { - impl MyObject { #[doc(hidden)] - pub fn trivial_invokable_wrapper(self: &MyObject, cpp: &MyObjectQt, param: i32) -> i32 { - return cpp.trivial_invokable(param); - } + #[cxx_name = "trivialInvokableWrapper"] + fn trivial_invokable(self: &MyObjectQt, param: i32) -> i32; } }, ); @@ -215,19 +173,9 @@ mod tests { &generated.cxx_mod_contents[2], quote! { extern "Rust" { - #[cxx_name = "opaqueInvokableWrapper"] - fn opaque_invokable_wrapper(self: &mut MyObject, cpp: Pin<&mut MyObjectQt>, param: &QColor) -> UniquePtr; - } - }, - ); - assert_tokens_eq( - &generated.cxx_qt_mod_contents[2], - quote! { - impl MyObject { #[doc(hidden)] - pub fn opaque_invokable_wrapper(self: &mut MyObject, cpp: Pin<&mut MyObjectQt>, param: &QColor) -> UniquePtr { - return cpp.opaque_invokable(param); - } + #[cxx_name = "opaqueInvokableWrapper"] + fn opaque_invokable(self: Pin<&mut MyObjectQt>, param: &QColor) -> UniquePtr; } }, ); @@ -237,19 +185,9 @@ mod tests { &generated.cxx_mod_contents[3], quote! { extern "Rust" { - #[cxx_name = "unsafeInvokableWrapper"] - unsafe fn unsafe_invokable_wrapper(self: &MyObject, cpp: &MyObjectQt, param: *mut T) -> *mut T; - } - }, - ); - assert_tokens_eq( - &generated.cxx_qt_mod_contents[3], - quote! { - impl MyObject { #[doc(hidden)] - pub unsafe fn unsafe_invokable_wrapper(self: &MyObject, cpp: &MyObjectQt, param: *mut T) -> *mut T { - return cpp.unsafe_invokable(param); - } + #[cxx_name = "unsafeInvokableWrapper"] + unsafe fn unsafe_invokable(self:&MyObjectQt, param: *mut T) -> *mut T; } }, ); diff --git a/crates/cxx-qt-gen/test_outputs/invokables.cpp b/crates/cxx-qt-gen/test_outputs/invokables.cpp index 9dcc60ac6..d87fe6399 100644 --- a/crates/cxx-qt-gen/test_outputs/invokables.cpp +++ b/crates/cxx-qt-gen/test_outputs/invokables.cpp @@ -34,14 +34,14 @@ void MyObject::invokable() const { const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex); - m_rustObj->invokableWrapper(*this); + invokableWrapper(); } void MyObject::invokableMutable() { const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex); - m_rustObj->invokableMutableWrapper(*this); + invokableMutableWrapper(); } void @@ -50,42 +50,42 @@ MyObject::invokableParameters(QColor const& opaque, ::std::int32_t primitive) const { const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex); - m_rustObj->invokableParametersWrapper(*this, opaque, trivial, primitive); + invokableParametersWrapper(opaque, trivial, primitive); } ::std::unique_ptr MyObject::invokableReturnOpaque() { const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex); - return m_rustObj->invokableReturnOpaqueWrapper(*this); + return invokableReturnOpaqueWrapper(); } QPoint MyObject::invokableReturnTrivial() { const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex); - return m_rustObj->invokableReturnTrivialWrapper(*this); + return invokableReturnTrivialWrapper(); } void MyObject::invokableFinal() const { const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex); - m_rustObj->invokableFinalWrapper(*this); + invokableFinalWrapper(); } void MyObject::invokableOverride() const { const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex); - m_rustObj->invokableOverrideWrapper(*this); + invokableOverrideWrapper(); } void MyObject::invokableVirtual() const { const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex); - m_rustObj->invokableVirtualWrapper(*this); + invokableVirtualWrapper(); } static_assert(alignof(MyObjectCxxQtThread) <= alignof(::std::size_t), diff --git a/crates/cxx-qt-gen/test_outputs/invokables.h b/crates/cxx-qt-gen/test_outputs/invokables.h index 4d5335fa3..60f1116a1 100644 --- a/crates/cxx-qt-gen/test_outputs/invokables.h +++ b/crates/cxx-qt-gen/test_outputs/invokables.h @@ -28,15 +28,25 @@ class MyObject : public QObject public: Q_INVOKABLE void invokable() const; + void invokableWrapper() const noexcept; Q_INVOKABLE void invokableMutable(); + void invokableMutableWrapper() noexcept; Q_INVOKABLE void invokableParameters(QColor const& opaque, QPoint const& trivial, ::std::int32_t primitive) const; + void invokableParametersWrapper(QColor const& opaque, + QPoint const& trivial, + ::std::int32_t primitive) const noexcept; Q_INVOKABLE ::std::unique_ptr invokableReturnOpaque(); + ::std::unique_ptr invokableReturnOpaqueWrapper() noexcept; Q_INVOKABLE QPoint invokableReturnTrivial(); + QPoint invokableReturnTrivialWrapper() noexcept; Q_INVOKABLE void invokableFinal() const final; + void invokableFinalWrapper() const noexcept; Q_INVOKABLE void invokableOverride() const override; + void invokableOverrideWrapper() const noexcept; Q_INVOKABLE virtual void invokableVirtual() const; + void invokableVirtualWrapper() const noexcept; MyObjectCxxQtThread qtThread() const; private: diff --git a/crates/cxx-qt-gen/test_outputs/invokables.rs b/crates/cxx-qt-gen/test_outputs/invokables.rs index dc716d7cf..e827f0bc4 100644 --- a/crates/cxx-qt-gen/test_outputs/invokables.rs +++ b/crates/cxx-qt-gen/test_outputs/invokables.rs @@ -38,48 +38,49 @@ mod ffi { type MyObject; } extern "Rust" { + #[doc(hidden)] #[cxx_name = "invokableWrapper"] - fn invokable_wrapper(self: &MyObject, cpp: &MyObjectQt); + fn invokable(self: &MyObjectQt); } extern "Rust" { + #[doc(hidden)] #[cxx_name = "invokableMutableWrapper"] - fn invokable_mutable_wrapper(self: &mut MyObject, cpp: Pin<&mut MyObjectQt>); + fn invokable_mutable(self: Pin<&mut MyObjectQt>); } extern "Rust" { + #[doc(hidden)] #[cxx_name = "invokableParametersWrapper"] - fn invokable_parameters_wrapper( - self: &MyObject, - cpp: &MyObjectQt, + fn invokable_parameters( + self: &MyObjectQt, opaque: &QColor, trivial: &QPoint, primitive: i32, ); } extern "Rust" { + #[doc(hidden)] #[cxx_name = "invokableReturnOpaqueWrapper"] - fn invokable_return_opaque_wrapper( - self: &mut MyObject, - cpp: Pin<&mut MyObjectQt>, - ) -> UniquePtr; + fn invokable_return_opaque(self: Pin<&mut MyObjectQt>) -> UniquePtr; } extern "Rust" { + #[doc(hidden)] #[cxx_name = "invokableReturnTrivialWrapper"] - fn invokable_return_trivial_wrapper( - self: &mut MyObject, - cpp: Pin<&mut MyObjectQt>, - ) -> QPoint; + fn invokable_return_trivial(self: Pin<&mut MyObjectQt>) -> QPoint; } extern "Rust" { + #[doc(hidden)] #[cxx_name = "invokableFinalWrapper"] - fn invokable_final_wrapper(self: &MyObject, cpp: &MyObjectQt); + fn invokable_final(self: &MyObjectQt); } extern "Rust" { + #[doc(hidden)] #[cxx_name = "invokableOverrideWrapper"] - fn invokable_override_wrapper(self: &MyObject, cpp: &MyObjectQt); + fn invokable_override(self: &MyObjectQt); } extern "Rust" { + #[doc(hidden)] #[cxx_name = "invokableVirtualWrapper"] - fn invokable_virtual_wrapper(self: &MyObject, cpp: &MyObjectQt); + fn invokable_virtual(self: &MyObjectQt); } unsafe extern "C++" { #[doc(hidden)] @@ -137,66 +138,6 @@ pub mod cxx_qt_ffi { type UniquePtr = cxx::UniquePtr; #[derive(Default)] pub struct MyObject; - impl MyObject { - #[doc(hidden)] - pub fn invokable_wrapper(self: &MyObject, cpp: &MyObjectQt) { - cpp.invokable(); - } - } - impl MyObject { - #[doc(hidden)] - pub fn invokable_mutable_wrapper(self: &mut MyObject, cpp: Pin<&mut MyObjectQt>) { - cpp.invokable_mutable(); - } - } - impl MyObject { - #[doc(hidden)] - pub fn invokable_parameters_wrapper( - self: &MyObject, - cpp: &MyObjectQt, - opaque: &QColor, - trivial: &QPoint, - primitive: i32, - ) { - cpp.invokable_parameters(opaque, trivial, primitive); - } - } - impl MyObject { - #[doc(hidden)] - pub fn invokable_return_opaque_wrapper( - self: &mut MyObject, - cpp: Pin<&mut MyObjectQt>, - ) -> UniquePtr { - return cpp.invokable_return_opaque(); - } - } - impl MyObject { - #[doc(hidden)] - pub fn invokable_return_trivial_wrapper( - self: &mut MyObject, - cpp: Pin<&mut MyObjectQt>, - ) -> QPoint { - return cpp.invokable_return_trivial(); - } - } - impl MyObject { - #[doc(hidden)] - pub fn invokable_final_wrapper(self: &MyObject, cpp: &MyObjectQt) { - cpp.invokable_final(); - } - } - impl MyObject { - #[doc(hidden)] - pub fn invokable_override_wrapper(self: &MyObject, cpp: &MyObjectQt) { - cpp.invokable_override(); - } - } - impl MyObject { - #[doc(hidden)] - pub fn invokable_virtual_wrapper(self: &MyObject, cpp: &MyObjectQt) { - cpp.invokable_virtual(); - } - } impl cxx_qt::Threading for MyObjectQt { type BoxedQueuedFn = MyObjectCxxQtThreadQueuedFn; type ThreadingTypeId = cxx::type_id!("cxx_qt::my_object::MyObjectCxxQtThread");