From b0a97008534917d56c826af5ff989e5f89d6a00b Mon Sep 17 00:00:00 2001 From: Andrew Hayzen Date: Wed, 20 Dec 2023 10:23:52 +0000 Subject: [PATCH] cxx-qt-gen: allow for optional #[qobject] macro This allows for using CXX-Qt helpers with normal C++ classes, eg #[inherit] and #[cxx_override] etc. Closes #824 --- CHANGELOG.md | 1 + .../src/generator/cpp/constructor.rs | 64 ++++++++++++- .../cxx-qt-gen/src/generator/cpp/inherit.rs | 3 + .../cxx-qt-gen/src/generator/cpp/qobject.rs | 17 +++- crates/cxx-qt-gen/src/parser/cxxqtdata.rs | 91 +++++++++++++++---- crates/cxx-qt-gen/src/parser/qobject.rs | 3 + crates/cxx-qt-gen/src/writer/cpp/header.rs | 38 ++++++-- crates/cxx-qt-gen/src/writer/cpp/mod.rs | 3 + 8 files changed, 186 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8ea38124..c3eca7eaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `cxx-qt-gen` now does not generate code requiring `cxx-qt-lib`, this allows for `cxx-qt-lib` to be optional - `cxx-qt-lib` headers must be given to `cxx-qt-build` with `.with_opts(cxx_qt_lib_headers::build_opts())` - File name is used for CXX bridges rather than module name to match upstream +- `#[qobject]` attribute is now optional on types in `extern "RustQt"` ### Fixed diff --git a/crates/cxx-qt-gen/src/generator/cpp/constructor.rs b/crates/cxx-qt-gen/src/generator/cpp/constructor.rs index 2157dcd00..8b112ecee 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/constructor.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/constructor.rs @@ -17,8 +17,8 @@ fn default_constructor( base_class: String, initializers: String, ) -> GeneratedCppQObjectBlocks { - GeneratedCppQObjectBlocks { - methods: vec![CppFragment::Pair { + let constructor = if qobject.has_qobject_macro { + CppFragment::Pair { header: format!( "explicit {class_name}(QObject* parent = nullptr);", class_name = qobject.ident @@ -34,7 +34,33 @@ fn default_constructor( namespace_internals = qobject.namespace_internals, rust_obj = qobject.rust_ident, ), - }], + } + } else { + CppFragment::Pair { + header: format!("explicit {class_name}();", class_name = qobject.ident), + source: formatdoc!( + r#" + {class_name}::{class_name}() + {base_class_line} + , ::rust::cxxqt1::CxxQtType<{rust_obj}>(::{namespace_internals}::createRs()){initializers} + {{ }} + "#, + base_class_line = if base_class.is_empty() { + unreachable!( + "Cannot have an empty #[base] attribute with no #[qobject] attribute" + ); + } else { + format!(": {base_class}()") + }, + class_name = qobject.ident, + namespace_internals = qobject.namespace_internals, + rust_obj = qobject.rust_ident, + ), + } + }; + + GeneratedCppQObjectBlocks { + methods: vec![constructor], ..Default::default() } } @@ -143,6 +169,7 @@ mod tests { namespace: "".to_string(), namespace_internals: "rust".to_string(), blocks: GeneratedCppQObjectBlocks::default(), + has_qobject_macro: true, } } @@ -222,6 +249,37 @@ mod tests { ); } + #[test] + fn default_constructor_no_qobject_macro() { + let mut qobject = qobject_for_testing(); + qobject.has_qobject_macro = false; + let blocks = generate( + &qobject, + &[], + "BaseClass".to_owned(), + &[], + &TypeNames::default(), + ) + .unwrap(); + + assert_empty_blocks(&blocks); + assert!(blocks.private_methods.is_empty()); + assert_eq!( + blocks.methods, + vec![CppFragment::Pair { + header: "explicit MyObject();".to_string(), + source: formatdoc!( + " + MyObject::MyObject() + : BaseClass() + , ::rust::cxxqt1::CxxQtType(::rust::createRs()) + {{ }} + " + ), + }] + ); + } + #[test] fn constructor_without_base_arguments() { let blocks = generate( diff --git a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs index 83a20773b..88298b1c1 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs @@ -22,6 +22,9 @@ pub fn generate( for method in inherited_methods { let return_type = syn_type_to_cpp_return_type(&method.method.sig.output, type_names)?; + // Note that no qobject macro with no base class is an error + // + // So a default of QObject is fine here let base_class = base_class.as_deref().unwrap_or("QObject"); result.methods.push(CppFragment::Header(formatdoc! { diff --git a/crates/cxx-qt-gen/src/generator/cpp/qobject.rs b/crates/cxx-qt-gen/src/generator/cpp/qobject.rs index eaa5d9c9d..f3c834ae7 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/qobject.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/qobject.rs @@ -92,6 +92,8 @@ pub struct GeneratedCppQObject { pub namespace_internals: String, /// The blocks of the QObject pub blocks: GeneratedCppQObjectBlocks, + /// Whether this type has a #[qobject] / Q_OBJECT macro + pub has_qobject_macro: bool, } impl GeneratedCppQObject { @@ -106,6 +108,7 @@ impl GeneratedCppQObject { namespace: qobject.namespace.clone(), namespace_internals: namespace_idents.internal, blocks: GeneratedCppQObjectBlocks::from(qobject), + has_qobject_macro: qobject.has_qobject_macro, }; // Ensure that we include MaybeLockGuard that is used in multiple places @@ -115,10 +118,16 @@ impl GeneratedCppQObject { .insert("#include ".to_owned()); // Build the base class - let base_class = qobject - .base_class - .clone() - .unwrap_or_else(|| "QObject".to_string()); + let base_class = qobject.base_class.clone().unwrap_or_else(|| { + // If there is a QObject macro then assume the base class is QObject + if qobject.has_qobject_macro { + "QObject".to_string() + } else { + unreachable!( + "Cannot have an empty #[base] attribute with no #[qobject] attribute" + ); + } + }); generated.blocks.base_classes.push(base_class.clone()); // Add the CxxQtType rust and rust_mut methods diff --git a/crates/cxx-qt-gen/src/parser/cxxqtdata.rs b/crates/cxx-qt-gen/src/parser/cxxqtdata.rs index 349a5552d..2c1165521 100644 --- a/crates/cxx-qt-gen/src/parser/cxxqtdata.rs +++ b/crates/cxx-qt-gen/src/parser/cxxqtdata.rs @@ -80,27 +80,44 @@ impl ParsedCxxQtData { syn::parse2(tokens.clone())?; // Check this type is tagged with a #[qobject] - if attribute_take_path(&mut foreign_alias.attrs, &["qobject"]) - .is_some() + let has_qobject_macro = + attribute_take_path(&mut foreign_alias.attrs, &["qobject"]) + .is_some(); + + // Load the QObject + let mut qobject = ParsedQObject::try_from(&foreign_alias)?; + qobject.has_qobject_macro = has_qobject_macro; + + // Ensure that the base class attribute is not empty, as this is not valid in both cases + // - when there is a qobject macro it is not valid + // - when there is not a qobject macro it is not valid + if qobject + .base_class + .as_ref() + .is_some_and(|base| base.is_empty()) { - // Load the QObject - let mut qobject = ParsedQObject::try_from(&foreign_alias)?; - - // Inject the bridge namespace if the qobject one is empty - if qobject.namespace.is_empty() && namespace.is_some() { - qobject.namespace = namespace.clone().unwrap(); - } - - // Note that we assume a compiler error will occur later - // if you had two structs with the same name - self.qobjects - .insert(foreign_alias.ident_left.clone(), qobject); - } else { return Err(Error::new( foreign_item.span(), - "type A = super::B must be tagged with #[qobject]", + "The #[base] attribute cannot be empty", )); } + + // Ensure that if there is no qobject macro that a base class is specificed + // + // Note this assumes the check above + if !qobject.has_qobject_macro && qobject.base_class.is_none() { + return Err(Error::new(foreign_item.span(), "A type without a #[qobject] attribute must specify a #[base] attribute")); + } + + // Inject the bridge namespace if the qobject one is empty + if qobject.namespace.is_empty() && namespace.is_some() { + qobject.namespace = namespace.clone().unwrap(); + } + + // Note that we assume a compiler error will occur later + // if you had two structs with the same name + self.qobjects + .insert(foreign_alias.ident_left.clone(), qobject); } // Const Macro, Type are unsupported in extern "RustQt" for now _others => { @@ -316,6 +333,13 @@ mod tests { assert!(result.is_ok()); assert_eq!(cxx_qt_data.qobjects.len(), 1); assert!(cxx_qt_data.qobjects.contains_key(&qobject_ident())); + assert!( + cxx_qt_data + .qobjects + .get(&qobject_ident()) + .unwrap() + .has_qobject_macro + ); } #[test] @@ -380,7 +404,7 @@ mod tests { } #[test] - fn test_find_qobjects_no_qobject() { + fn test_find_qobjects_no_qobject_no_base() { let mut cxx_qt_data = ParsedCxxQtData::new(format_ident!("ffi"), None); let module: ItemMod = parse_quote! { @@ -395,6 +419,39 @@ mod tests { assert!(result.is_err()); } + #[test] + fn test_find_qobjects_no_qobject_with_base() { + let mut cxx_qt_data = ParsedCxxQtData::new(format_ident!("ffi"), None); + + let module: ItemMod = parse_quote! { + mod module { + extern "RustQt" { + #[base = "OtherBase"] + type Other = super::OtherRust; + #[base = "MyObjectBase"] + type MyObject = super::MyObjectRust; + } + } + }; + let result = cxx_qt_data.find_qobject_types(&module.content.unwrap().1); + assert!(result.is_ok()); + assert_eq!(cxx_qt_data.qobjects.len(), 2); + assert!( + !cxx_qt_data + .qobjects + .get(&format_ident!("Other")) + .unwrap() + .has_qobject_macro + ); + assert!( + !cxx_qt_data + .qobjects + .get(&format_ident!("MyObject")) + .unwrap() + .has_qobject_macro + ); + } + #[test] fn test_find_and_merge_cxx_qt_item_struct_qobject_passthrough() { let mut cxx_qt_data = create_parsed_cxx_qt_data(); diff --git a/crates/cxx-qt-gen/src/parser/qobject.rs b/crates/cxx-qt-gen/src/parser/qobject.rs index fbcc0d607..f60e0a08f 100644 --- a/crates/cxx-qt-gen/src/parser/qobject.rs +++ b/crates/cxx-qt-gen/src/parser/qobject.rs @@ -57,6 +57,8 @@ pub struct ParsedQObject { pub locking: bool, /// Whether threading has been enabled for this QObject pub threading: bool, + /// Whether this type has a #[qobject] / Q_OBJECT macro + pub has_qobject_macro: bool, } impl TryFrom<&ForeignTypeIdentAlias> for ParsedQObject { @@ -97,6 +99,7 @@ impl TryFrom<&ForeignTypeIdentAlias> for ParsedQObject { qml_metadata, locking: true, threading: false, + has_qobject_macro: false, }) } } diff --git a/crates/cxx-qt-gen/src/writer/cpp/header.rs b/crates/cxx-qt-gen/src/writer/cpp/header.rs index a491913ef..a561486a9 100644 --- a/crates/cxx-qt-gen/src/writer/cpp/header.rs +++ b/crates/cxx-qt-gen/src/writer/cpp/header.rs @@ -76,12 +76,23 @@ fn forward_declare(generated: &GeneratedCppBlocks) -> Vec { /// For a given GeneratedCppBlocks write the classes fn qobjects_header(generated: &GeneratedCppBlocks) -> Vec { generated.qobjects.iter().map(|qobject| { + let ident = &qobject.ident; + let qobject_macro = if qobject.has_qobject_macro { + "Q_OBJECT" + } else { + "" + }; + let qobject_assert = if qobject.has_qobject_macro { + format!("static_assert(::std::is_base_of::value, \"{ident} must inherit from QObject\");") + } else { + "".to_owned() + }; let class_definition = namespaced( &qobject.namespace, &formatdoc! { r#" class {ident} : {base_classes} {{ - Q_OBJECT + {qobject_macro} public: {metaobjects} @@ -91,8 +102,8 @@ fn qobjects_header(generated: &GeneratedCppBlocks) -> Vec { {private_methods} }}; - static_assert(::std::is_base_of::value, "{ident} must inherit from QObject");"#, - ident = qobject.ident, + {qobject_assert}"#, + // Note that there is always a base class as we always have CxxQtType base_classes = qobject.blocks.base_classes.iter().map(|base| format!("public {}", base)).collect::>().join(", "), metaobjects = qobject.blocks.metaobjects.join("\n "), public_methods = create_block("public", &qobject.blocks.methods.iter().filter_map(pair_as_header).collect::>()), @@ -107,17 +118,24 @@ fn qobjects_header(generated: &GeneratedCppBlocks) -> Vec { .collect::>() .join("\n"); + let declare_metatype = if qobject.has_qobject_macro { + let ty = if qobject.namespace.is_empty() { + qobject.ident.clone() + } else { + format!("{namespace}::{ident}", namespace = qobject.namespace, ident = qobject.ident) + }; + format!("Q_DECLARE_METATYPE({ty}*)") + } else { + "".to_owned() + }; + formatdoc! {r#" {fragments} {class_definition} - Q_DECLARE_METATYPE({metatype}*) - "#, - metatype = if qobject.namespace.is_empty() { - qobject.ident.clone() - } else { - format!("{namespace}::{ident}", namespace = qobject.namespace, ident = qobject.ident) - },} + {declare_metatype} + "# + } }).collect::>() } diff --git a/crates/cxx-qt-gen/src/writer/cpp/mod.rs b/crates/cxx-qt-gen/src/writer/cpp/mod.rs index 265f84e4c..5f3d047de 100644 --- a/crates/cxx-qt-gen/src/writer/cpp/mod.rs +++ b/crates/cxx-qt-gen/src/writer/cpp/mod.rs @@ -62,6 +62,7 @@ mod tests { rust_ident: "MyObjectRust".to_owned(), namespace: "cxx_qt::my_object".to_owned(), namespace_internals: "cxx_qt::my_object::cxx_qt_my_object".to_owned(), + has_qobject_macro: true, blocks: GeneratedCppQObjectBlocks { base_classes: vec!["QStringListModel".to_owned()], includes: { @@ -185,6 +186,7 @@ mod tests { rust_ident: "FirstObjectRust".to_owned(), namespace: "cxx_qt".to_owned(), namespace_internals: "cxx_qt::cxx_qt_first_object".to_owned(), + has_qobject_macro: true, blocks: GeneratedCppQObjectBlocks { base_classes: vec!["QStringListModel".to_owned()], includes: { @@ -228,6 +230,7 @@ mod tests { rust_ident: "SecondObjectRust".to_owned(), namespace: "cxx_qt".to_owned(), namespace_internals: "cxx_qt::cxx_qt_second_object".to_owned(), + has_qobject_macro: true, blocks: GeneratedCppQObjectBlocks { base_classes: vec!["QStringListModel".to_owned()], includes: {