WIP: cxx-qt-gen: move #[qinvokable] to be inside extern "RustQt"

Closes KDAB#558

CHANGELOG.md

@@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Moved to `syn` 2.0 internally and for any exported `syn` types
 - `impl cxx_qt::Threading for qobject::T` now needs to be specified for `qt_thread()` to be available
 - `#[cxx_qt::qsignals]` and `#[cxx_qt::inherit]` are now used in an `extern "RustQt"` block as `#[qsignal]` and `#[inherit]`
+- `#[qinvokable]` is now defined as a signature in `extern "RustQt"`
 
 ### Removed
 

book/src/concepts/inheritance.md

@@ -15,6 +15,7 @@ To support creating such subclasses directly from within Rust, CXX-Qt provides y
 To access the methods of a base class in Rust, use the `#[inherit]` macro.
 It can be placed in front of a function in a `extern "RustQt"` block in a `#[cxx_qt::bridge]`.
 
+TODO: all of these examples need sorting out
 ```rust,ignore
 {{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_qalm}}
 

crates/cxx-qt-gen/src/generator/naming/invokable.rs

@@ -5,7 +5,7 @@
 use crate::{generator::naming::CombinedIdent, parser::invokable::ParsedQInvokable};
 use convert_case::{Case, Casing};
 use quote::format_ident;
-use syn::{Ident, ImplItemFn};
+use syn::{ForeignItemFn, Ident};
 
 /// Names for parts of a Q_INVOKABLE
 pub struct QInvokableName {
@@ -19,8 +19,8 @@ impl From<&ParsedQInvokable> for QInvokableName {
     }
 }
 
-impl From<&ImplItemFn> for QInvokableName {
-    fn from(method: &ImplItemFn) -> Self {
+impl From<&ForeignItemFn> for QInvokableName {
+    fn from(method: &ForeignItemFn) -> Self {
         let ident = &method.sig.ident;
         Self {
             name: CombinedIdent::from_rust_function(ident.clone()),
@@ -50,10 +50,8 @@ mod tests {
 
     #[test]
     fn test_from_impl_method() {
-        let item: ImplItemFn = parse_quote! {
-            fn my_invokable() {
-
-            }
+        let item: ForeignItemFn = parse_quote! {
+            fn my_invokable();
         };
         let parsed = ParsedQInvokable {
             method: item,

crates/cxx-qt-gen/src/generator/rust/invokable.rs

@@ -6,10 +6,7 @@
 use crate::{
     generator::{
         naming::{invokable::QInvokableName, qobject::QObjectName},
-        rust::{
-            fragment::RustFragmentPair, qobject::GeneratedRustQObjectBlocks,
-            types::is_unsafe_cxx_type,
-        },
+        rust::{fragment::RustFragmentPair, qobject::GeneratedRustQObjectBlocks},
     },
     parser::invokable::ParsedQInvokable,
 };
@@ -30,7 +27,6 @@ pub fn generate_rust_invokables(
         let wrapper_ident_cpp = idents.wrapper.cpp.to_string();
         let wrapper_ident_rust = &idents.wrapper.rust;
         let invokable_ident_rust = &idents.name.rust;
-        let original_method = &invokable.method;
 
         let cpp_struct = if invokable.mutable {
             quote! {  Pin<&mut #cpp_class_name_rust> }
@@ -62,21 +58,13 @@ pub fn generate_rust_invokables(
         } else {
             quote! { return }
         };
-        // Determine if unsafe is required due to an unsafe parameter or return type
-        let has_unsafe_param = invokable
-            .parameters
-            .iter()
-            .any(|parameter| is_unsafe_cxx_type(&parameter.ty));
-        let has_unsafe_return = if let ReturnType::Type(_, ty) = return_type {
-            is_unsafe_cxx_type(ty)
-        } else {
-            false
-        };
-        let has_unsafe = if has_unsafe_param || has_unsafe_return {
-            quote! { unsafe }
-        } else {
-            quote! {}
-        };
+
+        let mut unsafe_block = None;
+        let mut unsafe_call = Some(quote! { unsafe });
+        if invokable.safe {
+            std::mem::swap(&mut unsafe_call, &mut unsafe_block);
+        }
+
         let parameter_names = invokable
             .parameters
             .iter()
@@ -85,26 +73,22 @@ pub fn generate_rust_invokables(
 
         let fragment = RustFragmentPair {
             cxx_bridge: vec![quote! {
+                // TODO: is an unsafe block valid?
                 extern "Rust" {
                     #[cxx_name = #wrapper_ident_cpp]
-                    #has_unsafe fn #wrapper_ident_rust(#parameter_signatures) #return_type;
+                    #unsafe_call fn #wrapper_ident_rust(#parameter_signatures) #return_type;
                 }
             }],
             implementation: vec![
                 // TODO: not all methods have a wrapper
                 quote! {
                     impl #rust_struct_name_rust {
                         #[doc(hidden)]
-                        pub #has_unsafe fn #wrapper_ident_rust(#parameter_signatures) #return_type {
+                        pub #unsafe_call fn #wrapper_ident_rust(#parameter_signatures) #return_type {
                             #has_return cpp.#invokable_ident_rust(#(#parameter_names),*);
                         }
                     }
                 },
-                quote! {
-                    impl #cpp_class_name_rust {
-                        #original_method
-                    }
-                },
             ],
         };
 

crates/cxx-qt-gen/src/generator/rust/signals.rs

@@ -85,7 +85,7 @@ pub fn generate_rust_signals(
                     #[doc = "\n"]
                     #[doc = "Note that this method uses a AutoConnection connection type."]
                     #[must_use]
-                    fn #on_ident_rust(self: #self_type, func: fn(#self_type, #(#parameters),*)) -> CxxQtQMetaObjectConnection
+                    pub fn #on_ident_rust(self: #self_type, func: fn(#self_type, #(#parameters),*)) -> CxxQtQMetaObjectConnection
                     {
                         self.#connect_ident_rust(func, CxxQtConnectionType::AutoConnection)
                     }

crates/cxx-qt-gen/src/parser/cxxqtdata.rs

@@ -17,6 +17,8 @@ use syn::{
     TypePath,
 };
 
+use super::invokable::ParsedQInvokable;
+
 #[derive(Default)]
 pub struct ParsedCxxMappings {
     /// Map of the cxx_name of any types defined in CXX extern blocks
@@ -227,9 +229,17 @@ impl ParsedCxxQtData {
                     self.with_qobject(&parsed_inherited_method.qobject_ident)?
                         .inherited_methods
                         .push(parsed_inherited_method);
+                // Test if the function is an invokable
+                } else if let Some(index) = attribute_find_path(&foreign_fn.attrs, &["qinvokable"])
+                {
+                    let parsed_invokable_method =
+                        ParsedQInvokable::parse(foreign_fn, safe_call, index)?;
+                    self.with_qobject(&parsed_invokable_method.qobject_ident)?
+                        .invokables
+                        .push(parsed_invokable_method);
                 }
 
-                // TODO: test for qinvokable later
+                // TODO: non-signal/inherit/invokable functions should be exposed as C++ only methods
             }
         }
 
@@ -247,18 +257,16 @@ impl ParsedCxxQtData {
                 if let Some(qobject) = self.qobjects.get_mut(&path.segments[1].ident) {
                     if imp.trait_.is_some() {
                         qobject.parse_trait_impl(imp)?;
-                    } else {
-                        // Extract the ImplItem's from each Impl block
-                        qobject.parse_impl_items(&imp.items)?;
+                        return Ok(None);
                     }
+
+                    // non trait impls fall through
                 } else {
                     return Err(Error::new(
                         imp.span(),
                         "No matching QObject found for the given qobject::T impl block.",
                     ));
                 }
-
-                return Ok(None);
             // Find if we are an impl block for a qobject
             } else if let Some(qobject) = self.qobjects.get_mut(&path_to_single_ident(path)?) {
                 qobject.others.push(Item::Impl(imp));

crates/cxx-qt-gen/src/parser/invokable.rs

@@ -5,10 +5,10 @@
 
 use crate::{
     parser::parameter::ParsedFunctionParameter,
-    syntax::{attribute::*, implitemfn::is_method_mutable_pin_of_self},
+    syntax::{attribute::*, foreignmod, safety::Safety, types},
 };
 use std::collections::HashSet;
-use syn::{Ident, ImplItemFn, LitStr, Result};
+use syn::{spanned::Spanned, Error, ForeignItemFn, Ident, LitStr, Result};
 
 /// Describes a C++ specifier for the Q_INVOKABLE
 #[derive(Eq, Hash, PartialEq)]
@@ -21,26 +21,28 @@ pub enum ParsedQInvokableSpecifiers {
 /// Describes a single Q_INVOKABLE for a struct
 pub struct ParsedQInvokable {
     /// The original [syn::ImplItemFn] of the invokable
-    pub method: ImplItemFn,
+    pub method: ForeignItemFn,
+    /// The type of the self argument
+    pub qobject_ident: Ident,
     /// Whether this invokable is mutable
     pub mutable: bool,
+    /// Whether the method is safe to call.
+    pub safe: bool,
     /// The parameters of the invokable
     pub parameters: Vec<ParsedFunctionParameter>,
     /// Any specifiers that declared on the invokable
     pub specifiers: HashSet<ParsedQInvokableSpecifiers>,
 }
 
 impl ParsedQInvokable {
-    pub fn try_parse(method: &ImplItemFn) -> Result<Option<Self>> {
-        let index = attribute_find_path(&method.attrs, &["qinvokable"]);
-
-        if index.is_none() {
-            return Ok(None);
+    pub fn parse(mut method: ForeignItemFn, safety: Safety, index: usize) -> Result<Self> {
+        if safety == Safety::Unsafe && method.sig.unsafety.is_none() {
+            return Err(Error::new(
+                method.span(),
+                "Inherited methods must be marked as unsafe or wrapped in an `unsafe extern \"C++\"` block!",
+            ));
         }
