From 589d0cc1c120ca25e510e429dd8a81e661cb635d Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Tue, 29 Nov 2022 18:33:33 +0100 Subject: [PATCH 01/14] Parse cxx_qt::inherit! into ParsedInheritedMethod --- crates/cxx-qt-gen/src/lib.rs | 5 + crates/cxx-qt-gen/src/parser/inherit.rs | 46 ++++++ crates/cxx-qt-gen/src/parser/mod.rs | 1 + crates/cxx-qt-gen/src/parser/qobject.rs | 66 +++++++- crates/cxx-qt-gen/test_inputs/inheritance.rs | 34 ++++ .../test_inputs/inheritance.rs.license | 5 + .../cxx-qt-gen/test_outputs/inheritance.cpp | 59 +++++++ .../test_outputs/inheritance.cpp.license | 5 + crates/cxx-qt-gen/test_outputs/inheritance.h | 47 ++++++ .../test_outputs/inheritance.h.license | 5 + crates/cxx-qt-gen/test_outputs/inheritance.rs | 150 ++++++++++++++++++ .../test_outputs/inheritance.rs.license | 5 + 12 files changed, 421 insertions(+), 7 deletions(-) create mode 100644 crates/cxx-qt-gen/src/parser/inherit.rs create mode 100644 crates/cxx-qt-gen/test_inputs/inheritance.rs create mode 100644 crates/cxx-qt-gen/test_inputs/inheritance.rs.license create mode 100644 crates/cxx-qt-gen/test_outputs/inheritance.cpp create mode 100644 crates/cxx-qt-gen/test_outputs/inheritance.cpp.license create mode 100644 crates/cxx-qt-gen/test_outputs/inheritance.h create mode 100644 crates/cxx-qt-gen/test_outputs/inheritance.h.license create mode 100644 crates/cxx-qt-gen/test_outputs/inheritance.rs create mode 100644 crates/cxx-qt-gen/test_outputs/inheritance.rs.license diff --git a/crates/cxx-qt-gen/src/lib.rs b/crates/cxx-qt-gen/src/lib.rs index f91c46128..fe54db87f 100644 --- a/crates/cxx-qt-gen/src/lib.rs +++ b/crates/cxx-qt-gen/src/lib.rs @@ -128,4 +128,9 @@ mod tests { fn generates_signals() { test_code_generation!("signals"); } + + #[test] + fn generates_inheritance() { + test_code_generation!("inheritance"); + } } diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs new file mode 100644 index 000000000..da8e0fc42 --- /dev/null +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -0,0 +1,46 @@ +use crate::{ + parser::parameter::ParsedFunctionParameter, syntax::implitemmethod::is_method_mutable, +}; +use syn::{ + parse::{Parse, ParseStream}, + ForeignItemFn, Result, +}; + +/// This type is used when parsing the `cxx_qt::inherit!` macro contents into raw ForeignItemFn items +pub struct InheritMethods { + pub base_functions: Vec, +} + +impl Parse for InheritMethods { + fn parse(input: ParseStream) -> Result { + let mut base_functions = Vec::new(); + while !input.is_empty() { + base_functions.push(input.parse::()?); + } + Ok(InheritMethods { base_functions }) + } +} + +/// Describes a method found in cxx_qt::inherit! +pub struct ParsedInheritedMethod { + /// The original [syn::ForeignItemFn] of the inherited method declaration + pub method: ForeignItemFn, + /// whether the inherited method is marked as mutable + pub mutable: bool, + /// the parameters of the method, without the `self` argument + pub parameters: Vec, +} + +impl ParsedInheritedMethod { + pub fn parse(method: ForeignItemFn) -> Result { + let mutable = is_method_mutable(&method.sig); + + let parameters = ParsedFunctionParameter::parse_all_without_receiver(&method.sig)?; + + Ok(Self { + method, + mutable, + parameters, + }) + } +} diff --git a/crates/cxx-qt-gen/src/parser/mod.rs b/crates/cxx-qt-gen/src/parser/mod.rs index 8688db259..ae55c0545 100644 --- a/crates/cxx-qt-gen/src/parser/mod.rs +++ b/crates/cxx-qt-gen/src/parser/mod.rs @@ -4,6 +4,7 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 pub mod cxxqtdata; +pub mod inherit; pub mod invokable; pub mod parameter; pub mod property; diff --git a/crates/cxx-qt-gen/src/parser/qobject.rs b/crates/cxx-qt-gen/src/parser/qobject.rs index bd0a11a9d..32876fe38 100644 --- a/crates/cxx-qt-gen/src/parser/qobject.rs +++ b/crates/cxx-qt-gen/src/parser/qobject.rs @@ -3,18 +3,22 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::{ - invokable::ParsedQInvokable, - property::{ParsedQProperty, ParsedRustField}, - signals::ParsedSignalsEnum, -}; use crate::syntax::{ attribute::{attribute_find_path, attribute_tokens_to_map, AttributeDefault}, fields::fields_to_named_fields_mut, }; +use crate::{ + parser::{ + inherit::*, + invokable::ParsedQInvokable, + property::{ParsedQProperty, ParsedRustField}, + signals::ParsedSignalsEnum, + }, + syntax, +}; use syn::{ - spanned::Spanned, Error, Fields, Ident, ImplItem, ImplItemMethod, Item, ItemStruct, LitStr, - Result, + spanned::Spanned, Error, Fields, Ident, ImplItem, ImplItemMacro, ImplItemMethod, Item, + ItemStruct, LitStr, Result, }; /// Metadata for registering QML element @@ -46,6 +50,8 @@ pub struct ParsedQObject { /// /// These will also be exposed as Q_INVOKABLE on the C++ object pub invokables: Vec, + /// List of inherited methods + pub inherited_methods: Vec, /// List of methods that need to be implemented on the C++ object in Rust /// /// Note that they will only be visible on the Rust side @@ -97,6 +103,7 @@ impl ParsedQObject { namespace, signals: None, invokables: vec![], + inherited_methods: vec![], methods: vec![], properties, fields, @@ -182,6 +189,17 @@ impl ParsedQObject { Ok(()) } + fn parse_impl_inherit(&mut self, mac: &ImplItemMacro) -> Result<()> { + let inherited: InheritMethods = mac.mac.parse_body()?; + + for method in inherited.base_functions.into_iter() { + self.inherited_methods + .push(ParsedInheritedMethod::parse(method)?); + } + + 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 @@ -192,6 +210,11 @@ impl ParsedQObject { ImplItem::Method(method) => { self.parse_impl_method(method)?; } + ImplItem::Macro(mac) => { + if syntax::path::path_compare_str(&mac.mac.path, &["cxx_qt", "inherit"]) { + self.parse_impl_inherit(mac)?; + } + } _ => { return Err(Error::new(item.span(), "Only methods are supported.")); } @@ -357,6 +380,35 @@ pub mod tests { .contains(&ParsedQInvokableSpecifiers::Virtual)); } + #[test] + fn test_parse_inherited_methods() { + let mut qobject = create_parsed_qobject(); + let item: ItemImpl = tokens_to_syn(quote! { + impl T { + cxx_qt::inherit! { + fn test(&self); + + fn with_args(&self, arg: i32); + + #[cxx_name="withRename"] + fn with_rename(self: Pin<&mut Self>, arg: i32); + } + } + }); + assert!(qobject.parse_impl_items(&item.items).is_ok()); + + let inherited = &qobject.inherited_methods; + assert_eq!(inherited.len(), 3); + assert_eq!(inherited[0].mutable, false); + assert_eq!(inherited[1].mutable, false); + assert_eq!(inherited[2].mutable, true); + assert_eq!(inherited[0].parameters.len(), 0); + assert_eq!(inherited[1].parameters.len(), 1); + assert_eq!(inherited[1].parameters[0].ident, "arg"); + assert_eq!(inherited[2].parameters.len(), 1); + assert_eq!(inherited[2].parameters[0].ident, "arg"); + } + #[test] fn test_parse_impl_items_invalid() { let mut qobject = create_parsed_qobject(); diff --git a/crates/cxx-qt-gen/test_inputs/inheritance.rs b/crates/cxx-qt-gen/test_inputs/inheritance.rs new file mode 100644 index 000000000..051d06f16 --- /dev/null +++ b/crates/cxx-qt-gen/test_inputs/inheritance.rs @@ -0,0 +1,34 @@ +#[cxx_qt::bridge] +mod inheritance { + extern "C++" { + include!("cxx-qt-lib/qmodelindex.h"); + type QModelIndex = cxx_qt_lib::QModelIndex; + include!("cxx-qt-lib/qvariant.h"); + type QVariant = cxx_qt_lib::QVariant; + } + + #[cxx_qt::qobject(base = "QAbstractItemModel")] + #[derive(Default)] + pub struct MyObject { + data: Vec, + } + + impl qobject::MyObject { + #[qinvokable(cxx_override)] + pub fn data(&self, _index: &QModelIndex, _role: i32) -> QVariant { + QVariant::default() + } + + cxx_qt::inherit! { + fn buddy(&self, index: &QModelIndex) -> QModelIndex; + + #[cxx_name="hasChildren"] + fn has_children_super(&self, parent: &QModelIndex) -> bool; + } + + #[qinvokable(cxx_override)] + pub fn has_children(&self, _parent: &QModelIndex) -> bool { + false + } + } +} diff --git a/crates/cxx-qt-gen/test_inputs/inheritance.rs.license b/crates/cxx-qt-gen/test_inputs/inheritance.rs.license new file mode 100644 index 000000000..61425b8fc --- /dev/null +++ b/crates/cxx-qt-gen/test_inputs/inheritance.rs.license @@ -0,0 +1,5 @@ +SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +SPDX-FileContributor: Leon Matthes + +SPDX-License-Identifier: MIT OR Apache-2.0 + diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.cpp b/crates/cxx-qt-gen/test_outputs/inheritance.cpp new file mode 100644 index 000000000..263f99d15 --- /dev/null +++ b/crates/cxx-qt-gen/test_outputs/inheritance.cpp @@ -0,0 +1,59 @@ +#include "cxx-qt-gen/inheritance.cxxqt.h" + +MyObject::MyObject(QObject* parent) + : QAbstractItemModel(parent) + , m_rustObj(cxx_qt_my_object::createRs()) + , m_rustObjMutex(std::make_shared()) + , m_cxxQtThreadObj( + std::make_shared>(this)) +{ +} + +MyObject::~MyObject() +{ + const auto guard = std::unique_lock(m_cxxQtThreadObj->mutex); + m_cxxQtThreadObj->ptr = nullptr; +} + +const MyObjectRust& +MyObject::unsafeRust() const +{ + return *m_rustObj; +} + +MyObjectRust& +MyObject::unsafeRustMut() +{ + return *m_rustObj; +} + +std::unique_ptr +MyObject::qtThread() const +{ + return std::make_unique(m_cxxQtThreadObj, + m_rustObjMutex); +} + +QVariant +MyObject::data(const QModelIndex& _index, qint32 _role) const +{ + const std::lock_guard guard(*m_rustObjMutex); + return rust::cxxqtlib1::cxx_qt_convert{}( + m_rustObj->dataWrapper(*this, _index, _role)); +} + +bool +MyObject::hasChildren(const QModelIndex& _parent) const +{ + const std::lock_guard guard(*m_rustObjMutex); + return rust::cxxqtlib1::cxx_qt_convert{}( + m_rustObj->hasChildrenWrapper(*this, _parent)); +} + +namespace cxx_qt_my_object { +std::unique_ptr +newCppObject() +{ + return std::make_unique(); +} +} // namespace cxx_qt_my_object diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.cpp.license b/crates/cxx-qt-gen/test_outputs/inheritance.cpp.license new file mode 100644 index 000000000..61425b8fc --- /dev/null +++ b/crates/cxx-qt-gen/test_outputs/inheritance.cpp.license @@ -0,0 +1,5 @@ +SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +SPDX-FileContributor: Leon Matthes + +SPDX-License-Identifier: MIT OR Apache-2.0 + diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.h b/crates/cxx-qt-gen/test_outputs/inheritance.h new file mode 100644 index 000000000..9560dbe90 --- /dev/null +++ b/crates/cxx-qt-gen/test_outputs/inheritance.h @@ -0,0 +1,47 @@ +#pragma once + +#include +#include + +namespace rust::cxxqtlib1 { +template +class CxxQtThread; +} + +class MyObject; +using MyObjectCxxQtThread = rust::cxxqtlib1::CxxQtThread; + +#include "cxx-qt-gen/inheritance.cxx.h" + +class MyObject : public QAbstractItemModel +{ + Q_OBJECT + +public: + explicit MyObject(QObject* parent = nullptr); + ~MyObject(); + const MyObjectRust& unsafeRust() const; + MyObjectRust& unsafeRustMut(); + std::unique_ptr qtThread() const; + +public: + Q_INVOKABLE QVariant data(const QModelIndex& _index, + qint32 _role) const override; + Q_INVOKABLE bool hasChildren(const QModelIndex& _parent) const override; + +private: + rust::Box m_rustObj; + std::shared_ptr m_rustObjMutex; + std::shared_ptr> + m_cxxQtThreadObj; +}; + +static_assert(std::is_base_of::value, + "MyObject must inherit from QObject"); + +namespace cxx_qt_my_object { +std::unique_ptr +newCppObject(); +} // namespace cxx_qt_my_object + +Q_DECLARE_METATYPE(MyObject*) diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.h.license b/crates/cxx-qt-gen/test_outputs/inheritance.h.license new file mode 100644 index 000000000..61425b8fc --- /dev/null +++ b/crates/cxx-qt-gen/test_outputs/inheritance.h.license @@ -0,0 +1,5 @@ +SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +SPDX-FileContributor: Leon Matthes + +SPDX-License-Identifier: MIT OR Apache-2.0 + diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.rs b/crates/cxx-qt-gen/test_outputs/inheritance.rs new file mode 100644 index 000000000..fae308c4f --- /dev/null +++ b/crates/cxx-qt-gen/test_outputs/inheritance.rs @@ -0,0 +1,150 @@ +#[cxx::bridge(namespace = "")] +mod inheritance { + extern "C++" { + include!("cxx-qt-lib/qmodelindex.h"); + type QModelIndex = cxx_qt_lib::QModelIndex; + include!("cxx-qt-lib/qvariant.h"); + type QVariant = cxx_qt_lib::QVariant; + } + unsafe extern "C++" { + include ! (< QtCore / QObject >); + include!("cxx-qt-lib/convert.h"); + include!("cxx-qt-lib/cxxqt_thread.h"); + } + unsafe extern "C++" { + include!("cxx-qt-gen/inheritance.cxxqt.h"); + } + unsafe extern "C++" { + #[cxx_name = "MyObject"] + type MyObjectQt; + } + extern "Rust" { + #[cxx_name = "MyObjectRust"] + type MyObject; + } + extern "Rust" { + #[cxx_name = "dataWrapper"] + fn data_wrapper( + self: &MyObject, + cpp: &MyObjectQt, + _index: &QModelIndex, + _role: i32, + ) -> QVariant; + } + extern "Rust" { + #[cxx_name = "hasChildrenWrapper"] + fn has_children_wrapper(self: &MyObject, cpp: &MyObjectQt, _parent: &QModelIndex) -> bool; + } + unsafe extern "C++" { + type MyObjectCxxQtThread; + #[cxx_name = "unsafeRust"] + fn rust(self: &MyObjectQt) -> &MyObject; + #[cxx_name = "qtThread"] + fn qt_thread(self: &MyObjectQt) -> UniquePtr; + #[cxx_name = "queue"] + fn queue_boxed_fn( + self: &MyObjectCxxQtThread, + func: fn(Pin<&mut MyObjectQt>, Box), + arg: Box, + ) -> Result<()>; + #[rust_name = "new_cpp_object_my_object_qt"] + #[namespace = "cxx_qt_my_object"] + fn newCppObject() -> UniquePtr; + } + extern "C++" { + #[cxx_name = "unsafeRustMut"] + unsafe fn rust_mut(self: Pin<&mut MyObjectQt>) -> Pin<&mut MyObject>; + } + extern "Rust" { + #[namespace = "cxx_qt_my_object"] + type MyObjectCxxQtThreadQueuedFn; + #[cxx_name = "createRs"] + #[namespace = "cxx_qt_my_object"] + fn create_rs_my_object() -> Box; + } +} +pub use self::cxx_qt_inheritance::*; +mod cxx_qt_inheritance { + use super::inheritance::*; + use std::pin::Pin; + type UniquePtr = cxx::UniquePtr; + #[derive(Default)] + pub struct MyObject { + data: Vec, + } + impl MyObjectQt { + fn data(&self) -> &Vec { + &self.rust().data + } + } + impl MyObjectQt { + fn data_mut<'a>(mut self: Pin<&'a mut Self>) -> &'a mut Vec { + unsafe { &mut self.rust_mut().get_unchecked_mut().data } + } + } + impl MyObjectQt { + fn set_data(mut self: Pin<&mut Self>, value: Vec) { + unsafe { + self.rust_mut().data = value; + } + } + } + impl MyObject { + pub fn data_wrapper( + self: &MyObject, + cpp: &MyObjectQt, + _index: &QModelIndex, + _role: i32, + ) -> QVariant { + return cpp.data(_index, _role); + } + } + impl MyObjectQt { + pub fn data(&self, _index: &QModelIndex, _role: i32) -> QVariant { + QVariant::default() + } + } + impl MyObject { + pub fn has_children_wrapper( + self: &MyObject, + cpp: &MyObjectQt, + _parent: &QModelIndex, + ) -> bool { + return cpp.has_children(_parent); + } + } + impl MyObjectQt { + pub fn has_children(&self, _parent: &QModelIndex) -> bool { + false + } + } + unsafe impl Send for MyObjectCxxQtThread {} + impl MyObjectCxxQtThread { + pub fn queue(&self, f: F) -> std::result::Result<(), cxx::Exception> + where + F: FnOnce(std::pin::Pin<&mut MyObjectQt>), + F: Send + 'static, + { + #[allow(clippy::boxed_local)] + fn func( + obj: std::pin::Pin<&mut MyObjectQt>, + arg: std::boxed::Box, + ) { + (arg.inner)(obj) + } + let arg = MyObjectCxxQtThreadQueuedFn { + inner: std::boxed::Box::new(f), + }; + self.queue_boxed_fn(func, std::boxed::Box::new(arg)) + } + } + pub struct MyObjectCxxQtThreadQueuedFn { + inner: std::boxed::Box) + Send>, + } + pub fn create_rs_my_object() -> std::boxed::Box { + std::default::Default::default() + } + pub mod qobject { + pub type MyObject = super::MyObjectQt; + } +} diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.rs.license b/crates/cxx-qt-gen/test_outputs/inheritance.rs.license new file mode 100644 index 000000000..61425b8fc --- /dev/null +++ b/crates/cxx-qt-gen/test_outputs/inheritance.rs.license @@ -0,0 +1,5 @@ +SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +SPDX-FileContributor: Leon Matthes + +SPDX-License-Identifier: MIT OR Apache-2.0 + From f32fd051767175c3bc6c1b44dd7f97b3e7b966e8 Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Thu, 24 Nov 2022 17:21:55 +0100 Subject: [PATCH 02/14] cxx_qt::inherit! - add Rust generation --- .../src/generator/naming/invokable.rs | 10 +--- crates/cxx-qt-gen/src/generator/naming/mod.rs | 11 ++++ .../cxx-qt-gen/src/generator/rust/qobject.rs | 50 ++++++++++++++++++- crates/cxx-qt-gen/src/parser/inherit.rs | 29 ++++++++++- crates/cxx-qt-gen/test_inputs/inheritance.rs | 2 +- crates/cxx-qt-gen/test_outputs/inheritance.rs | 9 ++++ 6 files changed, 97 insertions(+), 14 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/naming/invokable.rs b/crates/cxx-qt-gen/src/generator/naming/invokable.rs index f8d1af387..9f81a47d5 100644 --- a/crates/cxx-qt-gen/src/generator/naming/invokable.rs +++ b/crates/cxx-qt-gen/src/generator/naming/invokable.rs @@ -23,20 +23,12 @@ impl From<&ImplItemMethod> for QInvokableName { fn from(method: &ImplItemMethod) -> Self { let ident = &method.sig.ident; Self { - name: name_from_ident(ident), + name: ident.clone().into(), wrapper: wrapper_from_ident(ident), } } } -/// For a given ident generate the Rust and C++ names -fn name_from_ident(ident: &Ident) -> CombinedIdent { - CombinedIdent { - cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), - rust: ident.clone(), - } -} - /// For a given ident generate the Rust and C++ wrapper names fn wrapper_from_ident(ident: &Ident) -> CombinedIdent { let ident = format_ident!("{ident}_wrapper"); diff --git a/crates/cxx-qt-gen/src/generator/naming/mod.rs b/crates/cxx-qt-gen/src/generator/naming/mod.rs index d9cde950b..0772f89b6 100644 --- a/crates/cxx-qt-gen/src/generator/naming/mod.rs +++ b/crates/cxx-qt-gen/src/generator/naming/mod.rs @@ -8,6 +8,8 @@ pub mod property; pub mod qobject; pub mod signals; +use convert_case::{Case, Casing}; +use quote::format_ident; use syn::Ident; /// Describes an ident which potentially has a different name in C++ and Rust @@ -17,3 +19,12 @@ pub struct CombinedIdent { /// The ident for rust pub rust: Ident, } + +impl From for CombinedIdent { + fn from(ident: Ident) -> Self { + Self { + cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), + rust: ident, + } + } +} diff --git a/crates/cxx-qt-gen/src/generator/rust/qobject.rs b/crates/cxx-qt-gen/src/generator/rust/qobject.rs index 1afe530e3..82c323804 100644 --- a/crates/cxx-qt-gen/src/generator/rust/qobject.rs +++ b/crates/cxx-qt-gen/src/generator/rust/qobject.rs @@ -12,9 +12,11 @@ use crate::{ signals::generate_rust_signals, }, }, - parser::qobject::ParsedQObject, + parser::{inherit::ParsedInheritedMethod, qobject::ParsedQObject}, }; -use quote::quote; +use convert_case::{Case, Casing}; +use proc_macro2::TokenStream; +use quote::{format_ident, quote, TokenStreamExt}; use syn::{Ident, ImplItemMethod, Item, Result}; #[derive(Default)] @@ -90,6 +92,10 @@ impl GeneratedRustQObject { generated .blocks .append(&mut generate_methods(&qobject.methods, &qobject_idents)?); + generated.blocks.append(&mut generate_inherited_methods( + &qobject_idents, + &*qobject.inherited_methods, + )?); if let Some(signals_enum) = &qobject.signals { generated @@ -143,6 +149,46 @@ pub fn generate_methods( Ok(blocks) } +fn generate_inherited_methods( + qobject_ident: &QObjectName, + methods: &[ParsedInheritedMethod], +) -> Result { + let mut blocks = GeneratedRustQObjectBlocks::default(); + let qobject_name = &qobject_ident.cpp_class.rust; + + let mut bridges = methods + .iter() + .map(|method| { + let parameters = method + .parameters + .iter() + .map(|parameter| { + let ident = ¶meter.ident; + let ty = ¶meter.ty; + quote! { #ident: #ty } + }) + .collect::>(); + let ident = &method.method.sig.ident; + let cxx_name_string = &method.ident.cpp.to_string(); + let self_param = if method.mutable { + quote! { self: Pin<&mut #qobject_name> } + } else { + quote! { self: &#qobject_name } + }; + let return_type = &method.method.sig.output; + syn::parse2(quote! { + unsafe extern "C++" { + #[cxx_name=#cxx_name_string] + fn #ident(#self_param, #(#parameters),*) #return_type; + } + }) + }) + .collect::>>()?; + + blocks.cxx_mod_contents.append(&mut bridges); + Ok(blocks) +} + /// Generate the C++ and Rust CXX definitions for the QObject fn generate_qobject_definitions( qobject_idents: &QObjectName, diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs index da8e0fc42..b3a2c0482 100644 --- a/crates/cxx-qt-gen/src/parser/inherit.rs +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -1,9 +1,16 @@ use crate::{ - parser::parameter::ParsedFunctionParameter, syntax::implitemmethod::is_method_mutable, + generator::naming::CombinedIdent, + parser::parameter::ParsedFunctionParameter, + syntax::{ + attribute::{attribute_tokens_to_ident, attribute_tokens_to_value}, + implitemmethod::is_method_mutable, + }, }; +use quote::format_ident; use syn::{ parse::{Parse, ParseStream}, - ForeignItemFn, Result, + spanned::Spanned, + Error, ForeignItemFn, LitStr, Result, }; /// This type is used when parsing the `cxx_qt::inherit!` macro contents into raw ForeignItemFn items @@ -29,6 +36,8 @@ pub struct ParsedInheritedMethod { pub mutable: bool, /// the parameters of the method, without the `self` argument pub parameters: Vec, + /// the name of the function in Rust, as well as C++ + pub ident: CombinedIdent, } impl ParsedInheritedMethod { @@ -37,10 +46,26 @@ impl ParsedInheritedMethod { let parameters = ParsedFunctionParameter::parse_all_without_receiver(&method.sig)?; + let mut ident = CombinedIdent::from(method.sig.ident.clone()); + for attribute in &method.attrs { + if !attribute.path.is_ident(&format_ident!("cxx_name")) { + return Err(Error::new( + attribute.span(), + "Unsupported attribute in cxx_qt::inherit!", + )); + } + + let name = attribute_tokens_to_value::(attribute)?; + + ident.cpp = format_ident!("{}", name.value()); + } + ident.cpp = format_ident!("{}_cxxqt_inherit", &ident.cpp); + Ok(Self { method, mutable, parameters, + ident, }) } } diff --git a/crates/cxx-qt-gen/test_inputs/inheritance.rs b/crates/cxx-qt-gen/test_inputs/inheritance.rs index 051d06f16..44e93240d 100644 --- a/crates/cxx-qt-gen/test_inputs/inheritance.rs +++ b/crates/cxx-qt-gen/test_inputs/inheritance.rs @@ -20,7 +20,7 @@ mod inheritance { } cxx_qt::inherit! { - fn buddy(&self, index: &QModelIndex) -> QModelIndex; + fn fetch_more(self: Pin<&mut Self>, index: &QModelIndex); #[cxx_name="hasChildren"] fn has_children_super(&self, parent: &QModelIndex) -> bool; diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.rs b/crates/cxx-qt-gen/test_outputs/inheritance.rs index fae308c4f..0494475c5 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.rs +++ b/crates/cxx-qt-gen/test_outputs/inheritance.rs @@ -17,6 +17,7 @@ mod inheritance { unsafe extern "C++" { #[cxx_name = "MyObject"] type MyObjectQt; + } extern "Rust" { #[cxx_name = "MyObjectRust"] @@ -35,6 +36,14 @@ mod inheritance { #[cxx_name = "hasChildrenWrapper"] fn has_children_wrapper(self: &MyObject, cpp: &MyObjectQt, _parent: &QModelIndex) -> bool; } + unsafe extern "C++" { + #[cxx_name = "fetchMore_cxxqt_inherit"] + fn fetch_more(self: Pin<&mut MyObjectQt>, index: &QModelIndex); + } + unsafe extern "C++" { + #[cxx_name = "hasChildren_cxxqt_inherit"] + fn has_children_super(self: &MyObjectQt, parent: &QModelIndex) -> bool; + } unsafe extern "C++" { type MyObjectCxxQtThread; #[cxx_name = "unsafeRust"] From 6b792079ae675bbfc779ae57ea7d5ed43525f75a Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Wed, 7 Dec 2022 14:52:09 +0100 Subject: [PATCH 03/14] CombinedIdent: Revert From implementation Use associated function `from_rust_function` instead. --- crates/cxx-qt-gen/src/generator/naming/invokable.rs | 2 +- crates/cxx-qt-gen/src/generator/naming/mod.rs | 4 ++-- crates/cxx-qt-gen/src/parser/inherit.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/naming/invokable.rs b/crates/cxx-qt-gen/src/generator/naming/invokable.rs index 9f81a47d5..e8787b175 100644 --- a/crates/cxx-qt-gen/src/generator/naming/invokable.rs +++ b/crates/cxx-qt-gen/src/generator/naming/invokable.rs @@ -23,7 +23,7 @@ impl From<&ImplItemMethod> for QInvokableName { fn from(method: &ImplItemMethod) -> Self { let ident = &method.sig.ident; Self { - name: ident.clone().into(), + name: CombinedIdent::from_rust_function(ident.clone()), wrapper: wrapper_from_ident(ident), } } diff --git a/crates/cxx-qt-gen/src/generator/naming/mod.rs b/crates/cxx-qt-gen/src/generator/naming/mod.rs index 0772f89b6..c245240f3 100644 --- a/crates/cxx-qt-gen/src/generator/naming/mod.rs +++ b/crates/cxx-qt-gen/src/generator/naming/mod.rs @@ -20,8 +20,8 @@ pub struct CombinedIdent { pub rust: Ident, } -impl From for CombinedIdent { - fn from(ident: Ident) -> Self { +impl CombinedIdent { + pub fn from_rust_function(ident: Ident) -> Self { Self { cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), rust: ident, diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs index b3a2c0482..03864cc5e 100644 --- a/crates/cxx-qt-gen/src/parser/inherit.rs +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -46,7 +46,7 @@ impl ParsedInheritedMethod { let parameters = ParsedFunctionParameter::parse_all_without_receiver(&method.sig)?; - let mut ident = CombinedIdent::from(method.sig.ident.clone()); + let mut ident = CombinedIdent::from_rust_function(method.sig.ident.clone()); for attribute in &method.attrs { if !attribute.path.is_ident(&format_ident!("cxx_name")) { return Err(Error::new( From 8f7213286dcc1aa78416ed9c0432b5f1411416ba Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Thu, 8 Dec 2022 15:33:49 +0100 Subject: [PATCH 04/14] cxx_qt::inherit: add rust generation --- .../cxx-qt-gen/src/generator/cpp/inherit.rs | 54 ++++++++++++++++ crates/cxx-qt-gen/src/generator/cpp/mod.rs | 1 + .../cxx-qt-gen/src/generator/cpp/qobject.rs | 5 +- .../cxx-qt-gen/src/generator/rust/qobject.rs | 7 +-- crates/cxx-qt-gen/src/parser/cxxqtdata.rs | 2 +- crates/cxx-qt-gen/src/parser/inherit.rs | 16 +++-- crates/cxx-qt-gen/src/parser/parameter.rs | 61 +++++++++++++------ crates/cxx-qt-gen/src/parser/qobject.rs | 11 ++-- crates/cxx-qt-gen/src/syntax/mod.rs | 1 + crates/cxx-qt-gen/src/syntax/types.rs | 33 ++++++++++ crates/cxx-qt-gen/src/writer/cpp/header.rs | 3 + .../cxx-qt-gen/test_outputs/inheritance.cpp | 31 +++++----- crates/cxx-qt-gen/test_outputs/inheritance.h | 32 ++++++---- crates/cxx-qt-gen/test_outputs/inheritance.rs | 1 + examples/qml_features/CMakeLists.txt | 2 - .../qml_features/cpp/qabstractlistmodelcxx.h | 40 ------------ .../rust/src/custom_base_class.rs | 43 ++++++------- 17 files changed, 213 insertions(+), 130 deletions(-) create mode 100644 crates/cxx-qt-gen/src/generator/cpp/inherit.rs create mode 100644 crates/cxx-qt-gen/src/syntax/types.rs delete mode 100644 examples/qml_features/cpp/qabstractlistmodelcxx.h diff --git a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs new file mode 100644 index 000000000..5f19f1ff1 --- /dev/null +++ b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs @@ -0,0 +1,54 @@ +// SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +// SPDX-FileContributor: Leon Matthes + +use indoc::formatdoc; + +// SPDX-License-Identifier: MIT OR Apache-2.0 +use crate::{ + generator::cpp::{fragment::CppFragment, qobject::GeneratedCppQObjectBlocks}, + parser::{cxxqtdata::ParsedCxxMappings, qobject::ParsedQObject}, +}; + +use syn::{Error, Result, ReturnType}; + +use super::types::CppType; + +pub fn generate( + qobject: &ParsedQObject, + cxx_mappings: &ParsedCxxMappings, +) -> Result { + let mut result = GeneratedCppQObjectBlocks::default(); + + for method in &qobject.inherited_methods { + let return_type = if let ReturnType::Type(_, ty) = &method.method.sig.output { + CppType::from(ty, &None, cxx_mappings)? + .as_cxx_ty() + .to_owned() + } else { + "void".to_owned() + }; + + let missing_base_class_error = || { + Error::new_spanned(&method.method, format!("C++ code generation failed!\nClass {} has inherited methods, but no base class!", qobject.qobject_struct.ident)) + }; + let base_class = qobject + .base_class + .clone() + .ok_or_else(missing_base_class_error)?; + + result.methods.push(CppFragment::Header(formatdoc! { + r#" + template + {return_type} {wrapper_ident}(Args ...args) {mutability} {{ + return {base_class}::{func_ident}(args...); + }}"#, + mutability = if method.mutable { "" } else { "const" }, + func_ident = method.ident.cpp, + wrapper_ident = method.wrapper_ident, + return_type = return_type, + base_class = base_class + })); + } + + Ok(result) +} diff --git a/crates/cxx-qt-gen/src/generator/cpp/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/mod.rs index 6c9fe9480..45c41dae3 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/mod.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/mod.rs @@ -4,6 +4,7 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 pub mod fragment; +pub mod inherit; pub mod invokable; pub mod property; pub mod qobject; diff --git a/crates/cxx-qt-gen/src/generator/cpp/qobject.rs b/crates/cxx-qt-gen/src/generator/cpp/qobject.rs index 9d6311c5a..74d124735 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/qobject.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/qobject.rs @@ -5,7 +5,7 @@ use crate::generator::{ cpp::{ - fragment::CppFragment, invokable::generate_cpp_invokables, + fragment::CppFragment, inherit, invokable::generate_cpp_invokables, property::generate_cpp_properties, signal::generate_cpp_signals, }, naming::{namespace::NamespaceName, qobject::QObjectName}, @@ -107,6 +107,9 @@ impl GeneratedCppQObject { cxx_mappings, )?); } + generated + .blocks + .append(&mut inherit::generate(qobject, cxx_mappings)?); Ok(generated) } diff --git a/crates/cxx-qt-gen/src/generator/rust/qobject.rs b/crates/cxx-qt-gen/src/generator/rust/qobject.rs index 82c323804..0661fcc8e 100644 --- a/crates/cxx-qt-gen/src/generator/rust/qobject.rs +++ b/crates/cxx-qt-gen/src/generator/rust/qobject.rs @@ -14,9 +14,8 @@ use crate::{ }, parser::{inherit::ParsedInheritedMethod, qobject::ParsedQObject}, }; -use convert_case::{Case, Casing}; use proc_macro2::TokenStream; -use quote::{format_ident, quote, TokenStreamExt}; +use quote::quote; use syn::{Ident, ImplItemMethod, Item, Result}; #[derive(Default)] @@ -94,7 +93,7 @@ impl GeneratedRustQObject { .append(&mut generate_methods(&qobject.methods, &qobject_idents)?); generated.blocks.append(&mut generate_inherited_methods( &qobject_idents, - &*qobject.inherited_methods, + &qobject.inherited_methods, )?); if let Some(signals_enum) = &qobject.signals { @@ -169,7 +168,7 @@ fn generate_inherited_methods( }) .collect::>(); let ident = &method.method.sig.ident; - let cxx_name_string = &method.ident.cpp.to_string(); + let cxx_name_string = &method.wrapper_ident.to_string(); let self_param = if method.mutable { quote! { self: Pin<&mut #qobject_name> } } else { diff --git a/crates/cxx-qt-gen/src/parser/cxxqtdata.rs b/crates/cxx-qt-gen/src/parser/cxxqtdata.rs index 97d1710d6..8278c796d 100644 --- a/crates/cxx-qt-gen/src/parser/cxxqtdata.rs +++ b/crates/cxx-qt-gen/src/parser/cxxqtdata.rs @@ -442,7 +442,7 @@ mod tests { let item: Item = tokens_to_syn(quote! { impl qobject::MyObject { #[qinvokable] - fn invokable() {} + fn invokable(&self) {} fn cpp_context() {} } diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs index 03864cc5e..6baf1628f 100644 --- a/crates/cxx-qt-gen/src/parser/inherit.rs +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -1,16 +1,18 @@ +// SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +// SPDX-FileContributor: Leon Matthes +// +// SPDX-License-Identifier: MIT OR Apache-2.0 + use crate::{ generator::naming::CombinedIdent, parser::parameter::ParsedFunctionParameter, - syntax::{ - attribute::{attribute_tokens_to_ident, attribute_tokens_to_value}, - implitemmethod::is_method_mutable, - }, + syntax::{attribute::attribute_tokens_to_value, implitemmethod::is_method_mutable}, }; use quote::format_ident; use syn::{ parse::{Parse, ParseStream}, spanned::Spanned, - Error, ForeignItemFn, LitStr, Result, + Error, ForeignItemFn, Ident, LitStr, Result, }; /// This type is used when parsing the `cxx_qt::inherit!` macro contents into raw ForeignItemFn items @@ -38,6 +40,7 @@ pub struct ParsedInheritedMethod { pub parameters: Vec, /// the name of the function in Rust, as well as C++ pub ident: CombinedIdent, + pub wrapper_ident: Ident, } impl ParsedInheritedMethod { @@ -59,13 +62,14 @@ impl ParsedInheritedMethod { ident.cpp = format_ident!("{}", name.value()); } - ident.cpp = format_ident!("{}_cxxqt_inherit", &ident.cpp); + let wrapper_ident = format_ident!("{}_cxxqt_inherit", &ident.cpp); Ok(Self { method, mutable, parameters, ident, + wrapper_ident, }) } } diff --git a/crates/cxx-qt-gen/src/parser/parameter.rs b/crates/cxx-qt-gen/src/parser/parameter.rs index 272cfdca5..c38419896 100644 --- a/crates/cxx-qt-gen/src/parser/parameter.rs +++ b/crates/cxx-qt-gen/src/parser/parameter.rs @@ -3,7 +3,11 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use syn::{spanned::Spanned, Error, FnArg, Ident, Pat, PatIdent, PatType, Result, Signature, Type}; +use crate::syntax::types; +use syn::{ + spanned::Spanned, Error, FnArg, Ident, Pat, PatIdent, PatType, Receiver, Result, Signature, + Type, +}; /// Describes a single parameter for a function pub struct ParsedFunctionParameter { @@ -16,29 +20,48 @@ pub struct ParsedFunctionParameter { } impl ParsedFunctionParameter { + /// This function parses the list of arguments pub fn parse_all_without_receiver( signature: &Signature, ) -> Result> { - signature - .inputs - .iter() - .map(|input| { - match input { - FnArg::Typed(type_pattern) => { - let parameter = ParsedFunctionParameter::parse(type_pattern)?; - - // Ignore self as a parameter - if parameter.ident == "self" { - return Ok(None); - } - Ok(Some(parameter)) - } + let mut iter = signature.inputs.iter(); + + let missing_self_arg = "First argument must be a supported `self` receiver type!\nUse `&self` or `self: Pin<&mut Self>` instead."; + match iter.next() { + Some(FnArg::Typed(type_pattern)) => { + let parameter = ParsedFunctionParameter::parse(type_pattern)?; + if parameter.ident == "self" && types::is_pin_of_self(¶meter.ty) { + Ok(()) + } else { + Err(Error::new_spanned(type_pattern, missing_self_arg)) + } + } + Some(FnArg::Receiver(Receiver { + reference: Some(_), // `self` needs to be by-ref, by-value is not supported. + mutability: None, // Mutable self references are not supported. + .. + })) => Ok(()), /*Okay, found a &self reference*/ + Some(arg) => Err(Error::new_spanned(arg, missing_self_arg)), + None => Err(Error::new_spanned(signature, "Missing 'self' receiver!")), + }?; + + iter.map(|input| { + match input { + FnArg::Typed(type_pattern) => { + let parameter = ParsedFunctionParameter::parse(type_pattern)?; + // Ignore self as a parameter - FnArg::Receiver(_) => Ok(None), + if parameter.ident == "self" { + return Ok(None); + } + Ok(Some(parameter)) } - }) - .filter_map(|result| result.map_or_else(|e| Some(Err(e)), |v| v.map(Ok))) - .collect::>>() + // Ignore self as a parameter + FnArg::Receiver(_) => Ok(None), + } + }) + .filter_map(|result| result.map_or_else(|e| Some(Err(e)), |v| v.map(Ok))) + .collect::>>() } pub fn parse(type_pattern: &PatType) -> Result { diff --git a/crates/cxx-qt-gen/src/parser/qobject.rs b/crates/cxx-qt-gen/src/parser/qobject.rs index 32876fe38..2f2055646 100644 --- a/crates/cxx-qt-gen/src/parser/qobject.rs +++ b/crates/cxx-qt-gen/src/parser/qobject.rs @@ -349,7 +349,7 @@ pub mod tests { fn invokable_with_return_cxx_type(self: Pin<&mut Self>) -> f64 {} #[qinvokable(cxx_final, cxx_override, cxx_virtual)] - fn invokable_with_specifiers() -> f64 {} + fn invokable_with_specifiers(&self) -> f64 {} fn cpp_context(&self) {} } @@ -395,13 +395,14 @@ pub mod tests { } } }); - assert!(qobject.parse_impl_items(&item.items).is_ok()); + + qobject.parse_impl_items(&item.items).unwrap(); let inherited = &qobject.inherited_methods; assert_eq!(inherited.len(), 3); - assert_eq!(inherited[0].mutable, false); - assert_eq!(inherited[1].mutable, false); - assert_eq!(inherited[2].mutable, true); + assert!(!inherited[0].mutable); + assert!(!inherited[1].mutable); + assert!(inherited[2].mutable); assert_eq!(inherited[0].parameters.len(), 0); assert_eq!(inherited[1].parameters.len(), 1); assert_eq!(inherited[1].parameters[0].ident, "arg"); diff --git a/crates/cxx-qt-gen/src/syntax/mod.rs b/crates/cxx-qt-gen/src/syntax/mod.rs index f72bbc9d3..69dc2e02d 100644 --- a/crates/cxx-qt-gen/src/syntax/mod.rs +++ b/crates/cxx-qt-gen/src/syntax/mod.rs @@ -10,6 +10,7 @@ pub mod implitemmethod; pub mod path; mod qtfile; mod qtitem; +pub mod types; pub use qtfile::{parse_qt_file, CxxQtFile}; pub use qtitem::CxxQtItem; diff --git a/crates/cxx-qt-gen/src/syntax/types.rs b/crates/cxx-qt-gen/src/syntax/types.rs new file mode 100644 index 000000000..1e8c1f35c --- /dev/null +++ b/crates/cxx-qt-gen/src/syntax/types.rs @@ -0,0 +1,33 @@ +// SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +// SPDX-FileContributor: Leon Matthes + +// SPDX-License-Identifier: MIT OR Apache-2.0 + +use crate::syntax::path::*; +use syn::{GenericArgument, PathArguments, Type, TypePath, TypeReference}; + +pub fn is_pin_of_self(ty: &Type) -> bool { + if let Type::Path(type_path) = ty { + if path_compare_str(&type_path.path, &["Pin"]) { + if let PathArguments::AngleBracketed(angles) = + &type_path.path.segments.first().unwrap().arguments + { + if let [GenericArgument::Type(Type::Reference(TypeReference { + elem: type_elem, + .. + }))] = *angles.args.iter().collect::>() + { + if let Type::Path(TypePath { + path: self_path, .. + }) = &**type_elem + { + if path_compare_str(self_path, &["Self"]) { + return true; + } + } + } + } + } + } + false +} diff --git a/crates/cxx-qt-gen/src/writer/cpp/header.rs b/crates/cxx-qt-gen/src/writer/cpp/header.rs index 0f31388f1..510643c69 100644 --- a/crates/cxx-qt-gen/src/writer/cpp/header.rs +++ b/crates/cxx-qt-gen/src/writer/cpp/header.rs @@ -117,6 +117,9 @@ fn qobjects_header(generated: &GeneratedCppBlocks) -> Vec { /// For a given GeneratedCppBlocks write this into a C++ header pub fn write_cpp_header(generated: &GeneratedCppBlocks) -> String { + // Headers included: + // - unique_ptr to the Rust object. + // - used for mutex locking the rust object. formatdoc! {r#" #pragma once diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.cpp b/crates/cxx-qt-gen/test_outputs/inheritance.cpp index 263f99d15..cb8ac534c 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.cpp +++ b/crates/cxx-qt-gen/test_outputs/inheritance.cpp @@ -3,19 +3,20 @@ MyObject::MyObject(QObject* parent) : QAbstractItemModel(parent) , m_rustObj(cxx_qt_my_object::createRs()) - , m_rustObjMutex(std::make_shared()) + , m_rustObjMutex(::std::make_shared<::std::recursive_mutex>()) , m_cxxQtThreadObj( - std::make_shared>(this)) + ::std::make_shared<::rust::cxxqtlib1::CxxQtGuardedPointer>( + this)) { } MyObject::~MyObject() { - const auto guard = std::unique_lock(m_cxxQtThreadObj->mutex); + const auto guard = ::std::unique_lock(m_cxxQtThreadObj->mutex); m_cxxQtThreadObj->ptr = nullptr; } -const MyObjectRust& +MyObjectRust const& MyObject::unsafeRust() const { return *m_rustObj; @@ -27,33 +28,33 @@ MyObject::unsafeRustMut() return *m_rustObj; } -std::unique_ptr +::std::unique_ptr MyObject::qtThread() const { - return std::make_unique(m_cxxQtThreadObj, - m_rustObjMutex); + return ::std::make_unique(m_cxxQtThreadObj, + m_rustObjMutex); } QVariant -MyObject::data(const QModelIndex& _index, qint32 _role) const +MyObject::data(QModelIndex const& _index, ::std::int32_t _role) const { - const std::lock_guard guard(*m_rustObjMutex); - return rust::cxxqtlib1::cxx_qt_convert{}( + const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex); + return ::rust::cxxqtlib1::cxx_qt_convert{}( m_rustObj->dataWrapper(*this, _index, _role)); } bool -MyObject::hasChildren(const QModelIndex& _parent) const +MyObject::hasChildren(QModelIndex const& _parent) const { - const std::lock_guard guard(*m_rustObjMutex); - return rust::cxxqtlib1::cxx_qt_convert{}( + const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex); + return ::rust::cxxqtlib1::cxx_qt_convert{}( m_rustObj->hasChildrenWrapper(*this, _parent)); } namespace cxx_qt_my_object { -std::unique_ptr +::std::unique_ptr newCppObject() { - return std::make_unique(); + return ::std::make_unique(); } } // namespace cxx_qt_my_object diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.h b/crates/cxx-qt-gen/test_outputs/inheritance.h index 9560dbe90..e7ab5adc1 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.h +++ b/crates/cxx-qt-gen/test_outputs/inheritance.h @@ -9,7 +9,7 @@ class CxxQtThread; } class MyObject; -using MyObjectCxxQtThread = rust::cxxqtlib1::CxxQtThread; +using MyObjectCxxQtThread = ::rust::cxxqtlib1::CxxQtThread; #include "cxx-qt-gen/inheritance.cxx.h" @@ -20,27 +20,37 @@ class MyObject : public QAbstractItemModel public: explicit MyObject(QObject* parent = nullptr); ~MyObject(); - const MyObjectRust& unsafeRust() const; + MyObjectRust const& unsafeRust() const; MyObjectRust& unsafeRustMut(); - std::unique_ptr qtThread() const; + ::std::unique_ptr qtThread() const; public: - Q_INVOKABLE QVariant data(const QModelIndex& _index, - qint32 _role) const override; - Q_INVOKABLE bool hasChildren(const QModelIndex& _parent) const override; + Q_INVOKABLE QVariant data(QModelIndex const& _index, + ::std::int32_t _role) const override; + Q_INVOKABLE bool hasChildren(QModelIndex const& _parent) const override; + template + void fetchMore_cxxqt_inherit(Args... args) + { + return QAbstractItemModel::fetchMore(args...); + } + template + bool hasChildren_cxxqt_inherit(Args... args) const + { + return QAbstractItemModel::hasChildren(args...); + } private: - rust::Box m_rustObj; - std::shared_ptr m_rustObjMutex; - std::shared_ptr> + ::rust::Box m_rustObj; + ::std::shared_ptr<::std::recursive_mutex> m_rustObjMutex; + ::std::shared_ptr<::rust::cxxqtlib1::CxxQtGuardedPointer> m_cxxQtThreadObj; }; -static_assert(std::is_base_of::value, +static_assert(::std::is_base_of::value, "MyObject must inherit from QObject"); namespace cxx_qt_my_object { -std::unique_ptr +::std::unique_ptr newCppObject(); } // namespace cxx_qt_my_object diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.rs b/crates/cxx-qt-gen/test_outputs/inheritance.rs index 0494475c5..c197c2e8a 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.rs +++ b/crates/cxx-qt-gen/test_outputs/inheritance.rs @@ -10,6 +10,7 @@ mod inheritance { include ! (< QtCore / QObject >); include!("cxx-qt-lib/convert.h"); include!("cxx-qt-lib/cxxqt_thread.h"); + include!("cxx-qt-lib/std_types.h"); } unsafe extern "C++" { include!("cxx-qt-gen/inheritance.cxxqt.h"); diff --git a/examples/qml_features/CMakeLists.txt b/examples/qml_features/CMakeLists.txt index 7c69864f0..847cb03b0 100644 --- a/examples/qml_features/CMakeLists.txt +++ b/examples/qml_features/CMakeLists.txt @@ -53,7 +53,6 @@ target_link_libraries(${APP_NAME}_lib INTERFACE add_executable(${APP_NAME} cpp/main.cpp - cpp/qabstractlistmodelcxx.h qml/qml.qrc ) @@ -85,7 +84,6 @@ copy_qml_test(types) copy_qml_test(uncreatable) add_executable(${APP_NAME}_test - cpp/qabstractlistmodelcxx.h tests/main.cpp ) target_include_directories(${APP_NAME}_test PRIVATE cpp) diff --git a/examples/qml_features/cpp/qabstractlistmodelcxx.h b/examples/qml_features/cpp/qabstractlistmodelcxx.h deleted file mode 100644 index 592009fb5..000000000 --- a/examples/qml_features/cpp/qabstractlistmodelcxx.h +++ /dev/null @@ -1,40 +0,0 @@ -// clang-format off -// SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company -// clang-format on -// SPDX-FileContributor: Andrew Hayzen -// -// SPDX-License-Identifier: MIT OR Apache-2.0 -#pragma once - -#include - -#include "rust/cxx.h" - -class QAbstractListModelCXX : public QAbstractListModel -{ -public: - explicit QAbstractListModelCXX(QObject* parent = nullptr) - : QAbstractListModel(parent) - { - } - - // Can't define in CXX as they are protected - // so crate public methods that are proxied - void beginInsertRows(int first, int last) - { - QAbstractItemModel::beginInsertRows(QModelIndex(), first, last); - } - - void endInsertRows() { QAbstractItemModel::endInsertRows(); } - - void beginRemoveRows(int first, int last) - { - QAbstractItemModel::beginRemoveRows(QModelIndex(), first, last); - } - - void endRemoveRows() { QAbstractItemModel::endRemoveRows(); } - - void beginResetModel() { QAbstractItemModel::beginResetModel(); } - - void endResetModel() { QAbstractItemModel::endResetModel(); } -}; diff --git a/examples/qml_features/rust/src/custom_base_class.rs b/examples/qml_features/rust/src/custom_base_class.rs index 239342d06..792021943 100644 --- a/examples/qml_features/rust/src/custom_base_class.rs +++ b/examples/qml_features/rust/src/custom_base_class.rs @@ -8,7 +8,7 @@ mod ffi { // ANCHOR: book_base_include unsafe extern "C++" { - include!("qabstractlistmodelcxx.h"); + include!(< QAbstractListModel >); // ANCHOR_END: book_base_include include!("cxx-qt-lib/qhash.h"); @@ -22,33 +22,11 @@ mod ffi { include!("cxx-qt-lib/qvector.h"); type QVector_i32 = cxx_qt_lib::QVector; - - #[cxx_name = "beginInsertRows"] - fn begin_insert_rows(self: Pin<&mut CustomBaseClassQt>, first: i32, last: i32); - #[cxx_name = "endInsertRows"] - fn end_insert_rows(self: Pin<&mut CustomBaseClassQt>); - - #[cxx_name = "beginRemoveRows"] - fn begin_remove_rows(self: Pin<&mut CustomBaseClassQt>, first: i32, last: i32); - #[cxx_name = "endRemoveRows"] - fn end_remove_rows(self: Pin<&mut CustomBaseClassQt>); - - #[cxx_name = "beginResetModel"] - fn begin_reset_model(self: Pin<&mut CustomBaseClassQt>); - #[cxx_name = "endResetModel"] - fn end_reset_model(self: Pin<&mut CustomBaseClassQt>); - - fn index( - self: &CustomBaseClassQt, - row: i32, - column: i32, - parent: &QModelIndex, - ) -> QModelIndex; } // ANCHOR: book_qobject_base #[cxx_qt::qobject( - base = "QAbstractListModelCXX", + base = "QAbstractListModel", qml_uri = "com.kdab.cxx_qt.demo", qml_version = "1.0" )] @@ -99,7 +77,8 @@ mod ffi { fn add_cpp_context(mut self: Pin<&mut Self>) { let count = self.vector().len(); - self.as_mut().begin_insert_rows(count as i32, count as i32); + self.as_mut() + .begin_insert_rows(&QModelIndex::default(), count as i32, count as i32); let id = *self.id(); self.as_mut().set_id(id + 1); self.as_mut().vector_mut().push((id, (id as f64) / 3.0)); @@ -137,7 +116,8 @@ mod ffi { return; } - self.as_mut().begin_remove_rows(index, index); + self.as_mut() + .begin_remove_rows(&QModelIndex::default(), index, index); self.as_mut().vector_mut().remove(index as usize); self.as_mut().end_remove_rows(); } @@ -145,6 +125,17 @@ mod ffi { // QAbstractListModel implementation impl qobject::CustomBaseClass { + cxx_qt::inherit! { + fn begin_insert_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); + fn end_insert_rows(self: Pin<&mut Self>); + + fn begin_remove_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); + fn end_remove_rows(self: Pin<&mut Self>); + + fn begin_reset_model(self: Pin<&mut Self>); + fn end_reset_model(self: Pin<&mut Self>); + } + #[qinvokable(cxx_override)] fn data(&self, index: &QModelIndex, role: i32) -> QVariant { if let Some((id, value)) = self.rust().vector.get(index.row() as usize) { From e98af8b3f9da014b124b874efb5f52837961a873 Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Wed, 25 Jan 2023 14:02:33 +0100 Subject: [PATCH 05/14] Allow cxx_qt::inherit on qobjects without base. Base class defaults to `QObject`. --- crates/cxx-qt-gen/src/generator/cpp/inherit.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs index 5f19f1ff1..788f23c8a 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs @@ -9,7 +9,7 @@ use crate::{ parser::{cxxqtdata::ParsedCxxMappings, qobject::ParsedQObject}, }; -use syn::{Error, Result, ReturnType}; +use syn::{Result, ReturnType}; use super::types::CppType; @@ -28,13 +28,7 @@ pub fn generate( "void".to_owned() }; - let missing_base_class_error = || { - Error::new_spanned(&method.method, format!("C++ code generation failed!\nClass {} has inherited methods, but no base class!", qobject.qobject_struct.ident)) - }; - let base_class = qobject - .base_class - .clone() - .ok_or_else(missing_base_class_error)?; + let base_class = qobject.base_class.as_deref().unwrap_or("QObject"); result.methods.push(CppFragment::Header(formatdoc! { r#" From 7a1bec630ce6c3390222d021ed650f269610e1f6 Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Wed, 1 Feb 2023 12:59:13 +0100 Subject: [PATCH 06/14] Add documentation for cxx_qt::inherit! Also incorporates requested changes. --- CHANGELOG.md | 1 + book/src/SUMMARY.md | 1 + book/src/concepts/index.md | 2 + book/src/concepts/inheritance.md | 76 +++++++++++++++++++ .../cxx-qt-gen/src/generator/cpp/inherit.rs | 4 +- crates/cxx-qt-gen/src/generator/naming/mod.rs | 23 ++++++ crates/cxx-qt-gen/src/parser/inherit.rs | 3 +- crates/cxx-qt-gen/src/parser/parameter.rs | 5 +- crates/cxx-qt-gen/src/parser/qobject.rs | 5 +- crates/cxx-qt-gen/src/syntax/types.rs | 38 +++++++++- .../test_inputs/inheritance.rs.license | 2 +- .../test_outputs/inheritance.cpp.license | 2 +- .../test_outputs/inheritance.h.license | 2 +- .../test_outputs/inheritance.rs.license | 2 +- crates/cxx-qt/src/lib.rs | 30 ++++++++ .../rust/src/custom_base_class.rs | 21 +++++ 16 files changed, 206 insertions(+), 11 deletions(-) create mode 100644 book/src/concepts/inheritance.md diff --git a/CHANGELOG.md b/CHANGELOG.md index bb1246633..7381dcba8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Added +- Support for inheriting methods from the superclass into Rust using `cxx_qt::inherit!`. - Register QML types at build time: `#[cxxqt::qobject(qml_uri = "foo.bar", qml_version = "1.0")]` - Register QRC resources at build time in Cargo builds (don't need to call initialization function from Rust `main` function) - Support for container types: `QSet`, `QHash`, `QList`, `QMap`, `QVector` diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index c565d9d15..6c6ac5937 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -31,3 +31,4 @@ SPDX-License-Identifier: MIT OR Apache-2.0 - [Build Systems](./concepts/build_systems.md) - [Threading](./concepts/threading.md) - [Nested Objects](./concepts/nested_objects.md) + - [Inheritance & Overriding](./concepts/inheritance.md) diff --git a/book/src/concepts/index.md b/book/src/concepts/index.md index 0e2b3108d..0281f58ea 100644 --- a/book/src/concepts/index.md +++ b/book/src/concepts/index.md @@ -1,6 +1,7 @@ @@ -21,3 +22,4 @@ SPDX-License-Identifier: MIT OR Apache-2.0 * [Threading concept and safety](./threading.md) * [Nesting Rust objects](./nested_objects.md) + * [Inheriting QObjects and overriding methods](./inheritance.md) diff --git a/book/src/concepts/inheritance.md b/book/src/concepts/inheritance.md new file mode 100644 index 000000000..3e2b85e56 --- /dev/null +++ b/book/src/concepts/inheritance.md @@ -0,0 +1,76 @@ + + +# Inheritance + +Some Qt APIs require you to override certain methods from an abstract base class. +Specifically [QAbstractItemModel](https://doc.qt.io/qt-6/qabstractitemmodel.html). + +To support creating such subclasses directly from within Rust, CXX-Qt provides you with multiple helpers. + +## Accessing base class methods +To access the methods of a base class in Rust, use the `cxx_qt::inherit!` macro. +It can be placed within a `impl qobject::T` block in a `#[cxx_qt::bridge]`. + +```rust,ignore +{{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_qalm}} + + impl qobject::CustomBaseClass { +{{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_qalm_impl}} + +{{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_clear}} + } +``` +[Full example](https://github.com/KDAB/cxx-qt/blob/main/examples/qml_features/rust/src/custom_base_class.rs) + +This code implements a QAbstractListModel subclass. +For this, access to the `beginResetModel` and related methods is required. +See [the Qt docs](https://doc.qt.io/qt-6/qabstractlistmodel.html) for more details on the specific subclassing requirements. +These methods are then made accessible by using `cxx_qt::inherit!`. + +Methods can be declared inside `cxx_qt::inherit!` in the same format as in plain CXX, with the same restrictions regarding which types can be used. +Additionally, the `self` type must be either `self: Pin<&mut Self>` or `&self`. + +The declared methods will be case-converted as in other CXX-Qt APIs. +To explicitly declare the C++ method name, use the `#[cxx_name="myFunctionName"]` attribute. + +## Overriding base class methods + +CXX-Qt allows invokables to be generated with the C++ modifiers necessary to implement inheritance. +This way methods can be overridden, declared as `virtual` or `final`. + +| C++ keyword | CXX-Qt attribute | +|-------------|-------------------------------| +| `override` | `qinvokable(cxx_override)` | +| `virtual` | `#[qinvokable(cxx_override)]` | +| `final` | `#[qinvokable(cxx_final)]` | + +The below example overrides the [`data`](https://doc.qt.io/qt-6/qabstractitemmodel.html#data) method inherited from the QAbstractListModel. +```rust,ignore +{{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_qalm}} + + impl qobject::CustomBaseClass { + +{{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_data}} + } +``` +[Full example](https://github.com/KDAB/cxx-qt/blob/main/examples/qml_features/rust/src/custom_base_class.rs) + +Note that if a method is overridden using `cxx_override` the base class version of the method can be accessed by using `cxx_qt::inherit!` in combination with the `#[cxx_name]` attribute. +In this case the base class version of the function must get a different name, as Rust can't natively express the concept of calling a base class method. + +Example: +```rust,ignore +{{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_qalm}} + + impl qobject::CustomBaseClass { +{{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_qalm_impl}} + +{{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_can_fetch_more}} + } +``` +[Full example](https://github.com/KDAB/cxx-qt/blob/main/examples/qml_features/rust/src/custom_base_class.rs) diff --git a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs index 788f23c8a..a5f5c4c83 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs @@ -1,9 +1,9 @@ -// SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +// SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company // SPDX-FileContributor: Leon Matthes +// SPDX-License-Identifier: MIT OR Apache-2.0 use indoc::formatdoc; -// SPDX-License-Identifier: MIT OR Apache-2.0 use crate::{ generator::cpp::{fragment::CppFragment, qobject::GeneratedCppQObjectBlocks}, parser::{cxxqtdata::ParsedCxxMappings, qobject::ParsedQObject}, diff --git a/crates/cxx-qt-gen/src/generator/naming/mod.rs b/crates/cxx-qt-gen/src/generator/naming/mod.rs index c245240f3..08abd43f8 100644 --- a/crates/cxx-qt-gen/src/generator/naming/mod.rs +++ b/crates/cxx-qt-gen/src/generator/naming/mod.rs @@ -21,6 +21,8 @@ pub struct CombinedIdent { } impl CombinedIdent { + /// Generate a CombinedIdent from a rust function name. + /// C++ will use the CamelCase version of the function name. pub fn from_rust_function(ident: Ident) -> Self { Self { cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), @@ -28,3 +30,24 @@ impl CombinedIdent { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_from_rust_function_camel_case_conversion() { + let ident = format_ident!("test_function"); + let combined = CombinedIdent::from_rust_function(ident.clone()); + assert_eq!(combined.cpp, format_ident!("testFunction")); + assert_eq!(combined.rust, ident); + } + + #[test] + fn test_from_rust_function_single_word() { + let ident = format_ident!("test"); + let combined = CombinedIdent::from_rust_function(ident.clone()); + assert_eq!(combined.cpp, ident); + assert_eq!(combined.rust, ident); + } +} diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs index 6baf1628f..5142cc51f 100644 --- a/crates/cxx-qt-gen/src/parser/inherit.rs +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +// SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company // SPDX-FileContributor: Leon Matthes // // SPDX-License-Identifier: MIT OR Apache-2.0 @@ -40,6 +40,7 @@ pub struct ParsedInheritedMethod { pub parameters: Vec, /// the name of the function in Rust, as well as C++ pub ident: CombinedIdent, + /// the name of the wrapper function in C++ pub wrapper_ident: Ident, } diff --git a/crates/cxx-qt-gen/src/parser/parameter.rs b/crates/cxx-qt-gen/src/parser/parameter.rs index c38419896..afc57c4b1 100644 --- a/crates/cxx-qt-gen/src/parser/parameter.rs +++ b/crates/cxx-qt-gen/src/parser/parameter.rs @@ -38,9 +38,10 @@ impl ParsedFunctionParameter { } Some(FnArg::Receiver(Receiver { reference: Some(_), // `self` needs to be by-ref, by-value is not supported. - mutability: None, // Mutable self references are not supported. + mutability: None, // Mutable `&mut self` references are not supported. + // Use `Pin<&mut Self>` instead. .. - })) => Ok(()), /*Okay, found a &self reference*/ + })) => Ok(()), // Okay, found a &self reference Some(arg) => Err(Error::new_spanned(arg, missing_self_arg)), None => Err(Error::new_spanned(signature, "Missing 'self' receiver!")), }?; diff --git a/crates/cxx-qt-gen/src/parser/qobject.rs b/crates/cxx-qt-gen/src/parser/qobject.rs index 2f2055646..02414587c 100644 --- a/crates/cxx-qt-gen/src/parser/qobject.rs +++ b/crates/cxx-qt-gen/src/parser/qobject.rs @@ -216,7 +216,10 @@ impl ParsedQObject { } } _ => { - return Err(Error::new(item.span(), "Only methods are supported.")); + return Err(Error::new( + item.span(), + "Only methods or cxx_qt::inherit is supported.", + )); } } } diff --git a/crates/cxx-qt-gen/src/syntax/types.rs b/crates/cxx-qt-gen/src/syntax/types.rs index 1e8c1f35c..89cae6502 100644 --- a/crates/cxx-qt-gen/src/syntax/types.rs +++ b/crates/cxx-qt-gen/src/syntax/types.rs @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +// SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company // SPDX-FileContributor: Leon Matthes // SPDX-License-Identifier: MIT OR Apache-2.0 @@ -6,6 +6,8 @@ use crate::syntax::path::*; use syn::{GenericArgument, PathArguments, Type, TypePath, TypeReference}; +/// Checks if the given type is a `Pin<&Self>` or `Pin<&mut Self>`. +/// `Pin>` is currently not supported. pub fn is_pin_of_self(ty: &Type) -> bool { if let Type::Path(type_path) = ty { if path_compare_str(&type_path.path, &["Pin"]) { @@ -31,3 +33,37 @@ pub fn is_pin_of_self(ty: &Type) -> bool { } false } + +#[cfg(test)] +mod tests { + + use crate::tests::tokens_to_syn; + use proc_macro2::TokenStream; + use quote::quote; + use syn::Type; + + fn assert_pin_of_self(tokens: TokenStream) { + let pin: Type = tokens_to_syn(tokens); + assert!(super::is_pin_of_self(&pin)); + } + + fn assert_not_pin_of_self(tokens: TokenStream) { + let pin: Type = tokens_to_syn(tokens); + assert!(!super::is_pin_of_self(&pin)); + } + + #[test] + fn test_is_pin_of_self() { + assert_pin_of_self(quote! { Pin<&Self> }); + assert_pin_of_self(quote! { Pin<&mut Self> }); + + // `Pin>` is currently not supported because it can't be used with + // Opaque C++ types. Use UniquePtr instead. + assert_not_pin_of_self(quote! { Pin> }); + assert_not_pin_of_self(quote! { Pin<&Self, Foo> }); + assert_not_pin_of_self(quote! { Pin }); + assert_not_pin_of_self(quote! { Pin }); + assert_not_pin_of_self(quote! { Pin<&Foo> }); + assert_not_pin_of_self(quote! { Pin<&mut Foo> }); + } +} diff --git a/crates/cxx-qt-gen/test_inputs/inheritance.rs.license b/crates/cxx-qt-gen/test_inputs/inheritance.rs.license index 61425b8fc..72648cfc1 100644 --- a/crates/cxx-qt-gen/test_inputs/inheritance.rs.license +++ b/crates/cxx-qt-gen/test_inputs/inheritance.rs.license @@ -1,4 +1,4 @@ -SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company SPDX-FileContributor: Leon Matthes SPDX-License-Identifier: MIT OR Apache-2.0 diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.cpp.license b/crates/cxx-qt-gen/test_outputs/inheritance.cpp.license index 61425b8fc..72648cfc1 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.cpp.license +++ b/crates/cxx-qt-gen/test_outputs/inheritance.cpp.license @@ -1,4 +1,4 @@ -SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company SPDX-FileContributor: Leon Matthes SPDX-License-Identifier: MIT OR Apache-2.0 diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.h.license b/crates/cxx-qt-gen/test_outputs/inheritance.h.license index 61425b8fc..72648cfc1 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.h.license +++ b/crates/cxx-qt-gen/test_outputs/inheritance.h.license @@ -1,4 +1,4 @@ -SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company SPDX-FileContributor: Leon Matthes SPDX-License-Identifier: MIT OR Apache-2.0 diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.rs.license b/crates/cxx-qt-gen/test_outputs/inheritance.rs.license index 61425b8fc..72648cfc1 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.rs.license +++ b/crates/cxx-qt-gen/test_outputs/inheritance.rs.license @@ -1,4 +1,4 @@ -SPDX-FileCopyrightText: 2022 Klarälvdalens Datakonsult AB, a KDAB Group company +SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company SPDX-FileContributor: Leon Matthes SPDX-License-Identifier: MIT OR Apache-2.0 diff --git a/crates/cxx-qt/src/lib.rs b/crates/cxx-qt/src/lib.rs index 34f2ff544..5577f6e14 100644 --- a/crates/cxx-qt/src/lib.rs +++ b/crates/cxx-qt/src/lib.rs @@ -106,6 +106,36 @@ pub fn qobject(_args: TokenStream, _input: TokenStream) -> TokenStream { unreachable!("cxx_qt::qobject should not be used as a macro by itself. Instead it should be used within a cxx_qt::bridge definition") } +/// A macro which allows you to access base class methods from within Rust. +/// +/// It should not be used by itself and instead should be used inside a cxx_qt::bridge definition. +/// Furthermore, the macro must be placed within the `impl` block of a `qobject::T`. +/// See [the book page](https://kdab.github.io/cxx-qt/book/concepts/inheritance.html) for more +/// details. +/// +/// # Example +/// ``` ignore +/// #[cxx_qt::bridge] +/// mod my_object { +/// #[cxx_qt::qobject(base="QAbstractListModel")] +/// #[derive(Default)] +/// struct MyObject; +/// +/// impl qobject::MyObject { +/// cxx_qt::inherit! { +/// fn begin_insert_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); +/// fn end_insert_rows(self: Pin<&mut Self>); +/// } +/// +/// // ... +/// } +/// } +/// ``` +#[proc_macro_attribute] +pub fn inherit(_args: TokenStream, _input: TokenStream) -> TokenStream { + unreachable!("cxx_qt::inherit should not be used as a macro by itself. Instead it should be used within a cxx_qt::bridge definition") +} + // Take the module and C++ namespace and generate the rust code fn extract_and_generate(module: ItemMod) -> TokenStream { let parser = Parser::from(module).unwrap(); diff --git a/examples/qml_features/rust/src/custom_base_class.rs b/examples/qml_features/rust/src/custom_base_class.rs index 792021943..431b29271 100644 --- a/examples/qml_features/rust/src/custom_base_class.rs +++ b/examples/qml_features/rust/src/custom_base_class.rs @@ -24,6 +24,7 @@ mod ffi { type QVector_i32 = cxx_qt_lib::QVector; } + // ANCHOR: book_inherit_qalm // ANCHOR: book_qobject_base #[cxx_qt::qobject( base = "QAbstractListModel", @@ -36,6 +37,7 @@ mod ffi { id: u32, vector: Vec<(u32, f64)>, } + // ANCHOR_END: book_inherit_qalm // ANCHOR: book_qsignals_inherit #[cxx_qt::qsignals(CustomBaseClass)] @@ -85,6 +87,7 @@ mod ffi { self.as_mut().end_insert_rows(); } + // ANCHOR: book_inherit_clear #[qinvokable] pub fn clear(mut self: Pin<&mut Self>) { self.as_mut().begin_reset_model(); @@ -92,6 +95,7 @@ mod ffi { self.as_mut().vector_mut().clear(); self.as_mut().end_reset_model(); } + // ANCHOR_END: book_inherit_clear #[qinvokable] pub fn multiply(mut self: Pin<&mut Self>, index: i32, factor: f64) { @@ -125,6 +129,7 @@ mod ffi { // QAbstractListModel implementation impl qobject::CustomBaseClass { + // ANCHOR: book_inherit_qalm_impl cxx_qt::inherit! { fn begin_insert_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); fn end_insert_rows(self: Pin<&mut Self>); @@ -134,8 +139,15 @@ mod ffi { fn begin_reset_model(self: Pin<&mut Self>); fn end_reset_model(self: Pin<&mut Self>); + + #[cxx_name="canFetchMore"] + fn base_can_fetch_more(&self, parent: &QModelIndex) -> bool; + + fn index(&self, row: i32, column: i32, parent: &QModelIndex) -> QModelIndex; } + // ANCHOR_END: book_inherit_qalm_impl + // ANCHOR: book_inherit_data #[qinvokable(cxx_override)] fn data(&self, index: &QModelIndex, role: i32) -> QVariant { if let Some((id, value)) = self.rust().vector.get(index.row() as usize) { @@ -148,6 +160,15 @@ mod ffi { QVariant::default() } + // ANCHOR_END: book_inherit_data + + // ANCHOR: book_inherit_can_fetch_more + // Example of overriding a C++ virtual method and calling the base class implementation. + #[qinvokable(cxx_override)] + pub fn can_fetch_more(&self, parent: &QModelIndex) -> bool { + self.base_can_fetch_more(parent) + } + // ANCHOR_END: book_inherit_can_fetch_more #[qinvokable(cxx_override)] pub fn role_names(&self) -> QHash_i32_QByteArray { From 17c040fbeef363dc0f9b3ebe6db848593217c6ab Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Wed, 1 Feb 2023 16:31:02 +0100 Subject: [PATCH 07/14] Require unsafe for inherited methods --- book/src/concepts/inheritance.md | 6 +- .../cxx-qt-gen/src/generator/rust/qobject.rs | 10 ++- crates/cxx-qt-gen/src/parser/inherit.rs | 64 +++++++++++++++++-- crates/cxx-qt-gen/src/parser/qobject.rs | 25 ++++++-- crates/cxx-qt-gen/test_inputs/inheritance.rs | 10 ++- crates/cxx-qt-gen/test_outputs/inheritance.h | 8 +-- crates/cxx-qt-gen/test_outputs/inheritance.rs | 8 +-- crates/cxx-qt/src/lib.rs | 10 ++- .../rust/src/custom_base_class.rs | 60 ++++++++++------- 9 files changed, 146 insertions(+), 55 deletions(-) diff --git a/book/src/concepts/inheritance.md b/book/src/concepts/inheritance.md index 3e2b85e56..7e1302110 100644 --- a/book/src/concepts/inheritance.md +++ b/book/src/concepts/inheritance.md @@ -32,7 +32,7 @@ For this, access to the `beginResetModel` and related methods is required. See [the Qt docs](https://doc.qt.io/qt-6/qabstractlistmodel.html) for more details on the specific subclassing requirements. These methods are then made accessible by using `cxx_qt::inherit!`. -Methods can be declared inside `cxx_qt::inherit!` in the same format as in plain CXX, with the same restrictions regarding which types can be used. +Methods can be declared inside `cxx_qt::inherit!` in `extern "C++"` blocks similar to C++, with the same restrictions regarding which types can be used. Additionally, the `self` type must be either `self: Pin<&mut Self>` or `&self`. The declared methods will be case-converted as in other CXX-Qt APIs. @@ -45,8 +45,8 @@ This way methods can be overridden, declared as `virtual` or `final`. | C++ keyword | CXX-Qt attribute | |-------------|-------------------------------| -| `override` | `qinvokable(cxx_override)` | -| `virtual` | `#[qinvokable(cxx_override)]` | +| `override` | `#[qinvokable(cxx_override)]` | +| `virtual` | `#[qinvokable(cxx_virtual)]` | | `final` | `#[qinvokable(cxx_final)]` | The below example overrides the [`data`](https://doc.qt.io/qt-6/qabstractitemmodel.html#data) method inherited from the QAbstractListModel. diff --git a/crates/cxx-qt-gen/src/generator/rust/qobject.rs b/crates/cxx-qt-gen/src/generator/rust/qobject.rs index 0661fcc8e..a05bd03da 100644 --- a/crates/cxx-qt-gen/src/generator/rust/qobject.rs +++ b/crates/cxx-qt-gen/src/generator/rust/qobject.rs @@ -175,10 +175,16 @@ fn generate_inherited_methods( quote! { self: &#qobject_name } }; let return_type = &method.method.sig.output; + + let mut unsafe_block = None; + let mut unsafe_call = Some(quote! { unsafe }); + if method.safe { + std::mem::swap(&mut unsafe_call, &mut unsafe_block); + } syn::parse2(quote! { - unsafe extern "C++" { + #unsafe_block extern "C++" { #[cxx_name=#cxx_name_string] - fn #ident(#self_param, #(#parameters),*) #return_type; + #unsafe_call fn #ident(#self_param, #(#parameters),*) #return_type; } }) }) diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs index 5142cc51f..e1b4de2d3 100644 --- a/crates/cxx-qt-gen/src/parser/inherit.rs +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -12,21 +12,59 @@ use quote::format_ident; use syn::{ parse::{Parse, ParseStream}, spanned::Spanned, - Error, ForeignItemFn, Ident, LitStr, Result, + Error, ForeignItem, ForeignItemFn, Ident, ItemForeignMod, LitStr, Result, Token, }; /// This type is used when parsing the `cxx_qt::inherit!` macro contents into raw ForeignItemFn items pub struct InheritMethods { - pub base_functions: Vec, + pub base_safe_functions: Vec, + pub base_unsafe_functions: Vec, } impl Parse for InheritMethods { fn parse(input: ParseStream) -> Result { - let mut base_functions = Vec::new(); + let mut base_safe_functions = Vec::new(); + let mut base_unsafe_functions = Vec::new(); + while !input.is_empty() { - base_functions.push(input.parse::()?); + // base_safe_functions.push(input.parse::()?); + // This looks somewhat counter-intuitive, but if we add `unsafe` + // to the `extern "C++"` block, the contained functions will be safe to call. + let is_safe = input.peek(Token![unsafe]); + if is_safe { + input.parse::()?; + } + + let extern_block = input.parse::()?; + if extern_block.abi.name != Some(LitStr::new("C++", extern_block.abi.span())) { + return Err(Error::new( + extern_block.abi.span(), + "Inherit blocks must be marked with `extern \"C++\"`", + )); + } + + for item in extern_block.items { + match item { + ForeignItem::Fn(function) => { + if is_safe { + base_safe_functions.push(function); + } else { + base_unsafe_functions.push(function); + } + } + _ => { + return Err(Error::new( + item.span(), + "Only functions are allowed in cxx_qt::inherit! blocks", + )) + } + } + } } - Ok(InheritMethods { base_functions }) + Ok(InheritMethods { + base_safe_functions, + base_unsafe_functions, + }) } } @@ -36,6 +74,8 @@ pub struct ParsedInheritedMethod { pub method: ForeignItemFn, /// whether the inherited method is marked as mutable pub mutable: bool, + /// Whether the method is safe to call. + pub safe: bool, /// the parameters of the method, without the `self` argument pub parameters: Vec, /// the name of the function in Rust, as well as C++ @@ -45,7 +85,17 @@ pub struct ParsedInheritedMethod { } impl ParsedInheritedMethod { - pub fn parse(method: ForeignItemFn) -> Result { + pub fn parse_unsafe(method: ForeignItemFn) -> Result { + if 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!", + )); + } + Self::parse_safe(method) + } + + pub fn parse_safe(method: ForeignItemFn) -> Result { let mutable = is_method_mutable(&method.sig); let parameters = ParsedFunctionParameter::parse_all_without_receiver(&method.sig)?; @@ -64,6 +114,7 @@ impl ParsedInheritedMethod { ident.cpp = format_ident!("{}", name.value()); } let wrapper_ident = format_ident!("{}_cxxqt_inherit", &ident.cpp); + let safe = method.sig.unsafety.is_none(); Ok(Self { method, @@ -71,6 +122,7 @@ impl ParsedInheritedMethod { parameters, ident, wrapper_ident, + safe, }) } } diff --git a/crates/cxx-qt-gen/src/parser/qobject.rs b/crates/cxx-qt-gen/src/parser/qobject.rs index 02414587c..1bbcdd1c5 100644 --- a/crates/cxx-qt-gen/src/parser/qobject.rs +++ b/crates/cxx-qt-gen/src/parser/qobject.rs @@ -192,9 +192,14 @@ impl ParsedQObject { fn parse_impl_inherit(&mut self, mac: &ImplItemMacro) -> Result<()> { let inherited: InheritMethods = mac.mac.parse_body()?; - for method in inherited.base_functions.into_iter() { + for method in inherited.base_safe_functions.into_iter() { self.inherited_methods - .push(ParsedInheritedMethod::parse(method)?); + .push(ParsedInheritedMethod::parse_safe(method)?); + } + + for method in inherited.base_unsafe_functions.into_iter() { + self.inherited_methods + .push(ParsedInheritedMethod::parse_unsafe(method)?); } Ok(()) @@ -389,12 +394,15 @@ pub mod tests { let item: ItemImpl = tokens_to_syn(quote! { impl T { cxx_qt::inherit! { - fn test(&self); + unsafe extern "C++" { + fn test(&self); - fn with_args(&self, arg: i32); - - #[cxx_name="withRename"] - fn with_rename(self: Pin<&mut Self>, arg: i32); + fn with_args(&self, arg: i32); + } + extern "C++" { + #[cxx_name="withRename"] + unsafe fn with_rename(self: Pin<&mut Self>, arg: i32); + } } } }); @@ -406,6 +414,9 @@ pub mod tests { assert!(!inherited[0].mutable); assert!(!inherited[1].mutable); assert!(inherited[2].mutable); + assert!(inherited[0].safe); + assert!(inherited[1].safe); + assert!(!inherited[2].safe); assert_eq!(inherited[0].parameters.len(), 0); assert_eq!(inherited[1].parameters.len(), 1); assert_eq!(inherited[1].parameters[0].ident, "arg"); diff --git a/crates/cxx-qt-gen/test_inputs/inheritance.rs b/crates/cxx-qt-gen/test_inputs/inheritance.rs index 44e93240d..fe10e1c13 100644 --- a/crates/cxx-qt-gen/test_inputs/inheritance.rs +++ b/crates/cxx-qt-gen/test_inputs/inheritance.rs @@ -20,10 +20,14 @@ mod inheritance { } cxx_qt::inherit! { - fn fetch_more(self: Pin<&mut Self>, index: &QModelIndex); + extern "C++" { + unsafe fn fetch_more(self: Pin<&mut Self>, index: &QModelIndex); + } - #[cxx_name="hasChildren"] - fn has_children_super(&self, parent: &QModelIndex) -> bool; + unsafe extern "C++" { + #[cxx_name="hasChildren"] + fn has_children_super(&self, parent: &QModelIndex) -> bool; + } } #[qinvokable(cxx_override)] diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.h b/crates/cxx-qt-gen/test_outputs/inheritance.h index e7ab5adc1..8462a26d6 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.h +++ b/crates/cxx-qt-gen/test_outputs/inheritance.h @@ -29,14 +29,14 @@ class MyObject : public QAbstractItemModel ::std::int32_t _role) const override; Q_INVOKABLE bool hasChildren(QModelIndex const& _parent) const override; template - void fetchMore_cxxqt_inherit(Args... args) + bool hasChildren_cxxqt_inherit(Args... args) const { - return QAbstractItemModel::fetchMore(args...); + return QAbstractItemModel::hasChildren(args...); } template - bool hasChildren_cxxqt_inherit(Args... args) const + void fetchMore_cxxqt_inherit(Args... args) { - return QAbstractItemModel::hasChildren(args...); + return QAbstractItemModel::fetchMore(args...); } private: diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.rs b/crates/cxx-qt-gen/test_outputs/inheritance.rs index c197c2e8a..92bac9372 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.rs +++ b/crates/cxx-qt-gen/test_outputs/inheritance.rs @@ -37,14 +37,14 @@ mod inheritance { #[cxx_name = "hasChildrenWrapper"] fn has_children_wrapper(self: &MyObject, cpp: &MyObjectQt, _parent: &QModelIndex) -> bool; } - unsafe extern "C++" { - #[cxx_name = "fetchMore_cxxqt_inherit"] - fn fetch_more(self: Pin<&mut MyObjectQt>, index: &QModelIndex); - } unsafe extern "C++" { #[cxx_name = "hasChildren_cxxqt_inherit"] fn has_children_super(self: &MyObjectQt, parent: &QModelIndex) -> bool; } + extern "C++" { + #[cxx_name = "fetchMore_cxxqt_inherit"] + unsafe fn fetch_more(self: Pin<&mut MyObjectQt>, index: &QModelIndex); + } unsafe extern "C++" { type MyObjectCxxQtThread; #[cxx_name = "unsafeRust"] diff --git a/crates/cxx-qt/src/lib.rs b/crates/cxx-qt/src/lib.rs index 5577f6e14..f7d1f2a0d 100644 --- a/crates/cxx-qt/src/lib.rs +++ b/crates/cxx-qt/src/lib.rs @@ -123,8 +123,14 @@ pub fn qobject(_args: TokenStream, _input: TokenStream) -> TokenStream { /// /// impl qobject::MyObject { /// cxx_qt::inherit! { -/// fn begin_insert_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); -/// fn end_insert_rows(self: Pin<&mut Self>); +/// extern "C++" { +/// // Unsafe to call +/// unsafe fn begin_insert_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); +/// } +/// unsafe extern "C++" { +/// // Safe to call - you are responsible to ensure this is true. +/// fn end_insert_rows(self: Pin<&mut Self>); +/// } /// } /// /// // ... diff --git a/examples/qml_features/rust/src/custom_base_class.rs b/examples/qml_features/rust/src/custom_base_class.rs index 431b29271..fbde18c79 100644 --- a/examples/qml_features/rust/src/custom_base_class.rs +++ b/examples/qml_features/rust/src/custom_base_class.rs @@ -79,21 +79,28 @@ mod ffi { fn add_cpp_context(mut self: Pin<&mut Self>) { let count = self.vector().len(); - self.as_mut() - .begin_insert_rows(&QModelIndex::default(), count as i32, count as i32); - let id = *self.id(); - self.as_mut().set_id(id + 1); - self.as_mut().vector_mut().push((id, (id as f64) / 3.0)); - self.as_mut().end_insert_rows(); + unsafe { + self.as_mut().begin_insert_rows( + &QModelIndex::default(), + count as i32, + count as i32, + ); + let id = *self.id(); + self.as_mut().set_id(id + 1); + self.as_mut().vector_mut().push((id, (id as f64) / 3.0)); + self.as_mut().end_insert_rows(); + } } // ANCHOR: book_inherit_clear #[qinvokable] pub fn clear(mut self: Pin<&mut Self>) { - self.as_mut().begin_reset_model(); - self.as_mut().set_id(0); - self.as_mut().vector_mut().clear(); - self.as_mut().end_reset_model(); + unsafe { + self.as_mut().begin_reset_model(); + self.as_mut().set_id(0); + self.as_mut().vector_mut().clear(); + self.as_mut().end_reset_model(); + } } // ANCHOR_END: book_inherit_clear @@ -120,10 +127,12 @@ mod ffi { return; } - self.as_mut() - .begin_remove_rows(&QModelIndex::default(), index, index); - self.as_mut().vector_mut().remove(index as usize); - self.as_mut().end_remove_rows(); + unsafe { + self.as_mut() + .begin_remove_rows(&QModelIndex::default(), index, index); + self.as_mut().vector_mut().remove(index as usize); + self.as_mut().end_remove_rows(); + } } } @@ -131,19 +140,22 @@ mod ffi { impl qobject::CustomBaseClass { // ANCHOR: book_inherit_qalm_impl cxx_qt::inherit! { - fn begin_insert_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); - fn end_insert_rows(self: Pin<&mut Self>); + extern "C++" { + unsafe fn begin_insert_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); + unsafe fn end_insert_rows(self: Pin<&mut Self>); - fn begin_remove_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); - fn end_remove_rows(self: Pin<&mut Self>); + unsafe fn begin_remove_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); + unsafe fn end_remove_rows(self: Pin<&mut Self>); - fn begin_reset_model(self: Pin<&mut Self>); - fn end_reset_model(self: Pin<&mut Self>); - - #[cxx_name="canFetchMore"] - fn base_can_fetch_more(&self, parent: &QModelIndex) -> bool; + unsafe fn begin_reset_model(self: Pin<&mut Self>); + unsafe fn end_reset_model(self: Pin<&mut Self>); + } + unsafe extern "C++" { + #[cxx_name="canFetchMore"] + fn base_can_fetch_more(&self, parent: &QModelIndex) -> bool; - fn index(&self, row: i32, column: i32, parent: &QModelIndex) -> QModelIndex; + fn index(&self, row: i32, column: i32, parent: &QModelIndex) -> QModelIndex; + } } // ANCHOR_END: book_inherit_qalm_impl From 3d794a9e8d4ccbaa029d85a6804f7107c8369427 Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Thu, 16 Feb 2023 16:13:28 +0100 Subject: [PATCH 08/14] Use #[cxx_qt::inherit] syntax instead of cxx_qt::inherit! --- CHANGELOG.md | 2 +- book/src/concepts/inheritance.md | 20 +-- crates/cxx-qt-gen/src/parser/cxxqtdata.rs | 109 +++++++++++++++- crates/cxx-qt-gen/src/parser/inherit.rs | 116 +++++++++++------- crates/cxx-qt-gen/src/parser/parameter.rs | 52 +++++--- crates/cxx-qt-gen/src/parser/qobject.rs | 78 ++---------- crates/cxx-qt-gen/src/syntax/foreignmod.rs | 85 ++++++++++++- crates/cxx-qt-gen/src/syntax/types.rs | 112 ++++++++++++++++- crates/cxx-qt-gen/test_inputs/inheritance.rs | 22 ++-- crates/cxx-qt/src/lib.rs | 21 ++-- .../rust/src/custom_base_class.rs | 60 +++++---- 11 files changed, 487 insertions(+), 190 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7381dcba8..8233e0f40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Added -- Support for inheriting methods from the superclass into Rust using `cxx_qt::inherit!`. +- Support for inheriting methods from the superclass into Rust using `#[cxx_qt::inherit]`. - Register QML types at build time: `#[cxxqt::qobject(qml_uri = "foo.bar", qml_version = "1.0")]` - Register QRC resources at build time in Cargo builds (don't need to call initialization function from Rust `main` function) - Support for container types: `QSet`, `QHash`, `QList`, `QMap`, `QVector` diff --git a/book/src/concepts/inheritance.md b/book/src/concepts/inheritance.md index 7e1302110..d96268926 100644 --- a/book/src/concepts/inheritance.md +++ b/book/src/concepts/inheritance.md @@ -13,15 +13,15 @@ Specifically [QAbstractItemModel](https://doc.qt.io/qt-6/qabstractitemmodel.html To support creating such subclasses directly from within Rust, CXX-Qt provides you with multiple helpers. ## Accessing base class methods -To access the methods of a base class in Rust, use the `cxx_qt::inherit!` macro. -It can be placed within a `impl qobject::T` block in a `#[cxx_qt::bridge]`. +To access the methods of a base class in Rust, use the `#[cxx_qt::inherit]` macro. +It can be placed in front of an `extern "C++"` block in a `#[cxx_qt::bridge]`. ```rust,ignore {{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_qalm}} - impl qobject::CustomBaseClass { -{{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_qalm_impl}} +{{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_qalm_impl_unsafe}} + impl qobject::CustomBaseClass { {{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_clear}} } ``` @@ -30,10 +30,10 @@ It can be placed within a `impl qobject::T` block in a `#[cxx_qt::bridge]`. This code implements a QAbstractListModel subclass. For this, access to the `beginResetModel` and related methods is required. See [the Qt docs](https://doc.qt.io/qt-6/qabstractlistmodel.html) for more details on the specific subclassing requirements. -These methods are then made accessible by using `cxx_qt::inherit!`. +These methods are then made accessible by using `#[cxx_qt::inherit]`. -Methods can be declared inside `cxx_qt::inherit!` in `extern "C++"` blocks similar to C++, with the same restrictions regarding which types can be used. -Additionally, the `self` type must be either `self: Pin<&mut Self>` or `&self`. +Methods can be declared inside `#[cxx_qt::inherit]` in `extern "C++"` blocks similar to CXX, with the same restrictions regarding which types can be used. +Additionally, the `self` type must be either `self: Pin<&mut qobject::T>` or `self: &qobject::T`. Where `qobject::T` may refer to any valid `#[cxx_qt::qobject]` in the `#[cxx_qt::bridge]` The declared methods will be case-converted as in other CXX-Qt APIs. To explicitly declare the C++ method name, use the `#[cxx_name="myFunctionName"]` attribute. @@ -60,16 +60,16 @@ The below example overrides the [`data`](https://doc.qt.io/qt-6/qabstractitemmod ``` [Full example](https://github.com/KDAB/cxx-qt/blob/main/examples/qml_features/rust/src/custom_base_class.rs) -Note that if a method is overridden using `cxx_override` the base class version of the method can be accessed by using `cxx_qt::inherit!` in combination with the `#[cxx_name]` attribute. +Note that if a method is overridden using `cxx_override` the base class version of the method can be accessed by using `#[cxx_qt::inherit]` in combination with the `#[cxx_name]` attribute. In this case the base class version of the function must get a different name, as Rust can't natively express the concept of calling a base class method. Example: ```rust,ignore {{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_qalm}} - impl qobject::CustomBaseClass { -{{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_qalm_impl}} +{{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_qalm_impl_safe}} + impl qobject::CustomBaseClass { {{#include ../../../examples/qml_features/rust/src/custom_base_class.rs:book_inherit_can_fetch_more}} } ``` diff --git a/crates/cxx-qt-gen/src/parser/cxxqtdata.rs b/crates/cxx-qt-gen/src/parser/cxxqtdata.rs index 8278c796d..7881a6454 100644 --- a/crates/cxx-qt-gen/src/parser/cxxqtdata.rs +++ b/crates/cxx-qt-gen/src/parser/cxxqtdata.rs @@ -3,19 +3,27 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::{qobject::ParsedQObject, signals::ParsedSignalsEnum}; +use crate::parser::{ + inherit::{InheritMethods, ParsedInheritedMethod}, + qobject::ParsedQObject, + signals::ParsedSignalsEnum, +}; use crate::syntax::attribute::attribute_tokens_to_value; use crate::syntax::foreignmod::{foreign_mod_to_foreign_item_types, verbatim_to_foreign_mod}; use crate::syntax::{ attribute::{attribute_find_path, attribute_tokens_to_ident}, path::path_to_single_ident, }; +use proc_macro2::TokenStream; +use quote::ToTokens; use std::collections::BTreeMap; use syn::{ - spanned::Spanned, Attribute, Error, Ident, Item, ItemEnum, ItemImpl, LitStr, Result, Type, - TypePath, + spanned::Spanned, Attribute, Error, Ident, Item, ItemEnum, ItemForeignMod, ItemImpl, LitStr, + Result, Type, TypePath, }; +use super::inherit::MaybeInheritMethods; + #[derive(Default)] pub struct ParsedCxxMappings { /// Map of the cxx_name of any types defined in CXX extern blocks @@ -183,10 +191,65 @@ impl ParsedCxxQtData { self.uses.push(item); Ok(None) } + Item::Verbatim(tokens) => self.try_parse_inherit_verbatim(tokens), + Item::ForeignMod(foreign_mod) => self.parse_foreign_mod(foreign_mod), _ => Ok(Some(item)), } } + fn try_parse_inherit_verbatim(&mut self, tokens: TokenStream) -> Result> { + let try_parse: MaybeInheritMethods = syn::parse2(tokens)?; + + match try_parse { + MaybeInheritMethods::Found(inherited) => { + self.add_inherited_methods(inherited)?; + Ok(None) + } + MaybeInheritMethods::PassThrough(item) => Ok(Some(item)), + } + } + + fn parse_inherit_mod(&mut self, tokens: TokenStream) -> Result<()> { + let inherited: InheritMethods = syn::parse2(tokens)?; + + self.add_inherited_methods(inherited) + } + + fn add_inherited_methods(&mut self, inherited: InheritMethods) -> Result<()> { + for method in inherited.base_functions.into_iter() { + let parsed_inherited_method = if inherited.is_safe { + ParsedInheritedMethod::parse_safe(method)? + } else { + ParsedInheritedMethod::parse_unsafe(method)? + }; + + if let Some(ref mut qobject) = self + .qobjects + .get_mut(&parsed_inherited_method.qobject_ident) + { + qobject.inherited_methods.push(parsed_inherited_method); + } else { + return Err(Error::new_spanned( + parsed_inherited_method.qobject_ident, + "No QObject with this name found.", + )); + } + } + Ok(()) + } + + fn parse_foreign_mod(&mut self, mut foreign_mod: ItemForeignMod) -> Result> { + // Check if the foreign mod has cxx_qt::inherit on it + if let Some(index) = attribute_find_path(&foreign_mod.attrs, &["cxx_qt", "inherit"]) { + // Remove the inherit attribute + foreign_mod.attrs.remove(index); + + self.parse_inherit_mod(foreign_mod.into_token_stream())?; + return Ok(None); + } + Ok(Some(Item::ForeignMod(foreign_mod))) + } + /// Parse a [syn::ItemEnum] into the qobjects if it's a CXX-Qt signal /// otherwise return as a [syn::Item] to pass through. fn parse_enum(&mut self, item_enum: ItemEnum) -> Result> { @@ -716,4 +779,44 @@ mod tests { "struct_namespace" ); } + + #[test] + fn test_parse_inherited_methods() { + let mut cxxqtdata = create_parsed_cxx_qt_data(); + + let unsafe_block: Item = tokens_to_syn(quote! { + #[cxx_qt::inherit] + unsafe extern "C++" { + fn test(self: &qobject::MyObject); + + fn with_args(self: &qobject::MyObject, arg: i32); + } + }); + let safe_block: Item = tokens_to_syn(quote! { + #[cxx_qt::inherit] + extern "C++" { + #[cxx_name="withRename"] + unsafe fn with_rename(self: Pin<&mut qobject::MyObject>, arg: i32); + } + }); + + cxxqtdata.parse_cxx_qt_item(unsafe_block).unwrap(); + cxxqtdata.parse_cxx_qt_item(safe_block).unwrap(); + + let qobject = cxxqtdata.qobjects.get(&qobject_ident()).unwrap(); + + let inherited = &qobject.inherited_methods; + assert_eq!(inherited.len(), 3); + assert!(!inherited[0].mutable); + assert!(!inherited[1].mutable); + assert!(inherited[2].mutable); + assert!(inherited[0].safe); + assert!(inherited[1].safe); + assert!(!inherited[2].safe); + assert_eq!(inherited[0].parameters.len(), 0); + assert_eq!(inherited[1].parameters.len(), 1); + assert_eq!(inherited[1].parameters[0].ident, "arg"); + assert_eq!(inherited[2].parameters.len(), 1); + assert_eq!(inherited[2].parameters[0].ident, "arg"); + } } diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs index e1b4de2d3..256f7a83e 100644 --- a/crates/cxx-qt-gen/src/parser/inherit.rs +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -6,72 +6,97 @@ use crate::{ generator::naming::CombinedIdent, parser::parameter::ParsedFunctionParameter, - syntax::{attribute::attribute_tokens_to_value, implitemmethod::is_method_mutable}, + syntax::{ + attribute::{attribute_find_path, attribute_tokens_to_value}, + foreignmod, types, + }, }; use quote::format_ident; use syn::{ parse::{Parse, ParseStream}, spanned::Spanned, - Error, ForeignItem, ForeignItemFn, Ident, ItemForeignMod, LitStr, Result, Token, + Attribute, Error, ForeignItem, ForeignItemFn, Ident, Item, ItemForeignMod, LitStr, Result, + Token, }; -/// This type is used when parsing the `cxx_qt::inherit!` macro contents into raw ForeignItemFn items +/// Used when parsing a syn::Item::Verbatim, that we suspect may be a `#[cxx_qt::inherit]` block, +/// but we don't yet know whether this is actually the case. +/// This is the case if `#[cxx_qt::inherit]` is used with `unsafe extern "C++"`. +pub enum MaybeInheritMethods { + /// We found a `#[cxx_qt::inherit]` block + Found(InheritMethods), + /// `#[cxx_qt::inherit]` block not found, pass this Item through to outside code! + PassThrough(Item), +} + +impl Parse for MaybeInheritMethods { + fn parse(input: ParseStream) -> Result { + let lookahead = input.fork(); + if let Ok(attribute) = lookahead.call(Attribute::parse_outer) { + if attribute_find_path(attribute.as_slice(), &["cxx_qt", "inherit"]).is_some() { + input.call(Attribute::parse_outer)?; + let methods = input.parse::()?; + return Ok(Self::Found(methods)); + } + } + + Ok(Self::PassThrough(input.parse()?)) + } +} + +/// This type is used when parsing the `#[cxx_qt::inherit]` macro contents into raw ForeignItemFn items pub struct InheritMethods { - pub base_safe_functions: Vec, - pub base_unsafe_functions: Vec, + pub is_safe: bool, + pub base_functions: Vec, } impl Parse for InheritMethods { fn parse(input: ParseStream) -> Result { - let mut base_safe_functions = Vec::new(); - let mut base_unsafe_functions = Vec::new(); - - while !input.is_empty() { - // base_safe_functions.push(input.parse::()?); - // This looks somewhat counter-intuitive, but if we add `unsafe` - // to the `extern "C++"` block, the contained functions will be safe to call. - let is_safe = input.peek(Token![unsafe]); - if is_safe { - input.parse::()?; - } + let mut base_functions = Vec::new(); - let extern_block = input.parse::()?; - if extern_block.abi.name != Some(LitStr::new("C++", extern_block.abi.span())) { - return Err(Error::new( - extern_block.abi.span(), - "Inherit blocks must be marked with `extern \"C++\"`", - )); - } + // base_safe_functions.push(input.parse::()?); + // This looks somewhat counter-intuitive, but if we add `unsafe` + // to the `extern "C++"` block, the contained functions will be safe to call. + let is_safe = input.peek(Token![unsafe]); + if is_safe { + input.parse::()?; + } + + let extern_block = input.parse::()?; + if extern_block.abi.name != Some(LitStr::new("C++", extern_block.abi.span())) { + return Err(Error::new( + extern_block.abi.span(), + "Inherit blocks must be marked with `extern \"C++\"`", + )); + } - for item in extern_block.items { - match item { - ForeignItem::Fn(function) => { - if is_safe { - base_safe_functions.push(function); - } else { - base_unsafe_functions.push(function); - } - } - _ => { - return Err(Error::new( - item.span(), - "Only functions are allowed in cxx_qt::inherit! blocks", - )) - } + for item in extern_block.items { + match item { + ForeignItem::Fn(function) => { + base_functions.push(function); + } + _ => { + return Err(Error::new( + item.span(), + "Only functions are allowed in #[cxx_qt::inherit] blocks", + )) } } } + Ok(InheritMethods { - base_safe_functions, - base_unsafe_functions, + is_safe, + base_functions, }) } } -/// Describes a method found in cxx_qt::inherit! +/// Describes a method found in #[cxx_qt::inherit] pub struct ParsedInheritedMethod { /// The original [syn::ForeignItemFn] of the inherited method declaration pub method: ForeignItemFn, + /// The type of the self argument + pub qobject_ident: Ident, /// whether the inherited method is marked as mutable pub mutable: bool, /// Whether the method is safe to call. @@ -96,16 +121,18 @@ impl ParsedInheritedMethod { } pub fn parse_safe(method: ForeignItemFn) -> Result { - let mutable = is_method_mutable(&method.sig); + let self_receiver = foreignmod::self_type_from_foreign_fn(&method.sig)?; + let (qobject_ident, mutability) = types::extract_qobject_ident(&self_receiver.typ)?; + let mutable = mutability.is_some(); - let parameters = ParsedFunctionParameter::parse_all_without_receiver(&method.sig)?; + 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.path.is_ident(&format_ident!("cxx_name")) { return Err(Error::new( attribute.span(), - "Unsupported attribute in cxx_qt::inherit!", + "Unsupported attribute in #[cxx_qt::inherit]", )); } @@ -118,6 +145,7 @@ impl ParsedInheritedMethod { Ok(Self { method, + qobject_ident, mutable, parameters, ident, diff --git a/crates/cxx-qt-gen/src/parser/parameter.rs b/crates/cxx-qt-gen/src/parser/parameter.rs index afc57c4b1..764b911ea 100644 --- a/crates/cxx-qt-gen/src/parser/parameter.rs +++ b/crates/cxx-qt-gen/src/parser/parameter.rs @@ -20,6 +20,40 @@ pub struct ParsedFunctionParameter { } impl ParsedFunctionParameter { + fn parse_remaining<'a>( + iter: impl Iterator, + ) -> Result> { + iter.map(|input| { + match input { + FnArg::Typed(type_pattern) => { + let parameter = ParsedFunctionParameter::parse(type_pattern)?; + + // Ignore self as a parameter + if parameter.ident == "self" { + return Ok(None); + } + Ok(Some(parameter)) + } + // Ignore self as a parameter + FnArg::Receiver(_) => Ok(None), + } + }) + .filter_map(|result| result.map_or_else(|e| Some(Err(e)), |v| v.map(Ok))) + .collect::>>() + } + + pub fn parse_all_ignoring_receiver( + signature: &Signature, + ) -> Result> { + let mut iter = signature.inputs.iter(); + // whilst we can ignore the receiver argument, make sure it actually exists + if iter.next().is_none() { + return Err(Error::new_spanned(signature, "Missing receiver argument!")); + } + + Self::parse_remaining(iter) + } + /// This function parses the list of arguments pub fn parse_all_without_receiver( signature: &Signature, @@ -46,23 +80,7 @@ impl ParsedFunctionParameter { None => Err(Error::new_spanned(signature, "Missing 'self' receiver!")), }?; - iter.map(|input| { - match input { - FnArg::Typed(type_pattern) => { - let parameter = ParsedFunctionParameter::parse(type_pattern)?; - - // Ignore self as a parameter - if parameter.ident == "self" { - return Ok(None); - } - Ok(Some(parameter)) - } - // Ignore self as a parameter - FnArg::Receiver(_) => Ok(None), - } - }) - .filter_map(|result| result.map_or_else(|e| Some(Err(e)), |v| v.map(Ok))) - .collect::>>() + Self::parse_remaining(iter) } pub fn parse(type_pattern: &PatType) -> Result { diff --git a/crates/cxx-qt-gen/src/parser/qobject.rs b/crates/cxx-qt-gen/src/parser/qobject.rs index 1bbcdd1c5..69e546f10 100644 --- a/crates/cxx-qt-gen/src/parser/qobject.rs +++ b/crates/cxx-qt-gen/src/parser/qobject.rs @@ -3,22 +3,19 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 +use crate::parser::{ + inherit::*, + invokable::ParsedQInvokable, + property::{ParsedQProperty, ParsedRustField}, + signals::ParsedSignalsEnum, +}; use crate::syntax::{ attribute::{attribute_find_path, attribute_tokens_to_map, AttributeDefault}, fields::fields_to_named_fields_mut, }; -use crate::{ - parser::{ - inherit::*, - invokable::ParsedQInvokable, - property::{ParsedQProperty, ParsedRustField}, - signals::ParsedSignalsEnum, - }, - syntax, -}; use syn::{ - spanned::Spanned, Error, Fields, Ident, ImplItem, ImplItemMacro, ImplItemMethod, Item, - ItemStruct, LitStr, Result, + spanned::Spanned, Error, Fields, Ident, ImplItem, ImplItemMethod, Item, ItemStruct, LitStr, + Result, }; /// Metadata for registering QML element @@ -189,22 +186,6 @@ impl ParsedQObject { Ok(()) } - fn parse_impl_inherit(&mut self, mac: &ImplItemMacro) -> Result<()> { - let inherited: InheritMethods = mac.mac.parse_body()?; - - for method in inherited.base_safe_functions.into_iter() { - self.inherited_methods - .push(ParsedInheritedMethod::parse_safe(method)?); - } - - for method in inherited.base_unsafe_functions.into_iter() { - self.inherited_methods - .push(ParsedInheritedMethod::parse_unsafe(method)?); - } - - 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 @@ -215,15 +196,10 @@ impl ParsedQObject { ImplItem::Method(method) => { self.parse_impl_method(method)?; } - ImplItem::Macro(mac) => { - if syntax::path::path_compare_str(&mac.mac.path, &["cxx_qt", "inherit"]) { - self.parse_impl_inherit(mac)?; - } - } _ => { return Err(Error::new( item.span(), - "Only methods or cxx_qt::inherit is supported.", + "Only methods are supported within a QObject impl block!", )); } } @@ -388,42 +364,6 @@ pub mod tests { .contains(&ParsedQInvokableSpecifiers::Virtual)); } - #[test] - fn test_parse_inherited_methods() { - let mut qobject = create_parsed_qobject(); - let item: ItemImpl = tokens_to_syn(quote! { - impl T { - cxx_qt::inherit! { - unsafe extern "C++" { - fn test(&self); - - fn with_args(&self, arg: i32); - } - extern "C++" { - #[cxx_name="withRename"] - unsafe fn with_rename(self: Pin<&mut Self>, arg: i32); - } - } - } - }); - - qobject.parse_impl_items(&item.items).unwrap(); - - let inherited = &qobject.inherited_methods; - assert_eq!(inherited.len(), 3); - assert!(!inherited[0].mutable); - assert!(!inherited[1].mutable); - assert!(inherited[2].mutable); - assert!(inherited[0].safe); - assert!(inherited[1].safe); - assert!(!inherited[2].safe); - assert_eq!(inherited[0].parameters.len(), 0); - assert_eq!(inherited[1].parameters.len(), 1); - assert_eq!(inherited[1].parameters[0].ident, "arg"); - assert_eq!(inherited[2].parameters.len(), 1); - assert_eq!(inherited[2].parameters[0].ident, "arg"); - } - #[test] fn test_parse_impl_items_invalid() { let mut qobject = create_parsed_qobject(); diff --git a/crates/cxx-qt-gen/src/syntax/foreignmod.rs b/crates/cxx-qt-gen/src/syntax/foreignmod.rs index d5ec135e8..4eb2ab9c2 100644 --- a/crates/cxx-qt-gen/src/syntax/foreignmod.rs +++ b/crates/cxx-qt-gen/src/syntax/foreignmod.rs @@ -7,7 +7,8 @@ use proc_macro2::{TokenStream, TokenTree}; use quote::{quote, ToTokens}; use syn::{ parse::{ParseStream, Parser}, - Attribute, ForeignItem, ForeignItemType, Ident, ItemForeignMod, Result, Token, Visibility, + Attribute, Error, ForeignItem, ForeignItemType, Ident, ItemForeignMod, PatIdent, PatType, + Result, Signature, Token, Visibility, }; /// For a given [syn::ForeignItem] return the [syn::ForeignItemType] if there is one @@ -111,8 +112,56 @@ fn verbatim_to_foreign_type(tokens: &TokenStream) -> Result Result { + let mut span: &dyn ToTokens = signature; + if let Some(arg) = signature.inputs.iter().next() { + span = arg; + if let syn::FnArg::Typed(PatType { + pat, + ty, + attrs, + colon_token: _, + }) = arg + { + if !attrs.is_empty() { + return Err(Error::new_spanned( + arg, + "Attributes on the `self:` receiver are not supported", + )); + } + if let syn::Pat::Ident(PatIdent { + ident, + // It should be a `self:` value, without `mut` or `&` + mutability: None, + by_ref: None, + attrs, + subpat: _, + }) = &**pat + { + if ident == "self" && attrs.is_empty() { + return Ok(ForeignFnSelf { + ident: ident.clone(), + typ: *ty.clone(), + }); + } + } + } + } + Err(Error::new_spanned( + span, + "Expected first argument to be a `self:` receiver", + )) +} + #[cfg(test)] mod tests { + use syn::ForeignItemFn; + use super::*; use crate::tests::tokens_to_syn; @@ -150,4 +199,38 @@ mod tests { assert_eq!(result.attrs.len(), 1); assert_eq!(result.items.len(), 1); } + + #[test] + fn test_foreign_fn_self() { + let foreign_fn: ForeignItemFn = tokens_to_syn(quote! { + fn foo(self: &qobject::T, a: A) -> B; + }); + let result = self_type_from_foreign_fn(&foreign_fn.sig).unwrap(); + assert_eq!(result.ident, "self"); + assert_eq!(result.typ.to_token_stream().to_string(), "& qobject :: T"); + } + + #[test] + fn test_foreign_fn_invalid_self() { + macro_rules! test { + ($($tt:tt)*) => { + let foreign_fn: ForeignItemFn = tokens_to_syn(quote! { + $($tt)* + }); + assert!(self_type_from_foreign_fn(&foreign_fn.sig).is_err()); + } + } + // Missing self + test! { fn foo(a: A) -> B; } + // self without type + test! { fn foo(self); } + // self with mut + test! { fn foo(mut self: T); } + // self reference + test! { fn foo(&self); } + // self reference with mut + test! { fn foo(&mut self); } + // attribute on self type + test! { fn foo(#[attr] self: T); } + } } diff --git a/crates/cxx-qt-gen/src/syntax/types.rs b/crates/cxx-qt-gen/src/syntax/types.rs index 89cae6502..62d84a1de 100644 --- a/crates/cxx-qt-gen/src/syntax/types.rs +++ b/crates/cxx-qt-gen/src/syntax/types.rs @@ -4,7 +4,10 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 use crate::syntax::path::*; -use syn::{GenericArgument, PathArguments, Type, TypePath, TypeReference}; +use syn::{ + token::Mut, Error, GenericArgument, Ident, Path, PathArguments, Result, Type, TypePath, + TypeReference, +}; /// Checks if the given type is a `Pin<&Self>` or `Pin<&mut Self>`. /// `Pin>` is currently not supported. @@ -34,6 +37,88 @@ pub fn is_pin_of_self(ty: &Type) -> bool { false } +fn extract_qobject_ident_from_path(path: &Path) -> Result { + if path.segments.len() == 2 && path.segments.first().unwrap().ident == "qobject" { + Ok(path.segments.last().unwrap().ident.clone()) + } else { + Err(Error::new_spanned(path, "Expected a qobject::T type!")) + } +} + +fn extract_qobject_ident_from_ref(ty: &TypeReference) -> Result<(Ident, Option)> { + if let Type::Path(type_path) = &*ty.elem { + Ok(( + extract_qobject_ident_from_path(&type_path.path)?, + ty.mutability, + )) + } else { + Err(Error::new_spanned( + ty, + "Expected type to be a `&qobject::T` reference!", + )) + } +} + +fn extract_qobject_from_mut_pin(ty: &TypePath) -> Result<(Ident, Mut)> { + if path_compare_str(&ty.path, &["Pin"]) { + if let PathArguments::AngleBracketed(angles) = &ty.path.segments.first().unwrap().arguments + { + if let [GenericArgument::Type(Type::Reference(reference))] = + *angles.args.iter().collect::>() + { + let (ident, mutability) = extract_qobject_ident_from_ref(reference)?; + if mutability.is_none() { + return Err(Error::new_spanned( + reference, + "Expected a mutable reference when using Pin<>!", + )); + } + return Ok((ident, mutability.unwrap())); + } + } + } + if ty + .path + .segments + .first() + .map(|segment| segment.ident == "qobject") + == Some(true) + { + Err(Error::new_spanned(ty, "Cannot take qobject::T by value, use either `self: &qobject::T`, or `Pin<&mut qobject::T>`")) + } else { + Err(Error::new_spanned( + ty, + "Expected a qobject::T refernce! Use either `&qobject::T` or `Pin<&mut qobject::T>`", + )) + } +} + +/// Extract the qobject ident from any of the following patterns: +/// - &qobject::T +/// - Pin<&mut qobject::T> +pub fn extract_qobject_ident(ty: &Type) -> Result<(Ident, Option)> { + match ty { + Type::Reference(type_ref) => { + let (ident, mutability) = extract_qobject_ident_from_ref(type_ref)?; + if mutability.is_some() { + return Err(Error::new_spanned( + type_ref, + "Cannot take qobject::T by mutable reference, use either `self: &qobject::T`, or `Pin<&mut qobject::T>`", + )); + } + Ok((ident, mutability)) + } + Type::Path(type_path) => { + let (ident, mutability) = extract_qobject_from_mut_pin(type_path)?; + Ok((ident, Some(mutability))) + } + _ => Err(Error::new_spanned( + ty, + "Expected type to be a &qobject::T or Pin<&mut qobject::T> reference!", + )), + } +} + #[cfg(test)] mod tests { @@ -66,4 +151,29 @@ mod tests { assert_not_pin_of_self(quote! { Pin<&Foo> }); assert_not_pin_of_self(quote! { Pin<&mut Foo> }); } + + fn assert_qobject_ident(tokens: TokenStream, expected_ident: &str, expected_mutability: bool) { + let ty: Type = tokens_to_syn(tokens); + let (ident, mutability) = super::extract_qobject_ident(&ty).unwrap(); + assert_eq!(ident.to_string(), expected_ident); + assert_eq!(mutability.is_some(), expected_mutability); + } + + fn assert_no_qobject_ident(tokens: TokenStream) { + let ty: Type = tokens_to_syn(tokens); + assert!(super::extract_qobject_ident(&ty).is_err()); + } + + #[test] + fn test_extract_qobject_ident() { + assert_qobject_ident(quote! { &qobject::Foo }, "Foo", false); + assert_qobject_ident(quote! { Pin<&mut qobject::Foo> }, "Foo", true); + + assert_no_qobject_ident(quote! { qobject::Foo }); + assert_no_qobject_ident(quote! { &mut qobject::Foo }); + assert_no_qobject_ident(quote! { Pin<&qobject::Foo> }); + assert_no_qobject_ident(quote! { Foo }); + assert_no_qobject_ident(quote! { qobject::X::Foo }); + assert_no_qobject_ident(quote! { Self }); + } } diff --git a/crates/cxx-qt-gen/test_inputs/inheritance.rs b/crates/cxx-qt-gen/test_inputs/inheritance.rs index fe10e1c13..e98ff8188 100644 --- a/crates/cxx-qt-gen/test_inputs/inheritance.rs +++ b/crates/cxx-qt-gen/test_inputs/inheritance.rs @@ -13,23 +13,23 @@ mod inheritance { data: Vec, } + #[cxx_qt::inherit] + unsafe extern "C++" { + #[cxx_name = "hasChildren"] + fn has_children_super(self: &qobject::MyObject, parent: &QModelIndex) -> bool; + } + + #[cxx_qt::inherit] + extern "C++" { + unsafe fn fetch_more(self: Pin<&mut qobject::MyObject>, index: &QModelIndex); + } + impl qobject::MyObject { #[qinvokable(cxx_override)] pub fn data(&self, _index: &QModelIndex, _role: i32) -> QVariant { QVariant::default() } - cxx_qt::inherit! { - extern "C++" { - unsafe fn fetch_more(self: Pin<&mut Self>, index: &QModelIndex); - } - - unsafe extern "C++" { - #[cxx_name="hasChildren"] - fn has_children_super(&self, parent: &QModelIndex) -> bool; - } - } - #[qinvokable(cxx_override)] pub fn has_children(&self, _parent: &QModelIndex) -> bool { false diff --git a/crates/cxx-qt/src/lib.rs b/crates/cxx-qt/src/lib.rs index f7d1f2a0d..4828efd54 100644 --- a/crates/cxx-qt/src/lib.rs +++ b/crates/cxx-qt/src/lib.rs @@ -121,19 +121,16 @@ pub fn qobject(_args: TokenStream, _input: TokenStream) -> TokenStream { /// #[derive(Default)] /// struct MyObject; /// -/// impl qobject::MyObject { -/// cxx_qt::inherit! { -/// extern "C++" { -/// // Unsafe to call -/// unsafe fn begin_insert_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); -/// } -/// unsafe extern "C++" { -/// // Safe to call - you are responsible to ensure this is true. -/// fn end_insert_rows(self: Pin<&mut Self>); -/// } -/// } +/// #[cxx_qt::inherit] +/// extern "C++" { +/// // Unsafe to call +/// unsafe fn begin_insert_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); +/// } /// -/// // ... +/// #[cxx_qt::inherit] +/// unsafe extern "C++" { +/// // Safe to call - you are responsible to ensure this is true! +/// fn end_insert_rows(self: Pin<&mut Self>); /// } /// } /// ``` diff --git a/examples/qml_features/rust/src/custom_base_class.rs b/examples/qml_features/rust/src/custom_base_class.rs index fbde18c79..4869f46fc 100644 --- a/examples/qml_features/rust/src/custom_base_class.rs +++ b/examples/qml_features/rust/src/custom_base_class.rs @@ -136,29 +136,47 @@ mod ffi { } } - // QAbstractListModel implementation - impl qobject::CustomBaseClass { - // ANCHOR: book_inherit_qalm_impl - cxx_qt::inherit! { - extern "C++" { - unsafe fn begin_insert_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); - unsafe fn end_insert_rows(self: Pin<&mut Self>); - - unsafe fn begin_remove_rows(self: Pin<&mut Self>, parent: &QModelIndex, first: i32, last: i32); - unsafe fn end_remove_rows(self: Pin<&mut Self>); - - unsafe fn begin_reset_model(self: Pin<&mut Self>); - unsafe fn end_reset_model(self: Pin<&mut Self>); - } - unsafe extern "C++" { - #[cxx_name="canFetchMore"] - fn base_can_fetch_more(&self, parent: &QModelIndex) -> bool; + // ANCHOR: book_inherit_qalm_impl_unsafe + #[cxx_qt::inherit] + extern "C++" { + unsafe fn begin_insert_rows( + self: Pin<&mut qobject::CustomBaseClass>, + parent: &QModelIndex, + first: i32, + last: i32, + ); + unsafe fn end_insert_rows(self: Pin<&mut qobject::CustomBaseClass>); + + unsafe fn begin_remove_rows( + self: Pin<&mut qobject::CustomBaseClass>, + parent: &QModelIndex, + first: i32, + last: i32, + ); + unsafe fn end_remove_rows(self: Pin<&mut qobject::CustomBaseClass>); + + unsafe fn begin_reset_model(self: Pin<&mut qobject::CustomBaseClass>); + unsafe fn end_reset_model(self: Pin<&mut qobject::CustomBaseClass>); + } + // ANCHOR_END: book_inherit_qalm_impl_unsafe - fn index(&self, row: i32, column: i32, parent: &QModelIndex) -> QModelIndex; - } - } - // ANCHOR_END: book_inherit_qalm_impl + // ANCHOR: book_inherit_qalm_impl_safe + #[cxx_qt::inherit] + unsafe extern "C++" { + #[cxx_name = "canFetchMore"] + fn base_can_fetch_more(self: &qobject::CustomBaseClass, parent: &QModelIndex) -> bool; + + fn index( + self: &qobject::CustomBaseClass, + row: i32, + column: i32, + parent: &QModelIndex, + ) -> QModelIndex; + } + // ANCHOR_END: book_inherit_qalm_impl_safe + // QAbstractListModel implementation + impl qobject::CustomBaseClass { // ANCHOR: book_inherit_data #[qinvokable(cxx_override)] fn data(&self, index: &QModelIndex, role: i32) -> QVariant { From 3cef7ba9b6dedb01a4ea1aa38366c98705935e85 Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Wed, 22 Feb 2023 10:31:16 +0100 Subject: [PATCH 09/14] Apply suggestions from code review - Explaining comment in qml_features - Improved wording in book Co-authored-by: Be --- book/src/concepts/inheritance.md | 2 +- examples/qml_features/rust/src/custom_base_class.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/book/src/concepts/inheritance.md b/book/src/concepts/inheritance.md index d96268926..7ec9169a6 100644 --- a/book/src/concepts/inheritance.md +++ b/book/src/concepts/inheritance.md @@ -61,7 +61,7 @@ The below example overrides the [`data`](https://doc.qt.io/qt-6/qabstractitemmod [Full example](https://github.com/KDAB/cxx-qt/blob/main/examples/qml_features/rust/src/custom_base_class.rs) Note that if a method is overridden using `cxx_override` the base class version of the method can be accessed by using `#[cxx_qt::inherit]` in combination with the `#[cxx_name]` attribute. -In this case the base class version of the function must get a different name, as Rust can't natively express the concept of calling a base class method. +In this case the base class version of the function must get a different name, as Rust can't have two functions with the same name on one type. Example: ```rust,ignore diff --git a/examples/qml_features/rust/src/custom_base_class.rs b/examples/qml_features/rust/src/custom_base_class.rs index 4869f46fc..0fcda1494 100644 --- a/examples/qml_features/rust/src/custom_base_class.rs +++ b/examples/qml_features/rust/src/custom_base_class.rs @@ -137,6 +137,7 @@ mod ffi { } // ANCHOR: book_inherit_qalm_impl_unsafe + // Create Rust bindings for C++ functions of the base class (QAbstractItemModel) #[cxx_qt::inherit] extern "C++" { unsafe fn begin_insert_rows( From 258ae0f58881475ecac6ac229184b0798f378f9a Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Wed, 22 Feb 2023 15:18:02 +0100 Subject: [PATCH 10/14] Incorporate feedback. - More tests - Extract rust generation into /generator/rust/inherit.rs - Avoid wildcard imports --- .../cxx-qt-gen/src/generator/cpp/inherit.rs | 118 +++++++++++++- .../cxx-qt-gen/src/generator/cpp/qobject.rs | 8 +- .../cxx-qt-gen/src/generator/rust/inherit.rs | 146 ++++++++++++++++++ crates/cxx-qt-gen/src/generator/rust/mod.rs | 1 + .../cxx-qt-gen/src/generator/rust/qobject.rs | 53 +------ crates/cxx-qt-gen/src/parser/cxxqtdata.rs | 6 +- crates/cxx-qt-gen/src/parser/inherit.rs | 146 ++++++++++++++++-- crates/cxx-qt-gen/src/parser/parameter.rs | 58 +++++++ crates/cxx-qt-gen/src/parser/qobject.rs | 2 +- crates/cxx-qt-gen/src/syntax/mod.rs | 1 + crates/cxx-qt-gen/src/syntax/safety.rs | 5 + crates/cxx-qt-gen/src/syntax/types.rs | 2 +- crates/cxx-qt-gen/test_outputs/inheritance.h | 4 +- crates/cxx-qt-gen/test_outputs/inheritance.rs | 4 +- 14 files changed, 468 insertions(+), 86 deletions(-) create mode 100644 crates/cxx-qt-gen/src/generator/rust/inherit.rs create mode 100644 crates/cxx-qt-gen/src/syntax/safety.rs diff --git a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs index a5f5c4c83..45f2e6f09 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs @@ -6,7 +6,7 @@ use indoc::formatdoc; use crate::{ generator::cpp::{fragment::CppFragment, qobject::GeneratedCppQObjectBlocks}, - parser::{cxxqtdata::ParsedCxxMappings, qobject::ParsedQObject}, + parser::{cxxqtdata::ParsedCxxMappings, inherit::ParsedInheritedMethod}, }; use syn::{Result, ReturnType}; @@ -14,12 +14,13 @@ use syn::{Result, ReturnType}; use super::types::CppType; pub fn generate( - qobject: &ParsedQObject, + inherited_methods: &[ParsedInheritedMethod], + base_class: &Option, cxx_mappings: &ParsedCxxMappings, ) -> Result { let mut result = GeneratedCppQObjectBlocks::default(); - for method in &qobject.inherited_methods { + for method in inherited_methods { let return_type = if let ReturnType::Type(_, ty) = &method.method.sig.output { CppType::from(ty, &None, cxx_mappings)? .as_cxx_ty() @@ -28,17 +29,18 @@ pub fn generate( "void".to_owned() }; - let base_class = qobject.base_class.as_deref().unwrap_or("QObject"); + let base_class = base_class.as_deref().unwrap_or("QObject"); result.methods.push(CppFragment::Header(formatdoc! { r#" template - {return_type} {wrapper_ident}(Args ...args) {mutability} {{ + {return_type} {wrapper_ident}(Args ...args){mutability} + {{ return {base_class}::{func_ident}(args...); }}"#, - mutability = if method.mutable { "" } else { "const" }, + mutability = if method.mutable { "" } else { " const" }, func_ident = method.ident.cpp, - wrapper_ident = method.wrapper_ident, + wrapper_ident = method.wrapper_ident(), return_type = return_type, base_class = base_class })); @@ -46,3 +48,105 @@ pub fn generate( Ok(result) } + +#[cfg(test)] +mod tests { + use pretty_assertions::assert_str_eq; + use syn::ForeignItemFn; + + use crate::{ + parser::inherit::ParsedInheritedMethod, syntax::safety::Safety, tests::tokens_to_syn, + }; + + use super::*; + use quote::quote; + + fn generate_from_foreign( + tokens: proc_macro2::TokenStream, + base_class: Option<&str>, + ) -> Result { + let method: ForeignItemFn = tokens_to_syn(tokens); + let inherited_methods = vec![ParsedInheritedMethod::parse(method, Safety::Safe).unwrap()]; + let base_class = base_class.map(|s| s.to_owned()); + generate( + &*inherited_methods, + &base_class, + &ParsedCxxMappings::default(), + ) + } + + fn assert_generated_eq(expected: &str, generated: &GeneratedCppQObjectBlocks) { + assert_eq!(generated.methods.len(), 1); + if let CppFragment::Header(header) = &generated.methods[0] { + assert_str_eq!(header, expected); + } else { + panic!("Expected header fragment"); + } + } + + #[test] + fn test_immutable() { + let generated = generate_from_foreign( + quote! { + fn test(self: &qobject::T, a: B, b: C); + }, + Some("TestBaseClass"), + ) + .unwrap(); + + assert_generated_eq( + indoc::indoc! {" + template + void testCxxqtInherit(Args ...args) const + { + return TestBaseClass::test(args...); + }" + }, + &generated, + ); + } + + #[test] + fn test_mutable() { + let generated = generate_from_foreign( + quote! { + fn test(self: Pin<&mut qobject::T>); + }, + Some("TestBaseClass"), + ) + .unwrap(); + + assert_generated_eq( + indoc::indoc! {" + template + void testCxxqtInherit(Args ...args) + { + return TestBaseClass::test(args...); + }" + }, + &generated, + ); + } + + #[test] + fn test_default_base_class() { + let generated = generate_from_foreign( + quote! { + fn test(self: &qobject::T); + }, + None, + ) + .unwrap(); + + assert_generated_eq( + indoc::indoc! {" + template + void testCxxqtInherit(Args ...args) const + { + return QObject::test(args...); + }" + }, + &generated, + ); + } +} diff --git a/crates/cxx-qt-gen/src/generator/cpp/qobject.rs b/crates/cxx-qt-gen/src/generator/cpp/qobject.rs index 74d124735..a69ad6590 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/qobject.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/qobject.rs @@ -107,9 +107,11 @@ impl GeneratedCppQObject { cxx_mappings, )?); } - generated - .blocks - .append(&mut inherit::generate(qobject, cxx_mappings)?); + generated.blocks.append(&mut inherit::generate( + &*qobject.inherited_methods, + &qobject.base_class, + cxx_mappings, + )?); Ok(generated) } diff --git a/crates/cxx-qt-gen/src/generator/rust/inherit.rs b/crates/cxx-qt-gen/src/generator/rust/inherit.rs new file mode 100644 index 000000000..64b254250 --- /dev/null +++ b/crates/cxx-qt-gen/src/generator/rust/inherit.rs @@ -0,0 +1,146 @@ +use crate::{ + generator::{naming::qobject::QObjectName, rust::qobject::GeneratedRustQObjectBlocks}, + parser::inherit::ParsedInheritedMethod, +}; +use proc_macro2::TokenStream; +use quote::quote; +use syn::{Item, Result}; + +pub fn generate( + qobject_ident: &QObjectName, + methods: &[ParsedInheritedMethod], +) -> Result { + let mut blocks = GeneratedRustQObjectBlocks::default(); + let qobject_name = &qobject_ident.cpp_class.rust; + + let mut bridges = methods + .iter() + .map(|method| { + let parameters = method + .parameters + .iter() + .map(|parameter| { + let ident = ¶meter.ident; + let ty = ¶meter.ty; + quote! { #ident: #ty } + }) + .collect::>(); + let ident = &method.method.sig.ident; + let cxx_name_string = &method.wrapper_ident().to_string(); + let self_param = if method.mutable { + quote! { self: Pin<&mut #qobject_name> } + } else { + quote! { self: &#qobject_name } + }; + let return_type = &method.method.sig.output; + + let mut unsafe_block = None; + let mut unsafe_call = Some(quote! { unsafe }); + if method.safe { + std::mem::swap(&mut unsafe_call, &mut unsafe_block); + } + syn::parse2(quote! { + #unsafe_block extern "C++" { + #[cxx_name=#cxx_name_string] + #unsafe_call fn #ident(#self_param, #(#parameters),*) #return_type; + } + }) + }) + .collect::>>()?; + + blocks.cxx_mod_contents.append(&mut bridges); + Ok(blocks) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + generator::naming::qobject::tests::create_qobjectname, + syntax::safety::Safety, + tests::{assert_tokens_eq, tokens_to_syn}, + }; + use syn::ForeignItemFn; + + fn generate_from_foreign( + tokens: proc_macro2::TokenStream, + safety: Safety, + ) -> Result { + let method: ForeignItemFn = tokens_to_syn(tokens); + let inherited_methods = vec![ParsedInheritedMethod::parse(method, safety).unwrap()]; + generate(&create_qobjectname(), &*inherited_methods) + } + + #[test] + fn test_mutable() { + let generated = generate_from_foreign( + quote! { + fn test(self: Pin<&mut qobject::MyObject>, a: B, b: C); + }, + Safety::Safe, + ) + .unwrap(); + + assert_eq!(generated.cxx_mod_contents.len(), 1); + assert_eq!(generated.cxx_qt_mod_contents.len(), 0); + + assert_tokens_eq( + &generated.cxx_mod_contents[0], + quote! { + unsafe extern "C++" { + #[cxx_name="testCxxqtInherit"] + fn test(self: Pin<&mut MyObjectQt>, a: B, b: C); + } + }, + ); + } + + #[test] + fn test_immutable() { + let generated = generate_from_foreign( + quote! { + fn test(self: &qobject::MyObject, a: B, b: C); + }, + Safety::Safe, + ) + .unwrap(); + + assert_eq!(generated.cxx_mod_contents.len(), 1); + assert_eq!(generated.cxx_qt_mod_contents.len(), 0); + + assert_tokens_eq( + &generated.cxx_mod_contents[0], + quote! { + unsafe extern "C++" { + #[cxx_name="testCxxqtInherit"] + fn test(self: &MyObjectQt, a: B, b: C); + } + }, + ); + } + + #[test] + fn test_unsafe() { + let generated = generate_from_foreign( + quote! { + unsafe fn test(self: &qobject::MyObject); + }, + Safety::Unsafe, + ) + .unwrap(); + + assert_eq!(generated.cxx_mod_contents.len(), 1); + assert_eq!(generated.cxx_qt_mod_contents.len(), 0); + + assert_tokens_eq( + &generated.cxx_mod_contents[0], + // TODO: Maybe remove the trailing comma after self? + quote! { + extern "C++" { + #[cxx_name="testCxxqtInherit"] + unsafe fn test(self: &MyObjectQt,); + } + }, + ); + } +} diff --git a/crates/cxx-qt-gen/src/generator/rust/mod.rs b/crates/cxx-qt-gen/src/generator/rust/mod.rs index 968e2576b..4ef34d92a 100644 --- a/crates/cxx-qt-gen/src/generator/rust/mod.rs +++ b/crates/cxx-qt-gen/src/generator/rust/mod.rs @@ -5,6 +5,7 @@ pub mod field; pub mod fragment; +pub mod inherit; pub mod invokable; pub mod property; pub mod qobject; diff --git a/crates/cxx-qt-gen/src/generator/rust/qobject.rs b/crates/cxx-qt-gen/src/generator/rust/qobject.rs index a05bd03da..fcae71c00 100644 --- a/crates/cxx-qt-gen/src/generator/rust/qobject.rs +++ b/crates/cxx-qt-gen/src/generator/rust/qobject.rs @@ -7,14 +7,13 @@ use crate::{ generator::{ naming::{namespace::NamespaceName, qobject::QObjectName}, rust::{ - field::generate_rust_fields, fragment::RustFragmentPair, + field::generate_rust_fields, fragment::RustFragmentPair, inherit, invokable::generate_rust_invokables, property::generate_rust_properties, signals::generate_rust_signals, }, }, - parser::{inherit::ParsedInheritedMethod, qobject::ParsedQObject}, + parser::qobject::ParsedQObject, }; -use proc_macro2::TokenStream; use quote::quote; use syn::{Ident, ImplItemMethod, Item, Result}; @@ -91,7 +90,7 @@ impl GeneratedRustQObject { generated .blocks .append(&mut generate_methods(&qobject.methods, &qobject_idents)?); - generated.blocks.append(&mut generate_inherited_methods( + generated.blocks.append(&mut inherit::generate( &qobject_idents, &qobject.inherited_methods, )?); @@ -148,52 +147,6 @@ pub fn generate_methods( Ok(blocks) } -fn generate_inherited_methods( - qobject_ident: &QObjectName, - methods: &[ParsedInheritedMethod], -) -> Result { - let mut blocks = GeneratedRustQObjectBlocks::default(); - let qobject_name = &qobject_ident.cpp_class.rust; - - let mut bridges = methods - .iter() - .map(|method| { - let parameters = method - .parameters - .iter() - .map(|parameter| { - let ident = ¶meter.ident; - let ty = ¶meter.ty; - quote! { #ident: #ty } - }) - .collect::>(); - let ident = &method.method.sig.ident; - let cxx_name_string = &method.wrapper_ident.to_string(); - let self_param = if method.mutable { - quote! { self: Pin<&mut #qobject_name> } - } else { - quote! { self: &#qobject_name } - }; - let return_type = &method.method.sig.output; - - let mut unsafe_block = None; - let mut unsafe_call = Some(quote! { unsafe }); - if method.safe { - std::mem::swap(&mut unsafe_call, &mut unsafe_block); - } - syn::parse2(quote! { - #unsafe_block extern "C++" { - #[cxx_name=#cxx_name_string] - #unsafe_call fn #ident(#self_param, #(#parameters),*) #return_type; - } - }) - }) - .collect::>>()?; - - blocks.cxx_mod_contents.append(&mut bridges); - Ok(blocks) -} - /// Generate the C++ and Rust CXX definitions for the QObject fn generate_qobject_definitions( qobject_idents: &QObjectName, diff --git a/crates/cxx-qt-gen/src/parser/cxxqtdata.rs b/crates/cxx-qt-gen/src/parser/cxxqtdata.rs index 7881a6454..0e061df56 100644 --- a/crates/cxx-qt-gen/src/parser/cxxqtdata.rs +++ b/crates/cxx-qt-gen/src/parser/cxxqtdata.rs @@ -217,11 +217,7 @@ impl ParsedCxxQtData { fn add_inherited_methods(&mut self, inherited: InheritMethods) -> Result<()> { for method in inherited.base_functions.into_iter() { - let parsed_inherited_method = if inherited.is_safe { - ParsedInheritedMethod::parse_safe(method)? - } else { - ParsedInheritedMethod::parse_unsafe(method)? - }; + let parsed_inherited_method = ParsedInheritedMethod::parse(method, inherited.safety)?; if let Some(ref mut qobject) = self .qobjects diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs index 256f7a83e..d987cbc22 100644 --- a/crates/cxx-qt-gen/src/parser/inherit.rs +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -8,7 +8,9 @@ use crate::{ parser::parameter::ParsedFunctionParameter, syntax::{ attribute::{attribute_find_path, attribute_tokens_to_value}, - foreignmod, types, + foreignmod, + safety::Safety, + types, }, }; use quote::format_ident; @@ -46,7 +48,7 @@ impl Parse for MaybeInheritMethods { /// This type is used when parsing the `#[cxx_qt::inherit]` macro contents into raw ForeignItemFn items pub struct InheritMethods { - pub is_safe: bool, + pub safety: Safety, pub base_functions: Vec, } @@ -54,11 +56,14 @@ impl Parse for InheritMethods { fn parse(input: ParseStream) -> Result { let mut base_functions = Vec::new(); - // base_safe_functions.push(input.parse::()?); // This looks somewhat counter-intuitive, but if we add `unsafe` // to the `extern "C++"` block, the contained functions will be safe to call. - let is_safe = input.peek(Token![unsafe]); - if is_safe { + let safety = if input.peek(Token![unsafe]) { + Safety::Safe + } else { + Safety::Unsafe + }; + if safety == Safety::Safe { input.parse::()?; } @@ -85,7 +90,7 @@ impl Parse for InheritMethods { } Ok(InheritMethods { - is_safe, + safety, base_functions, }) } @@ -105,22 +110,17 @@ pub struct ParsedInheritedMethod { pub parameters: Vec, /// the name of the function in Rust, as well as C++ pub ident: CombinedIdent, - /// the name of the wrapper function in C++ - pub wrapper_ident: Ident, } impl ParsedInheritedMethod { - pub fn parse_unsafe(method: ForeignItemFn) -> Result { - if method.sig.unsafety.is_none() { + pub fn parse(method: ForeignItemFn, safety: Safety) -> Result { + 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!", )); } - Self::parse_safe(method) - } - pub fn parse_safe(method: ForeignItemFn) -> Result { let self_receiver = foreignmod::self_type_from_foreign_fn(&method.sig)?; let (qobject_ident, mutability) = types::extract_qobject_ident(&self_receiver.typ)?; let mutable = mutability.is_some(); @@ -140,7 +140,6 @@ impl ParsedInheritedMethod { ident.cpp = format_ident!("{}", name.value()); } - let wrapper_ident = format_ident!("{}_cxxqt_inherit", &ident.cpp); let safe = method.sig.unsafety.is_none(); Ok(Self { @@ -149,8 +148,125 @@ impl ParsedInheritedMethod { mutable, parameters, ident, - wrapper_ident, safe, }) } + + /// the name of the wrapper function in C++ + pub fn wrapper_ident(&self) -> Ident { + format_ident!("{}CxxqtInherit", self.ident.cpp) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tests::tokens_to_syn; + use quote::quote; + + #[test] + fn test_parse_unsafe_mod() { + let module = quote! { + extern "C++" { + unsafe fn test(self: &qobject::T); + } + }; + let parsed: InheritMethods = syn::parse2(module).unwrap(); + assert_eq!(parsed.base_functions.len(), 1); + assert_eq!(parsed.safety, Safety::Unsafe); + } + + #[test] + fn test_parse_safe_mod() { + let module = quote! { + #[cxx_qt::inherit] + unsafe extern "C++" { + fn test(self: &qobject::T); + unsafe fn test2(self: &qobject::T); + } + }; + let parsed: MaybeInheritMethods = syn::parse2(module).unwrap(); + match parsed { + MaybeInheritMethods::Found(inherit) => { + assert_eq!(inherit.base_functions.len(), 2); + assert_eq!(inherit.safety, Safety::Safe); + } + MaybeInheritMethods::PassThrough(item) => { + panic!("Expected InheritMethods, got {:?}", item); + } + } + } + + fn assert_parse_error(tokens: proc_macro2::TokenStream) { + let function: ForeignItemFn = tokens_to_syn(tokens); + + let result = ParsedInheritedMethod::parse(function, Safety::Safe); + assert!(result.is_err()); + } + + #[test] + fn test_parser_errors() { + // Missing self type + assert_parse_error(quote! { + fn test(&self); + }); + // Missing qobject:: + assert_parse_error(quote! { + fn test(self: &T); + }); + assert_parse_error(quote! { + fn test(self: &mut T); + }); + assert_parse_error(quote! { + fn test(self: Pin<&mut T>); + }); + // Pointer types + assert_parse_error(quote! { + fn test(self: *const T); + }); + assert_parse_error(quote! { + fn test(self: *mut T); + }); + // Invalid pin usage + assert_parse_error(quote! { + fn test(self: Pin<&T>); + }); + assert_parse_error(quote! { + fn test(self: &mut T); + }); + // Attributes + assert_parse_error(quote! { + #[myattribute] + fn test(self: &qobject::T); + }); + assert_parse_error(quote! { + fn test(#[test] self: &qobject::T); + }); + // Missing "unsafe" + let function: ForeignItemFn = tokens_to_syn(quote! { + fn test(self: &qobject::T); + }); + assert!(ParsedInheritedMethod::parse(function, Safety::Unsafe).is_err()); + } + + #[test] + fn test_parse_safe() { + let function: ForeignItemFn = tokens_to_syn(quote! { + #[cxx_name="testFunction"] + fn test(self: Pin<&mut qobject::T>, a: i32, b: &str); + }); + + let parsed = ParsedInheritedMethod::parse(function, Safety::Safe).unwrap(); + + assert_eq!(parsed.qobject_ident, format_ident!("T")); + assert_eq!(parsed.mutable, true); + assert_eq!(parsed.parameters.len(), 2); + assert_eq!(parsed.ident.rust, format_ident!("test")); + assert_eq!(parsed.ident.cpp, format_ident!("testFunction")); + assert_eq!(parsed.safe, true); + assert_eq!( + parsed.wrapper_ident(), + format_ident!("testFunctionCxxqtInherit") + ); + } } diff --git a/crates/cxx-qt-gen/src/parser/parameter.rs b/crates/cxx-qt-gen/src/parser/parameter.rs index 764b911ea..31f88d16d 100644 --- a/crates/cxx-qt-gen/src/parser/parameter.rs +++ b/crates/cxx-qt-gen/src/parser/parameter.rs @@ -101,3 +101,61 @@ impl ParsedFunctionParameter { }) } } + +#[cfg(test)] +mod tests { + use quote::ToTokens; + use syn::ForeignItemFn; + + use super::*; + + #[test] + fn test_parse_all_without_receiver() { + let function: ForeignItemFn = syn::parse_quote! { + fn foo(&self, a: i32, b: String); + }; + + let parameters = + ParsedFunctionParameter::parse_all_without_receiver(&function.sig).unwrap(); + assert_eq!(parameters.len(), 2); + assert_eq!(parameters[0].ident, "a"); + assert_eq!(parameters[0].ty.to_token_stream().to_string(), "i32"); + assert_eq!(parameters[1].ident, "b"); + assert_eq!(parameters[1].ty.to_token_stream().to_string(), "String"); + } + + #[test] + fn test_parse_all_without_receiver_invalid_self() { + fn assert_parse_error(function: ForeignItemFn) { + assert!(ParsedFunctionParameter::parse_all_without_receiver(&function.sig).is_err()); + } + // Missing self + assert_parse_error(syn::parse_quote! { + fn foo(a: i32, b: String); + }); + // self parameter points to non-self type + assert_parse_error(syn::parse_quote! { + fn foo(self: T); + }); + // self parameter is a non-self pin + assert_parse_error(syn::parse_quote! { + fn foo(self: Pin<&mut T>); + }) + } + + #[test] + fn test_parse_all_ignoring_receiver() { + // This supports using a type as `self` that's not "Self". + let function: ForeignItemFn = syn::parse_quote! { + fn foo(self: T, a: i32, b: String); + }; + + let parameters = + ParsedFunctionParameter::parse_all_ignoring_receiver(&function.sig).unwrap(); + assert_eq!(parameters.len(), 2); + assert_eq!(parameters[0].ident, "a"); + assert_eq!(parameters[0].ty.to_token_stream().to_string(), "i32"); + assert_eq!(parameters[1].ident, "b"); + assert_eq!(parameters[1].ty.to_token_stream().to_string(), "String"); + } +} diff --git a/crates/cxx-qt-gen/src/parser/qobject.rs b/crates/cxx-qt-gen/src/parser/qobject.rs index 69e546f10..3dafeab16 100644 --- a/crates/cxx-qt-gen/src/parser/qobject.rs +++ b/crates/cxx-qt-gen/src/parser/qobject.rs @@ -4,7 +4,7 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 use crate::parser::{ - inherit::*, + inherit::ParsedInheritedMethod, invokable::ParsedQInvokable, property::{ParsedQProperty, ParsedRustField}, signals::ParsedSignalsEnum, diff --git a/crates/cxx-qt-gen/src/syntax/mod.rs b/crates/cxx-qt-gen/src/syntax/mod.rs index 69dc2e02d..d4eb1fd89 100644 --- a/crates/cxx-qt-gen/src/syntax/mod.rs +++ b/crates/cxx-qt-gen/src/syntax/mod.rs @@ -10,6 +10,7 @@ pub mod implitemmethod; pub mod path; mod qtfile; mod qtitem; +pub mod safety; pub mod types; pub use qtfile::{parse_qt_file, CxxQtFile}; diff --git a/crates/cxx-qt-gen/src/syntax/safety.rs b/crates/cxx-qt-gen/src/syntax/safety.rs new file mode 100644 index 000000000..7a0d763a3 --- /dev/null +++ b/crates/cxx-qt-gen/src/syntax/safety.rs @@ -0,0 +1,5 @@ +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum Safety { + Safe, + Unsafe, +} diff --git a/crates/cxx-qt-gen/src/syntax/types.rs b/crates/cxx-qt-gen/src/syntax/types.rs index 62d84a1de..03d9d8ffc 100644 --- a/crates/cxx-qt-gen/src/syntax/types.rs +++ b/crates/cxx-qt-gen/src/syntax/types.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::syntax::path::*; +use crate::syntax::path::path_compare_str; use syn::{ token::Mut, Error, GenericArgument, Ident, Path, PathArguments, Result, Type, TypePath, TypeReference, diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.h b/crates/cxx-qt-gen/test_outputs/inheritance.h index 8462a26d6..2fa1510ab 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.h +++ b/crates/cxx-qt-gen/test_outputs/inheritance.h @@ -29,12 +29,12 @@ class MyObject : public QAbstractItemModel ::std::int32_t _role) const override; Q_INVOKABLE bool hasChildren(QModelIndex const& _parent) const override; template - bool hasChildren_cxxqt_inherit(Args... args) const + bool hasChildrenCxxqtInherit(Args... args) const { return QAbstractItemModel::hasChildren(args...); } template - void fetchMore_cxxqt_inherit(Args... args) + void fetchMoreCxxqtInherit(Args... args) { return QAbstractItemModel::fetchMore(args...); } diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.rs b/crates/cxx-qt-gen/test_outputs/inheritance.rs index 92bac9372..b7d640f03 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.rs +++ b/crates/cxx-qt-gen/test_outputs/inheritance.rs @@ -38,11 +38,11 @@ mod inheritance { fn has_children_wrapper(self: &MyObject, cpp: &MyObjectQt, _parent: &QModelIndex) -> bool; } unsafe extern "C++" { - #[cxx_name = "hasChildren_cxxqt_inherit"] + #[cxx_name = "hasChildrenCxxqtInherit"] fn has_children_super(self: &MyObjectQt, parent: &QModelIndex) -> bool; } extern "C++" { - #[cxx_name = "fetchMore_cxxqt_inherit"] + #[cxx_name = "fetchMoreCxxqtInherit"] unsafe fn fetch_more(self: Pin<&mut MyObjectQt>, index: &QModelIndex); } unsafe extern "C++" { From 861820f32505dcd889eb257c4bc2ec0ac210d9ed Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Wed, 22 Feb 2023 15:36:32 +0100 Subject: [PATCH 11/14] Move naming functions onto CombinedIdent --- .../src/generator/naming/functions.rs | 37 ++++++++++ .../src/generator/naming/invokable.rs | 16 ++-- crates/cxx-qt-gen/src/generator/naming/mod.rs | 35 +-------- .../src/generator/naming/property.rs | 74 ++++++++++--------- .../src/generator/naming/qobject.rs | 30 ++++---- .../src/generator/naming/signals.rs | 34 +++++---- 6 files changed, 118 insertions(+), 108 deletions(-) create mode 100644 crates/cxx-qt-gen/src/generator/naming/functions.rs diff --git a/crates/cxx-qt-gen/src/generator/naming/functions.rs b/crates/cxx-qt-gen/src/generator/naming/functions.rs new file mode 100644 index 000000000..b0eab5c04 --- /dev/null +++ b/crates/cxx-qt-gen/src/generator/naming/functions.rs @@ -0,0 +1,37 @@ +use convert_case::{Case, Casing}; +use quote::format_ident; +use syn::Ident; + +use super::*; + +impl CombinedIdent { + /// Generate a CombinedIdent from a rust function name. + /// C++ will use the CamelCase version of the function name. + pub fn from_rust_function(ident: Ident) -> Self { + Self { + cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), + rust: ident, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_from_rust_function_camel_case_conversion() { + let ident = format_ident!("test_function"); + let combined = CombinedIdent::from_rust_function(ident.clone()); + assert_eq!(combined.cpp, format_ident!("testFunction")); + assert_eq!(combined.rust, ident); + } + + #[test] + fn test_from_rust_function_single_word() { + let ident = format_ident!("test"); + let combined = CombinedIdent::from_rust_function(ident.clone()); + assert_eq!(combined.cpp, ident); + assert_eq!(combined.rust, ident); + } +} diff --git a/crates/cxx-qt-gen/src/generator/naming/invokable.rs b/crates/cxx-qt-gen/src/generator/naming/invokable.rs index e8787b175..2305a8f51 100644 --- a/crates/cxx-qt-gen/src/generator/naming/invokable.rs +++ b/crates/cxx-qt-gen/src/generator/naming/invokable.rs @@ -24,17 +24,19 @@ impl From<&ImplItemMethod> for QInvokableName { let ident = &method.sig.ident; Self { name: CombinedIdent::from_rust_function(ident.clone()), - wrapper: wrapper_from_ident(ident), + wrapper: CombinedIdent::wrapper_from_invokable(&ident), } } } -/// For a given ident generate the Rust and C++ wrapper names -fn wrapper_from_ident(ident: &Ident) -> CombinedIdent { - let ident = format_ident!("{ident}_wrapper"); - CombinedIdent { - cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), - rust: ident, +impl CombinedIdent { + /// For a given ident generate the Rust and C++ wrapper names + fn wrapper_from_invokable(ident: &Ident) -> Self { + let ident = format_ident!("{ident}_wrapper"); + Self { + cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), + rust: ident, + } } } diff --git a/crates/cxx-qt-gen/src/generator/naming/mod.rs b/crates/cxx-qt-gen/src/generator/naming/mod.rs index 08abd43f8..8952dfd73 100644 --- a/crates/cxx-qt-gen/src/generator/naming/mod.rs +++ b/crates/cxx-qt-gen/src/generator/naming/mod.rs @@ -2,14 +2,13 @@ // SPDX-FileContributor: Andrew Hayzen // // SPDX-License-Identifier: MIT OR Apache-2.0 +pub mod functions; pub mod invokable; pub mod namespace; pub mod property; pub mod qobject; pub mod signals; -use convert_case::{Case, Casing}; -use quote::format_ident; use syn::Ident; /// Describes an ident which potentially has a different name in C++ and Rust @@ -19,35 +18,3 @@ pub struct CombinedIdent { /// The ident for rust pub rust: Ident, } - -impl CombinedIdent { - /// Generate a CombinedIdent from a rust function name. - /// C++ will use the CamelCase version of the function name. - pub fn from_rust_function(ident: Ident) -> Self { - Self { - cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), - rust: ident, - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_from_rust_function_camel_case_conversion() { - let ident = format_ident!("test_function"); - let combined = CombinedIdent::from_rust_function(ident.clone()); - assert_eq!(combined.cpp, format_ident!("testFunction")); - assert_eq!(combined.rust, ident); - } - - #[test] - fn test_from_rust_function_single_word() { - let ident = format_ident!("test"); - let combined = CombinedIdent::from_rust_function(ident.clone()); - assert_eq!(combined.cpp, ident); - assert_eq!(combined.rust, ident); - } -} diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs index c34db6550..daee78986 100644 --- a/crates/cxx-qt-gen/src/generator/naming/property.rs +++ b/crates/cxx-qt-gen/src/generator/naming/property.rs @@ -20,11 +20,11 @@ pub struct QPropertyName { impl From<&Ident> for QPropertyName { fn from(ident: &Ident) -> Self { Self { - name: name_from_ident(ident), - getter: getter_from_ident(ident), - getter_mutable: getter_mutable_from_ident(ident), - setter: setter_from_ident(ident), - notify: notify_from_ident(ident), + name: CombinedIdent::from_property(ident.clone()), + getter: CombinedIdent::getter_from_property(ident.clone()), + getter_mutable: CombinedIdent::getter_mutable_from_property(ident), + setter: CombinedIdent::setter_from_property(ident), + notify: CombinedIdent::notify_from_property(ident), } } } @@ -35,45 +35,47 @@ impl From<&ParsedQProperty> for QPropertyName { } } -/// For a given ident generate the Rust and C++ getter names -fn getter_from_ident(ident: &Ident) -> CombinedIdent { - CombinedIdent { - cpp: format_ident!("get{}", ident.to_string().to_case(Case::Pascal)), - rust: ident.clone(), +impl CombinedIdent { + /// For a given ident generate the Rust and C++ getter names + fn getter_from_property(ident: Ident) -> Self { + Self { + cpp: format_ident!("get{}", ident.to_string().to_case(Case::Pascal)), + rust: ident, + } } -} -/// For a given ident generate the Rust and C++ getter mutable names -fn getter_mutable_from_ident(ident: &Ident) -> CombinedIdent { - CombinedIdent { - cpp: format_ident!("get{}Mut", ident.to_string().to_case(Case::Pascal)), - rust: format_ident!("{ident}_mut"), + /// For a given ident generate the Rust and C++ getter mutable names + fn getter_mutable_from_property(ident: &Ident) -> Self { + Self { + cpp: format_ident!("get{}Mut", ident.to_string().to_case(Case::Pascal)), + rust: format_ident!("{ident}_mut"), + } } -} -/// For a given ident generate the Rust and C++ names -fn name_from_ident(ident: &Ident) -> CombinedIdent { - CombinedIdent { - cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), - rust: ident.clone(), + /// For a given ident generate the Rust and C++ names + fn from_property(ident: Ident) -> Self { + Self { + cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), + rust: ident, + } } -} -/// For a given ident generate the Rust and C++ notify names -fn notify_from_ident(ident: &Ident) -> CombinedIdent { - let ident = format_ident!("{ident}_changed"); - CombinedIdent { - cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), - rust: ident, + /// For a given ident generate the Rust and C++ notify names + fn notify_from_property(ident: &Ident) -> Self { + let ident = format_ident!("{ident}_changed"); + Self { + cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), + rust: ident, + } } -} -/// For a given ident generate the Rust and C++ setter names -fn setter_from_ident(ident: &Ident) -> CombinedIdent { - let ident = format_ident!("set_{ident}"); - CombinedIdent { - cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), - rust: ident, + /// For a given ident generate the Rust and C++ setter names + fn setter_from_property(ident: &Ident) -> Self { + let ident = format_ident!("set_{ident}"); + Self { + cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), + rust: ident, + } } } diff --git a/crates/cxx-qt-gen/src/generator/naming/qobject.rs b/crates/cxx-qt-gen/src/generator/naming/qobject.rs index 4039bd671..dd53b6c37 100644 --- a/crates/cxx-qt-gen/src/generator/naming/qobject.rs +++ b/crates/cxx-qt-gen/src/generator/naming/qobject.rs @@ -28,22 +28,14 @@ impl From<&ParsedQObject> for QObjectName { impl From<&Ident> for QObjectName { fn from(ident: &Ident) -> Self { Self { - cpp_class: cpp_class_from_ident(ident), - rust_struct: rust_struct_from_ident(ident), + cpp_class: CombinedIdent::cpp_class_from_rust_struct(ident.clone()), + rust_struct: CombinedIdent::from_rust_struct(ident.clone()), cxx_qt_thread_class: cxx_qt_thread_class_from_ident(ident), cxx_qt_thread_queued_fn_struct: cxx_qt_thread_queued_fn_struct_from_ident(ident), } } } -/// For a given ident generate the C++ class name -fn cpp_class_from_ident(ident: &Ident) -> CombinedIdent { - CombinedIdent { - cpp: ident.clone(), - rust: format_ident!("{ident}Qt"), - } -} - /// For a given ident generate the CxxQtThread ident fn cxx_qt_thread_class_from_ident(ident: &Ident) -> Ident { format_ident!("{ident}CxxQtThread") @@ -54,11 +46,19 @@ fn cxx_qt_thread_queued_fn_struct_from_ident(ident: &Ident) -> Ident { format_ident!("{ident}CxxQtThreadQueuedFn") } -/// For a given ident generate the Rust and C++ names -fn rust_struct_from_ident(ident: &Ident) -> CombinedIdent { - CombinedIdent { - cpp: format_ident!("{ident}Rust"), - rust: ident.clone(), +impl CombinedIdent { + /// For a given ident generate the C++ class name + fn cpp_class_from_rust_struct(ident: Ident) -> Self { + let rust = format_ident!("{ident}Qt"); + Self { cpp: ident, rust } + } + + /// For a given ident generate the Rust and C++ names + fn from_rust_struct(ident: Ident) -> Self { + Self { + cpp: format_ident!("{ident}Rust"), + rust: ident, + } } } diff --git a/crates/cxx-qt-gen/src/generator/naming/signals.rs b/crates/cxx-qt-gen/src/generator/naming/signals.rs index 3dca82654..607260560 100644 --- a/crates/cxx-qt-gen/src/generator/naming/signals.rs +++ b/crates/cxx-qt-gen/src/generator/naming/signals.rs @@ -26,28 +26,30 @@ impl From<&ParsedSignal> for QSignalName { Self { enum_name: signal.ident.clone(), - name: name_from_ident(&cxx_ident), - emit_name: emit_name_from_ident(&cxx_ident), + name: CombinedIdent::from_signal(&cxx_ident), + emit_name: CombinedIdent::emit_from_signal(&cxx_ident), } } } -/// For a given signal ident generate the Rust and C++ names -fn name_from_ident(ident: &Ident) -> CombinedIdent { - CombinedIdent { - cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), - // Note that signal names are in camel case so we need to convert to snake and can't clone - rust: format_ident!("{}", ident.to_string().to_case(Case::Snake)), +impl CombinedIdent { + /// For a given signal ident generate the Rust and C++ names + fn from_signal(ident: &Ident) -> Self { + Self { + cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), + // Note that signal names are in camel case so we need to convert to snake and can't clone + rust: format_ident!("{}", ident.to_string().to_case(Case::Snake)), + } } -} -/// For a given signal ident generate the Rust and C++ emit name -fn emit_name_from_ident(ident: &Ident) -> CombinedIdent { - // Note that signal names are in camel case so we need to convert to snake first - let ident = format_ident!("emit_{}", ident.to_string().to_case(Case::Snake)); - CombinedIdent { - cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), - rust: ident, + /// For a given signal ident generate the Rust and C++ emit name + fn emit_from_signal(ident: &Ident) -> Self { + // Note that signal names are in camel case so we need to convert to snake first + let ident = format_ident!("emit_{}", ident.to_string().to_case(Case::Snake)); + Self { + cpp: format_ident!("{}", ident.to_string().to_case(Case::Camel)), + rust: ident, + } } } From 273ec1b208a1d57e5c5b01252839453dd4467fb0 Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Wed, 22 Feb 2023 17:12:23 +0100 Subject: [PATCH 12/14] Fix CI failures - Clippy lints - SPDX licenses --- crates/cxx-qt-gen/src/generator/cpp/inherit.rs | 2 +- crates/cxx-qt-gen/src/generator/cpp/qobject.rs | 2 +- crates/cxx-qt-gen/src/generator/naming/functions.rs | 5 +++++ crates/cxx-qt-gen/src/generator/naming/invokable.rs | 2 +- crates/cxx-qt-gen/src/generator/rust/inherit.rs | 7 ++++++- crates/cxx-qt-gen/src/parser/inherit.rs | 6 +++--- crates/cxx-qt-gen/src/syntax/safety.rs | 5 +++++ 7 files changed, 22 insertions(+), 7 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs index 45f2e6f09..99a2b68ae 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs @@ -69,7 +69,7 @@ mod tests { let inherited_methods = vec![ParsedInheritedMethod::parse(method, Safety::Safe).unwrap()]; let base_class = base_class.map(|s| s.to_owned()); generate( - &*inherited_methods, + &inherited_methods, &base_class, &ParsedCxxMappings::default(), ) diff --git a/crates/cxx-qt-gen/src/generator/cpp/qobject.rs b/crates/cxx-qt-gen/src/generator/cpp/qobject.rs index a69ad6590..23341b7b6 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/qobject.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/qobject.rs @@ -108,7 +108,7 @@ impl GeneratedCppQObject { )?); } generated.blocks.append(&mut inherit::generate( - &*qobject.inherited_methods, + &qobject.inherited_methods, &qobject.base_class, cxx_mappings, )?); diff --git a/crates/cxx-qt-gen/src/generator/naming/functions.rs b/crates/cxx-qt-gen/src/generator/naming/functions.rs index b0eab5c04..b5ca2161e 100644 --- a/crates/cxx-qt-gen/src/generator/naming/functions.rs +++ b/crates/cxx-qt-gen/src/generator/naming/functions.rs @@ -1,3 +1,8 @@ +// SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company +// SPDX-FileContributor: Leon Matthes + +// SPDX-License-Identifier: MIT OR Apache-2.0 + use convert_case::{Case, Casing}; use quote::format_ident; use syn::Ident; diff --git a/crates/cxx-qt-gen/src/generator/naming/invokable.rs b/crates/cxx-qt-gen/src/generator/naming/invokable.rs index 2305a8f51..ee7061fdc 100644 --- a/crates/cxx-qt-gen/src/generator/naming/invokable.rs +++ b/crates/cxx-qt-gen/src/generator/naming/invokable.rs @@ -24,7 +24,7 @@ impl From<&ImplItemMethod> for QInvokableName { let ident = &method.sig.ident; Self { name: CombinedIdent::from_rust_function(ident.clone()), - wrapper: CombinedIdent::wrapper_from_invokable(&ident), + wrapper: CombinedIdent::wrapper_from_invokable(ident), } } } diff --git a/crates/cxx-qt-gen/src/generator/rust/inherit.rs b/crates/cxx-qt-gen/src/generator/rust/inherit.rs index 64b254250..99ec45898 100644 --- a/crates/cxx-qt-gen/src/generator/rust/inherit.rs +++ b/crates/cxx-qt-gen/src/generator/rust/inherit.rs @@ -1,3 +1,8 @@ +// SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company +// SPDX-FileContributor: Leon Matthes + +// SPDX-License-Identifier: MIT OR Apache-2.0 + use crate::{ generator::{naming::qobject::QObjectName, rust::qobject::GeneratedRustQObjectBlocks}, parser::inherit::ParsedInheritedMethod, @@ -68,7 +73,7 @@ mod tests { ) -> Result { let method: ForeignItemFn = tokens_to_syn(tokens); let inherited_methods = vec![ParsedInheritedMethod::parse(method, safety).unwrap()]; - generate(&create_qobjectname(), &*inherited_methods) + generate(&create_qobjectname(), &inherited_methods) } #[test] diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs index d987cbc22..be4a08087 100644 --- a/crates/cxx-qt-gen/src/parser/inherit.rs +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -192,7 +192,7 @@ mod tests { assert_eq!(inherit.safety, Safety::Safe); } MaybeInheritMethods::PassThrough(item) => { - panic!("Expected InheritMethods, got {:?}", item); + panic!("Expected InheritMethods, got {item:?}"); } } } @@ -259,14 +259,14 @@ mod tests { let parsed = ParsedInheritedMethod::parse(function, Safety::Safe).unwrap(); assert_eq!(parsed.qobject_ident, format_ident!("T")); - assert_eq!(parsed.mutable, true); assert_eq!(parsed.parameters.len(), 2); assert_eq!(parsed.ident.rust, format_ident!("test")); assert_eq!(parsed.ident.cpp, format_ident!("testFunction")); - assert_eq!(parsed.safe, true); assert_eq!( parsed.wrapper_ident(), format_ident!("testFunctionCxxqtInherit") ); + assert!(parsed.mutable); + assert!(parsed.safe); } } diff --git a/crates/cxx-qt-gen/src/syntax/safety.rs b/crates/cxx-qt-gen/src/syntax/safety.rs index 7a0d763a3..9ac4d5e40 100644 --- a/crates/cxx-qt-gen/src/syntax/safety.rs +++ b/crates/cxx-qt-gen/src/syntax/safety.rs @@ -1,3 +1,8 @@ +// SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company +// SPDX-FileContributor: Leon Matthes + +// SPDX-License-Identifier: MIT OR Apache-2.0 + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum Safety { Safe, From 86e1da601e915754d46e06296b9dd6b50e5cbc1d Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Thu, 23 Feb 2023 10:03:17 +0100 Subject: [PATCH 13/14] Improve wording in book Co-authored-by: Be --- book/src/concepts/inheritance.md | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/book/src/concepts/inheritance.md b/book/src/concepts/inheritance.md index 7ec9169a6..073043df1 100644 --- a/book/src/concepts/inheritance.md +++ b/book/src/concepts/inheritance.md @@ -7,8 +7,7 @@ SPDX-License-Identifier: MIT OR Apache-2.0 # Inheritance -Some Qt APIs require you to override certain methods from an abstract base class. -Specifically [QAbstractItemModel](https://doc.qt.io/qt-6/qabstractitemmodel.html). +Some Qt APIs require you to override certain methods from an abstract base class, for example [QAbstractItemModel](https://doc.qt.io/qt-6/qabstractitemmodel.html). To support creating such subclasses directly from within Rust, CXX-Qt provides you with multiple helpers. @@ -28,12 +27,11 @@ It can be placed in front of an `extern "C++"` block in a `#[cxx_qt::bridge]`. [Full example](https://github.com/KDAB/cxx-qt/blob/main/examples/qml_features/rust/src/custom_base_class.rs) This code implements a QAbstractListModel subclass. -For this, access to the `beginResetModel` and related methods is required. +For this, the `clear` method implemented in Rust needs to call `beginResetModel` and related methods from the base class, which are made accessible by using `#[cxx_qt::inherit]`. See [the Qt docs](https://doc.qt.io/qt-6/qabstractlistmodel.html) for more details on the specific subclassing requirements. -These methods are then made accessible by using `#[cxx_qt::inherit]`. Methods can be declared inside `#[cxx_qt::inherit]` in `extern "C++"` blocks similar to CXX, with the same restrictions regarding which types can be used. -Additionally, the `self` type must be either `self: Pin<&mut qobject::T>` or `self: &qobject::T`. Where `qobject::T` may refer to any valid `#[cxx_qt::qobject]` in the `#[cxx_qt::bridge]` +Additionally, the `self` type must be either `self: Pin<&mut qobject::T>` or `self: &qobject::T`, where `qobject::T` must refer to a QObject marked with `#[cxx_qt::qobject]` in the `#[cxx_qt::bridge]` The declared methods will be case-converted as in other CXX-Qt APIs. To explicitly declare the C++ method name, use the `#[cxx_name="myFunctionName"]` attribute. @@ -60,8 +58,8 @@ The below example overrides the [`data`](https://doc.qt.io/qt-6/qabstractitemmod ``` [Full example](https://github.com/KDAB/cxx-qt/blob/main/examples/qml_features/rust/src/custom_base_class.rs) -Note that if a method is overridden using `cxx_override` the base class version of the method can be accessed by using `#[cxx_qt::inherit]` in combination with the `#[cxx_name]` attribute. -In this case the base class version of the function must get a different name, as Rust can't have two functions with the same name on one type. +When a method is overridden using `cxx_override`, the base class version of the method can be accessed by using `#[cxx_qt::inherit]` in combination with the `#[cxx_name]` attribute. +In this case the base class version of the function must get a different name because Rust can't have two functions with the same name on one type. Example: ```rust,ignore From 8cc2d5250547d34cf0c8beb1755074cf422606b2 Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Thu, 23 Feb 2023 10:29:02 +0100 Subject: [PATCH 14/14] Rename CxxqtInherit > CxxQtInherit Also fix wildcard import --- crates/cxx-qt-gen/src/generator/cpp/inherit.rs | 6 +++--- crates/cxx-qt-gen/src/generator/naming/functions.rs | 2 +- crates/cxx-qt-gen/src/generator/rust/inherit.rs | 6 +++--- crates/cxx-qt-gen/src/parser/inherit.rs | 4 ++-- crates/cxx-qt-gen/test_outputs/inheritance.h | 4 ++-- crates/cxx-qt-gen/test_outputs/inheritance.rs | 4 ++-- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs index 99a2b68ae..78e412f2f 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/inherit.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/inherit.rs @@ -97,7 +97,7 @@ mod tests { assert_generated_eq( indoc::indoc! {" template - void testCxxqtInherit(Args ...args) const + void testCxxQtInherit(Args ...args) const { return TestBaseClass::test(args...); }" @@ -119,7 +119,7 @@ mod tests { assert_generated_eq( indoc::indoc! {" template - void testCxxqtInherit(Args ...args) + void testCxxQtInherit(Args ...args) { return TestBaseClass::test(args...); }" @@ -141,7 +141,7 @@ mod tests { assert_generated_eq( indoc::indoc! {" template - void testCxxqtInherit(Args ...args) const + void testCxxQtInherit(Args ...args) const { return QObject::test(args...); }" diff --git a/crates/cxx-qt-gen/src/generator/naming/functions.rs b/crates/cxx-qt-gen/src/generator/naming/functions.rs index b5ca2161e..0ec3de78b 100644 --- a/crates/cxx-qt-gen/src/generator/naming/functions.rs +++ b/crates/cxx-qt-gen/src/generator/naming/functions.rs @@ -7,7 +7,7 @@ use convert_case::{Case, Casing}; use quote::format_ident; use syn::Ident; -use super::*; +use super::CombinedIdent; impl CombinedIdent { /// Generate a CombinedIdent from a rust function name. diff --git a/crates/cxx-qt-gen/src/generator/rust/inherit.rs b/crates/cxx-qt-gen/src/generator/rust/inherit.rs index 99ec45898..2f73a4c04 100644 --- a/crates/cxx-qt-gen/src/generator/rust/inherit.rs +++ b/crates/cxx-qt-gen/src/generator/rust/inherit.rs @@ -93,7 +93,7 @@ mod tests { &generated.cxx_mod_contents[0], quote! { unsafe extern "C++" { - #[cxx_name="testCxxqtInherit"] + #[cxx_name="testCxxQtInherit"] fn test(self: Pin<&mut MyObjectQt>, a: B, b: C); } }, @@ -117,7 +117,7 @@ mod tests { &generated.cxx_mod_contents[0], quote! { unsafe extern "C++" { - #[cxx_name="testCxxqtInherit"] + #[cxx_name="testCxxQtInherit"] fn test(self: &MyObjectQt, a: B, b: C); } }, @@ -142,7 +142,7 @@ mod tests { // TODO: Maybe remove the trailing comma after self? quote! { extern "C++" { - #[cxx_name="testCxxqtInherit"] + #[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 be4a08087..160fc28f7 100644 --- a/crates/cxx-qt-gen/src/parser/inherit.rs +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -154,7 +154,7 @@ impl ParsedInheritedMethod { /// the name of the wrapper function in C++ pub fn wrapper_ident(&self) -> Ident { - format_ident!("{}CxxqtInherit", self.ident.cpp) + format_ident!("{}CxxQtInherit", self.ident.cpp) } } @@ -264,7 +264,7 @@ mod tests { assert_eq!(parsed.ident.cpp, format_ident!("testFunction")); assert_eq!( parsed.wrapper_ident(), - format_ident!("testFunctionCxxqtInherit") + format_ident!("testFunctionCxxQtInherit") ); assert!(parsed.mutable); assert!(parsed.safe); diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.h b/crates/cxx-qt-gen/test_outputs/inheritance.h index 2fa1510ab..9de751ac1 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.h +++ b/crates/cxx-qt-gen/test_outputs/inheritance.h @@ -29,12 +29,12 @@ class MyObject : public QAbstractItemModel ::std::int32_t _role) const override; Q_INVOKABLE bool hasChildren(QModelIndex const& _parent) const override; template - bool hasChildrenCxxqtInherit(Args... args) const + bool hasChildrenCxxQtInherit(Args... args) const { return QAbstractItemModel::hasChildren(args...); } template - void fetchMoreCxxqtInherit(Args... args) + void fetchMoreCxxQtInherit(Args... args) { return QAbstractItemModel::fetchMore(args...); } diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.rs b/crates/cxx-qt-gen/test_outputs/inheritance.rs index b7d640f03..1ac66b300 100644 --- a/crates/cxx-qt-gen/test_outputs/inheritance.rs +++ b/crates/cxx-qt-gen/test_outputs/inheritance.rs @@ -38,11 +38,11 @@ mod inheritance { fn has_children_wrapper(self: &MyObject, cpp: &MyObjectQt, _parent: &QModelIndex) -> bool; } unsafe extern "C++" { - #[cxx_name = "hasChildrenCxxqtInherit"] + #[cxx_name = "hasChildrenCxxQtInherit"] fn has_children_super(self: &MyObjectQt, parent: &QModelIndex) -> bool; } extern "C++" { - #[cxx_name = "fetchMoreCxxqtInherit"] + #[cxx_name = "fetchMoreCxxQtInherit"] unsafe fn fetch_more(self: Pin<&mut MyObjectQt>, index: &QModelIndex); } unsafe extern "C++" {