Skip to content

Commit

Permalink
cxx-qt-gen: ensure that attributes are handled correctly for cxx_qt::…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
ahayzen-kdab committed Jun 7, 2023
1 parent 23eaa42 commit 0c74c6d
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 33 deletions.
17 changes: 3 additions & 14 deletions crates/cxx-qt-gen/src/generator/rust/inherit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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,
)
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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,);
}
Expand Down
43 changes: 30 additions & 13 deletions crates/cxx-qt-gen/src/parser/inherit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {
Expand Down Expand Up @@ -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(),
Expand All @@ -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 {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
});
Expand Down
2 changes: 2 additions & 0 deletions crates/cxx-qt-gen/test_inputs/inheritance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
8 changes: 2 additions & 6 deletions crates/cxx-qt-gen/test_outputs/inheritance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
8 changes: 8 additions & 0 deletions examples/qml_features/rust/src/custom_base_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 0c74c6d

Please sign in to comment.