-        Ok(Some(Self::parse(method, index.unwrap())?))
-    }
 
-    fn parse(method: &ImplItemFn, index: usize) -> Result<Self> {
         // Parse any C++ specifiers
         let mut specifiers = HashSet::new();
         let attrs_map = attribute_tokens_to_map::<Ident, LitStr>(
@@ -56,21 +58,24 @@ impl ParsedQInvokable {
         if attrs_map.contains_key(&quote::format_ident!("cxx_virtual")) {
             specifiers.insert(ParsedQInvokableSpecifiers::Virtual);
         }
+        method.attrs.remove(index);
 
         // Determine if the invokable is mutable
-        let mutable = is_method_mutable_pin_of_self(&method.sig);
+        let self_receiver = foreignmod::self_type_from_foreign_fn(&method.sig)?;
+        let (qobject_ident, mutability) = types::extract_qobject_ident(&self_receiver.ty)?;
+        let mutable = mutability.is_some();
 
-        // Read the signal inputs into parameter blocks
-        let parameters = ParsedFunctionParameter::parse_all_without_receiver(&method.sig)?;
+        let parameters = ParsedFunctionParameter::parse_all_ignoring_receiver(&method.sig)?;
+
+        let safe = method.sig.unsafety.is_none();
 
-        // Remove the invokable attribute
-        let mut method = method.clone();
-        method.attrs.remove(index);
         Ok(ParsedQInvokable {
             method,
+            qobject_ident,
             mutable,
             parameters,
             specifiers,
+            safe,
         })
     }
 }

crates/cxx-qt-gen/src/parser/qobject.rs

@@ -17,8 +17,8 @@ use crate::{
     syntax::path::path_compare_str,
 };
 use syn::{
-    spanned::Spanned, Error, Fields, Ident, ImplItem, ImplItemFn, Item, ItemImpl, ItemStruct,
-    LitStr, Result, Visibility,
+    spanned::Spanned, Error, Fields, Ident, ImplItem, Item, ItemImpl, ItemStruct, LitStr, Result,
+    Visibility,
 };
 
 /// Metadata for registering QML element
@@ -195,35 +195,6 @@ impl ParsedQObject {
         }
     }
 
