From 0c74c6d1871e793d4e5ed01cc7e6dcc18f4e6cb2 Mon Sep 17 00:00:00 2001 From: Andrew Hayzen Date: Thu, 1 Jun 2023 15:15:40 +0100 Subject: [PATCH] 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 #557 --- .../cxx-qt-gen/src/generator/rust/inherit.rs | 17 ++------ crates/cxx-qt-gen/src/parser/inherit.rs | 43 +++++++++++++------ crates/cxx-qt-gen/test_inputs/inheritance.rs | 2 + crates/cxx-qt-gen/test_outputs/inheritance.rs | 8 +--- .../rust/src/custom_base_class.rs | 8 ++++ 5 files changed, 45 insertions(+), 33 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/rust/inherit.rs b/crates/cxx-qt-gen/src/generator/rust/inherit.rs index ef1d7ae1c..d4b8f3708 100644 --- 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::>(); 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 index 2b2105c57..b681cc78f 100644 --- 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 { 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 { + pub fn parse(mut method: ForeignItemFn, safety: Safety) -> Result { 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 index e98ff8188..feed2de87 100644 --- 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 index f021c7753..374ef4320 100644 --- 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 index e26ac2410..437fca87d 100644 --- a/examples/qml_features/rust/src/custom_base_class.rs +++ b/examples/qml_features/rust/src/custom_base_class.rs @@ -161,23 +161,29 @@ 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 @@ -185,9 +191,11 @@ pub mod ffi { // 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,