cxx-qt-gen: ensure that attributes are handled correctly for cxx_qt::…

…inherit Ensure that attributes on the extern block are empty, otherwise unsafe detection can fail. Ensure that attributes on the method as passed through, otherwise doc lines don't work. Related to KDAB#557
ahayzen-kdab · Jun 5, 2023 · 2dab1b8 · 2dab1b8
1 parent 58d2d58
commit 2dab1b8
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 33 deletions.
diff --git a/crates/cxx-qt-gen/src/generator/rust/inherit.rs b/crates/cxx-qt-gen/src/generator/rust/inherit.rs
@@ -32,7 +32,6 @@ pub fn generate(
                 .collect::<Vec<TokenStream>>();
             let ident = &method.method.sig.ident;
             let cxx_name_string = &method.wrapper_ident().to_string();
-            let ident_cpp_str = method.ident.cpp.to_string();
             let self_param = if method.mutable {
                 quote! { self: Pin<&mut #qobject_name> }
             } else {
@@ -45,11 +44,10 @@ pub fn generate(
             if method.safe {
                 std::mem::swap(&mut unsafe_call, &mut unsafe_block);
             }
+            let attrs = &method.method.attrs;
             syn::parse2(quote! {
                 #unsafe_block extern "C++" {
-                    #[doc = "CXX-Qt generated method which calls the C++ method"]
-                    #[doc = #ident_cpp_str]
-                    #[doc = "on the base class"]
+                    #(#attrs)*
                     #[cxx_name = #cxx_name_string]
                     #unsafe_call fn #ident(#self_param, #(#parameters),*) #return_type;
                 }
@@ -82,7 +80,7 @@ mod tests {
     fn test_mutable() {
         let generated = generate_from_foreign(
             parse_quote! {
-                    fn test(self: Pin<&mut qobject::MyObject>, a: B, b: C);
+                fn test(self: Pin<&mut qobject::MyObject>, a: B, b: C);
             },
             Safety::Safe,
         )
@@ -95,9 +93,6 @@ mod tests {
             &generated.cxx_mod_contents[0],
             quote! {
                 unsafe extern "C++" {
-                    #[doc = "CXX-Qt generated method which calls the C++ method"]
-                    #[doc = "test"]
-                    #[doc = "on the base class"]
                     #[cxx_name = "testCxxQtInherit"]
                     fn test(self: Pin<&mut MyObjectQt>, a: B, b: C);
                 }
@@ -122,9 +117,6 @@ mod tests {
             &generated.cxx_mod_contents[0],
             quote! {
                 unsafe extern "C++" {
-                    #[doc = "CXX-Qt generated method which calls the C++ method"]
-                    #[doc = "test"]
-                    #[doc = "on the base class"]
                     #[cxx_name = "testCxxQtInherit"]
                     fn test(self: &MyObjectQt, a: B, b: C);
                 }
@@ -150,9 +142,6 @@ mod tests {
             // TODO: Maybe remove the trailing comma after self?
             quote! {
                 extern "C++" {
-                    #[doc = "CXX-Qt generated method which calls the C++ method"]
-                    #[doc = "test"]
-                    #[doc = "on the base class"]
                     #[cxx_name = "testCxxQtInherit"]
                     unsafe fn test(self: &MyObjectQt,);
                 }

diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs
@@ -53,6 +53,17 @@ impl Parse for InheritMethods {
     fn parse(input: ParseStream) -> Result<Self> {
         let mut base_functions = Vec::new();
 
+        // Ensure that any attributes on the block have been removed
+        //
+        // Otherwise parsing of unsafe can fail due to #[doc]
+        let attrs = input.call(Attribute::parse_outer)?;
+        if !attrs.is_empty() {
+            return Err(Error::new(
+                attrs.first().span(),
+                "Unexpected attribute on #[cxx_qt::qsignals] block.",
+            ));
+        }
+
         // This looks somewhat counter-intuitive, but if we add `unsafe`
         // to the `extern "C++"` block, the contained functions will be safe to call.
         let safety = if input.peek(Token![unsafe]) {
@@ -110,7 +121,7 @@ pub struct ParsedInheritedMethod {
 }
 
 impl ParsedInheritedMethod {
-    pub fn parse(method: ForeignItemFn, safety: Safety) -> Result<Self> {
+    pub fn parse(mut method: ForeignItemFn, safety: Safety) -> Result<Self> {
         if safety == Safety::Unsafe && method.sig.unsafety.is_none() {
             return Err(Error::new(
                 method.span(),
@@ -125,19 +136,16 @@ impl ParsedInheritedMethod {
         let parameters = ParsedFunctionParameter::parse_all_ignoring_receiver(&method.sig)?;
 
         let mut ident = CombinedIdent::from_rust_function(method.sig.ident.clone());
-        for attribute in &method.attrs {
-            if !attribute.meta.path().is_ident(&format_ident!("cxx_name")) {
-                return Err(Error::new(
-                    attribute.span(),
-                    "Unsupported attribute in #[cxx_qt::inherit]",
-                ));
-            }
 
+        if let Some(index) = attribute_find_path(&method.attrs, &["cxx_name"]) {
             ident.cpp = format_ident!(
                 "{}",
-                expr_to_string(&attribute.meta.require_name_value()?.value)?
+                expr_to_string(&method.attrs[index].meta.require_name_value()?.value)?
             );
+
+            method.attrs.remove(index);
         }
+
         let safe = method.sig.unsafety.is_none();
 
         Ok(Self {
@@ -195,6 +203,19 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_parse_attributes() {
+        let module = quote! {
+            unsafe extern "C++" {
+                #[attribute]
+                fn test(self: &qobject::T);
+            }
+        };
+        let parsed: InheritMethods = syn::parse2(module).unwrap();
+        assert_eq!(parsed.base_functions.len(), 1);
+        assert_eq!(parsed.base_functions[0].attrs.len(), 1);
+    }
+
     fn assert_parse_error(function: ForeignItemFn) {
         let result = ParsedInheritedMethod::parse(function, Safety::Safe);
         assert!(result.is_err());
@@ -231,10 +252,6 @@ mod tests {
             fn test(self: &mut T);
         });
         // Attributes
-        assert_parse_error(parse_quote! {
-            #[myattribute]
-            fn test(self: &qobject::T);
-        });
         assert_parse_error(parse_quote! {
             fn test(#[test] self: &qobject::T);
         });

diff --git a/crates/cxx-qt-gen/test_inputs/inheritance.rs b/crates/cxx-qt-gen/test_inputs/inheritance.rs
@@ -15,12 +15,14 @@ mod inheritance {
 
     #[cxx_qt::inherit]
     unsafe extern "C++" {
+        /// Inherited hasChildren from the base class
         #[cxx_name = "hasChildren"]
         fn has_children_super(self: &qobject::MyObject, parent: &QModelIndex) -> bool;
     }
 
     #[cxx_qt::inherit]
     extern "C++" {
+        /// Inherited fetchMore from the base class
         unsafe fn fetch_more(self: Pin<&mut qobject::MyObject>, index: &QModelIndex);
     }
 

diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.rs b/crates/cxx-qt-gen/test_outputs/inheritance.rs
@@ -50,16 +50,12 @@ mod inheritance {
         fn has_children_wrapper(self: &MyObject, cpp: &MyObjectQt, _parent: &QModelIndex) -> bool;
     }
     unsafe extern "C++" {
-        #[doc = "CXX-Qt generated method which calls the C++ method"]
-        #[doc = "hasChildren"]
-        #[doc = "on the base class"]
+        #[doc = " Inherited hasChildren from the base class"]
         #[cxx_name = "hasChildrenCxxQtInherit"]
         fn has_children_super(self: &MyObjectQt, parent: &QModelIndex) -> bool;
     }
     extern "C++" {
-        #[doc = "CXX-Qt generated method which calls the C++ method"]
-        #[doc = "fetchMore"]
-        #[doc = "on the base class"]
+        #[doc = " Inherited fetchMore from the base class"]
         #[cxx_name = "fetchMoreCxxQtInherit"]
         unsafe fn fetch_more(self: Pin<&mut MyObjectQt>, index: &QModelIndex);
     }

diff --git a/examples/qml_features/rust/src/custom_base_class.rs b/examples/qml_features/rust/src/custom_base_class.rs
@@ -161,33 +161,41 @@ pub mod ffi {
     // Create Rust bindings for C++ functions of the base class (QAbstractItemModel)
     #[cxx_qt::inherit]
     extern "C++" {
+        /// Inherited beginInsertRows from the base class
         unsafe fn begin_insert_rows(
             self: Pin<&mut qobject::CustomBaseClass>,
             parent: &QModelIndex,
             first: i32,
             last: i32,
         );
+        /// Inherited endInsertRows from the base class
         unsafe fn end_insert_rows(self: Pin<&mut qobject::CustomBaseClass>);
 
+        /// Inherited beginRemoveRows from the base class
         unsafe fn begin_remove_rows(
             self: Pin<&mut qobject::CustomBaseClass>,
             parent: &QModelIndex,
             first: i32,
             last: i32,
         );
+        /// Inherited endRemoveRows from the base class
         unsafe fn end_remove_rows(self: Pin<&mut qobject::CustomBaseClass>);
 
+        /// Inherited beginResetModel from the base class
         unsafe fn begin_reset_model(self: Pin<&mut qobject::CustomBaseClass>);
+        /// Inherited endResetModel from the base class
         unsafe fn end_reset_model(self: Pin<&mut qobject::CustomBaseClass>);
     }
     // ANCHOR_END: book_inherit_qalm_impl_unsafe
 
     // ANCHOR: book_inherit_qalm_impl_safe
     #[cxx_qt::inherit]
     unsafe extern "C++" {
+        /// Inherited canFetchMore from the base class
         #[cxx_name = "canFetchMore"]
         fn base_can_fetch_more(self: &qobject::CustomBaseClass, parent: &QModelIndex) -> bool;
 
+        /// Inherited index from the base class
         fn index(
             self: &qobject::CustomBaseClass,
             row: i32,