-    fn parse_impl_method(&mut self, method: &ImplItemFn) -> Result<()> {
-        if let Some(invokable) = ParsedQInvokable::try_parse(method)? {
-            self.invokables.push(invokable);
-        } else {
-            self.passthrough_impl_items
-                .push(ImplItem::Fn(method.clone()));
-        }
-        Ok(())
-    }
-
-    /// Extract all methods (both invokable and non-invokable) from [syn::ImplItem]'s from each Impl block
-    ///
-    /// These will have come from a impl qobject::T block
-    pub fn parse_impl_items(&mut self, items: &[ImplItem]) -> Result<()> {
-        for item in items {
-            // Check if this item is a method
-            match item {
-                ImplItem::Fn(method) => {
-                    self.parse_impl_method(method)?;
-                }
-                _ => {
-                    self.passthrough_impl_items.push(item.clone());
-                }
-            }
-        }
-
-        Ok(())
-    }
-
     pub fn parse_trait_impl(&mut self, imp: ItemImpl) -> Result<()> {
         let (not, trait_path, _) = &imp
             .trait_

crates/cxx-qt-gen/src/syntax/mod.rs

@@ -7,7 +7,6 @@ pub mod attribute;
 pub mod expr;
 pub mod fields;
 pub mod foreignmod;
-pub mod implitemfn;
 pub mod path;
 mod qtfile;
 mod qtitem;