From dc49d332b5131a62c95ee988897b78476696e73d Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Tue, 9 Jul 2024 16:47:24 +0100 Subject: [PATCH 01/18] Add HashSet of flags to ParsedQPoperty for optional getters and setters --- crates/cxx-qt-gen/Cargo.toml | 6 +- .../src/generator/cpp/property/mod.rs | 3 + .../src/generator/naming/property.rs | 1 + .../src/generator/rust/property/mod.rs | 3 + crates/cxx-qt-gen/src/parser/property.rs | 83 ++++++++++++++++++- 5 files changed, 93 insertions(+), 3 deletions(-) diff --git a/crates/cxx-qt-gen/Cargo.toml b/crates/cxx-qt-gen/Cargo.toml index d0b90eedf..4952752bc 100644 --- a/crates/cxx-qt-gen/Cargo.toml +++ b/crates/cxx-qt-gen/Cargo.toml @@ -6,7 +6,11 @@ [package] name = "cxx-qt-gen" version.workspace = true -authors = ["Andrew Hayzen ", "Gerhard de Clercq ", "Leon Matthes "] +authors = [ + "Andrew Hayzen ", + "Gerhard de Clercq ", + "Leon Matthes ", +] edition.workspace = true license.workspace = true description = "Code generation for integrating `cxx-qt` into higher level tools" diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs index 4a47f17ab..ce3743d82 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs @@ -73,10 +73,12 @@ mod tests { ParsedQProperty { ident: format_ident!("trivial_property"), ty: parse_quote! { i32 }, + flags: Default::default(), }, ParsedQProperty { ident: format_ident!("opaque_property"), ty: parse_quote! { UniquePtr }, + flags: Default::default(), }, ]; let qobject_idents = create_qobjectname(); @@ -358,6 +360,7 @@ mod tests { let properties = vec![ParsedQProperty { ident: format_ident!("mapped_property"), ty: parse_quote! { A }, + flags: Default::default(), }]; let qobject_idents = create_qobjectname(); diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs index 96bd38578..5e577cea8 100644 --- a/crates/cxx-qt-gen/src/generator/naming/property.rs +++ b/crates/cxx-qt-gen/src/generator/naming/property.rs @@ -83,6 +83,7 @@ pub mod tests { let property = ParsedQProperty { ident: format_ident!("my_property"), ty, + flags: Default::default(), }; QPropertyNames::from(&property) } diff --git a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs index ac676a7b0..4026bc3aa 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs @@ -77,14 +77,17 @@ mod tests { ParsedQProperty { ident: format_ident!("trivial_property"), ty: parse_quote! { i32 }, + flags: Default::default(), }, ParsedQProperty { ident: format_ident!("opaque_property"), ty: parse_quote! { UniquePtr }, + flags: Default::default(), }, ParsedQProperty { ident: format_ident!("unsafe_property"), ty: parse_quote! { *mut T }, + flags: Default::default(), }, ]; let qobject_idents = create_qobjectname(); diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index 93d4daf67..fb0b688d3 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -3,7 +3,16 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use syn::{parse::ParseStream, Attribute, Ident, Result, Token, Type}; +use std::collections::HashSet; + +use syn::{parse::ParseStream, punctuated::Punctuated, Attribute, Ident, Result, Token, Type}; + +#[derive(Debug, Eq, PartialEq, Hash)] +pub enum QPropertyFlag { + Read, + Write, + Notify, +} /// Describes a single Q_PROPERTY for a struct pub struct ParsedQProperty { @@ -11,6 +20,8 @@ pub struct ParsedQProperty { pub ident: Ident, /// The [syn::Type] of the property pub ty: Type, + /// HashSet of [QPropertyFlag]s which were specified + pub flags: HashSet, } impl ParsedQProperty { @@ -20,10 +31,42 @@ impl ParsedQProperty { let _comma = input.parse::()?; let ident = input.parse()?; + if input.is_empty() { + // No flags so return with empty HashSet + return Ok(Self { + ident, + ty, + flags: Default::default(), + }); + } + + let _comma = input.parse::()?; // Start of final identifiers + + // TODO: Allow parser to store pairs of items e.g read = get_value, Might be useful to use Meta::NameValue + let punctuated_flags: Punctuated = + Punctuated::parse_terminated(input)?; + + let flags: Vec = punctuated_flags.into_iter().collect(); // Removes the commas while collecting into Vec + + let mut flags_set: HashSet = HashSet::new(); + + for identifier in flags { + match identifier.to_string().as_str() { + "read" => flags_set.insert(QPropertyFlag::Read), + "write" => flags_set.insert(QPropertyFlag::Write), + "notify" => flags_set.insert(QPropertyFlag::Notify), + _ => panic!("Invalid Token"), // TODO: might not be a good idea to error here + }; + } + // TODO: later we'll need to parse setters and getters here // which are key-value, hence this not being parsed as a list - Ok(Self { ident, ty }) + Ok(Self { + ident, + ty, + flags: flags_set, + }) }) } } @@ -46,6 +89,42 @@ mod tests { assert_eq!(property.ty, parse_quote! { T }); } + #[test] + fn test_parse_read_flag() { + let mut input: ItemStruct = parse_quote! { + #[qproperty(T, name, read)] + struct MyStruct; + }; + let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); + assert_eq!(property.ident, format_ident!("name")); + assert_eq!(property.ty, parse_quote! { T }); + assert!(property.flags.contains(&QPropertyFlag::Read)); + } + + #[test] + fn test_parse_all_flags() { + let mut input: ItemStruct = parse_quote! { + #[qproperty(T, name, read, write, notify)] + struct MyStruct; + }; + let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); + assert_eq!(property.ident, format_ident!("name")); + assert_eq!(property.ty, parse_quote! { T }); + assert!(property.flags.contains(&QPropertyFlag::Read)); + assert!(property.flags.contains(&QPropertyFlag::Write)); + assert!(property.flags.contains(&QPropertyFlag::Notify)); + } + + #[test] + #[should_panic] + fn test_parse_invalid_flags() { + let mut input: ItemStruct = parse_quote! { + #[qproperty(T, name, read, write, A)] + struct MyStruct; + }; + let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); + } + #[test] fn test_parse_property_arg_extra() { let mut input: ItemStruct = parse_quote! { From 452ba322bc4a5bf4f0fc4abe892ad37c5aa8673d Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Wed, 10 Jul 2024 12:09:42 +0100 Subject: [PATCH 02/18] Add support to parse input into a hashset of flags in the ParsedQProperty struct --- crates/cxx-qt-gen/src/parser/property.rs | 154 +++++++++++++++++++---- 1 file changed, 131 insertions(+), 23 deletions(-) diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index fb0b688d3..750b9c521 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -5,13 +5,48 @@ use std::collections::HashSet; -use syn::{parse::ParseStream, punctuated::Punctuated, Attribute, Ident, Result, Token, Type}; +use syn::{ + parse::{Error, ParseStream}, + punctuated::Punctuated, + spanned::Spanned, + Attribute, Ident, Lit, Meta, MetaNameValue, Path, Result, Token, Type, +}; +/// Enum type for optional flags in qproperty macro +/// Can contain a string identifier of a custom getter, setter or notifier #[derive(Debug, Eq, PartialEq, Hash)] pub enum QPropertyFlag { - Read, - Write, - Notify, + Read(Option), + Write(Option), + Notify(Option), +} + +impl QPropertyFlag { + fn from_string(identifier: Ident, value: Option) -> Result { + return match identifier.to_string().as_str() { + "read" => Ok(QPropertyFlag::Read(value)), + "write" => Ok(QPropertyFlag::Write(value)), + "notify" => Ok(QPropertyFlag::Notify(value)), + _ => Err(Error::new( + identifier.span(), + "Flags must be one of read, write or notify", + )), + }; + } + + fn from_meta(meta_value: Meta) -> Result { + match meta_value { + Meta::Path(path) => Ok(Self::from_string(parse_path_to_ident(&path), None)?), + Meta::NameValue(name_value) => { + let kv_pair = parse_meta_name_value(&name_value)?; + Ok(Self::from_string(kv_pair.0, Some(kv_pair.1))?) + } + _ => Err(Error::new( + meta_value.span(), + "Invalid syntax, flags must be specified as either `read` or `read = 'my_getter'`", + )), + } + } } /// Describes a single Q_PROPERTY for a struct @@ -24,6 +59,35 @@ pub struct ParsedQProperty { pub flags: HashSet, } +fn parse_path_to_ident(path: &Path) -> Ident { + path.segments[0].ident.clone() +} + +fn parse_meta_name_value(name_value: &MetaNameValue) -> Result<(Ident, String)> { + let ident = parse_path_to_ident(&name_value.path); + let expr = &name_value.value; + let mut str_value: String = String::from(""); + match expr { + syn::Expr::Lit(literal) => match &literal.lit { + Lit::Str(str_lit) => str_value = str_lit.value(), + _ => { + return Err(Error::new( + expr.span(), + "Expression must be a string literal", + )) + } + }, + _ => { + return Err(Error::new( + expr.span(), + "Expression must be a string literal", + )) + } + } + + return Ok((ident, str_value)); +} + impl ParsedQProperty { pub fn parse(attr: Attribute) -> Result { attr.parse_args_with(|input: ParseStream| -> Result { @@ -42,21 +106,15 @@ impl ParsedQProperty { let _comma = input.parse::()?; // Start of final identifiers - // TODO: Allow parser to store pairs of items e.g read = get_value, Might be useful to use Meta::NameValue - let punctuated_flags: Punctuated = + let punctuated_flags: Punctuated = Punctuated::parse_terminated(input)?; - let flags: Vec = punctuated_flags.into_iter().collect(); // Removes the commas while collecting into Vec + let flags: Vec = punctuated_flags.into_iter().collect(); // Removes the commas while collecting into Vec - let mut flags_set: HashSet = HashSet::new(); + let mut flag_set: HashSet = HashSet::new(); - for identifier in flags { - match identifier.to_string().as_str() { - "read" => flags_set.insert(QPropertyFlag::Read), - "write" => flags_set.insert(QPropertyFlag::Write), - "notify" => flags_set.insert(QPropertyFlag::Notify), - _ => panic!("Invalid Token"), // TODO: might not be a good idea to error here - }; + for flag in flags { + flag_set.insert(QPropertyFlag::from_meta(flag)?); } // TODO: later we'll need to parse setters and getters here @@ -65,7 +123,7 @@ impl ParsedQProperty { Ok(Self { ident, ty, - flags: flags_set, + flags: flag_set, }) }) } @@ -98,7 +156,7 @@ mod tests { let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); assert_eq!(property.ident, format_ident!("name")); assert_eq!(property.ty, parse_quote! { T }); - assert!(property.flags.contains(&QPropertyFlag::Read)); + assert!(property.flags.contains(&QPropertyFlag::Read(None))); } #[test] @@ -110,19 +168,69 @@ mod tests { let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); assert_eq!(property.ident, format_ident!("name")); assert_eq!(property.ty, parse_quote! { T }); - assert!(property.flags.contains(&QPropertyFlag::Read)); - assert!(property.flags.contains(&QPropertyFlag::Write)); - assert!(property.flags.contains(&QPropertyFlag::Notify)); + assert!(property.flags.contains(&QPropertyFlag::Read(None))); + assert!(property.flags.contains(&QPropertyFlag::Write(None))); + assert!(property.flags.contains(&QPropertyFlag::Notify(None))); } #[test] - #[should_panic] - fn test_parse_invalid_flags() { + fn test_parse_kwargs() { let mut input: ItemStruct = parse_quote! { - #[qproperty(T, name, read, write, A)] + #[qproperty(T, name, read = "blah", write, notify = "blahblah")] struct MyStruct; }; let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); + assert_eq!(property.ident, format_ident!("name")); + assert_eq!(property.ty, parse_quote! { T }); + assert!(property + .flags + .contains(&QPropertyFlag::Read(Some(String::from("blah"))))); + assert!(property.flags.contains(&QPropertyFlag::Write(None))); + assert!(property + .flags + .contains(&QPropertyFlag::Notify(Some(String::from("blahblah"))))); + } + + #[test] + fn test_parse_invalid_flag() { + let mut input: ItemStruct = parse_quote! { + #[qproperty(T, name, read = "blah", a, notify = "blahblah")] + struct MyStruct; + }; + let property = ParsedQProperty::parse(input.attrs.remove(0)); + assert!(property.is_err()) + } + + #[test] + fn test_parse_invalid_flag_value() { + let mut input: ItemStruct = parse_quote! { + #[qproperty(T, name, read = blah, write, notify = "blahblah")] + struct MyStruct; + }; + let property = ParsedQProperty::parse(input.attrs.remove(0)); + assert!(property.is_err()) + } + + #[test] + fn test_parse_missing_flags() { + let mut input: ItemStruct = parse_quote! { + #[qproperty(T, name, notify = "blahblah")] + struct MyStruct; + }; + let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); + assert!(property + .flags + .contains(&QPropertyFlag::Notify(Some(String::from("blahblah"))))) + } + + #[test] + fn test_parse_invalid_literal() { + let mut input: ItemStruct = parse_quote! { + #[qproperty(T, name, notify = 3)] + struct MyStruct; + }; + let property = ParsedQProperty::parse(input.attrs.remove(0)); + assert!(property.is_err()); } #[test] From 4e82194d4886270b94db692bd025d5b692283bd8 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Wed, 10 Jul 2024 13:00:52 +0100 Subject: [PATCH 03/18] Refactor for QPropertyFlag to use Idents instead of Strings --- crates/cxx-qt-gen/src/parser/property.rs | 78 +++++++++++------------- 1 file changed, 36 insertions(+), 42 deletions(-) diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index 750b9c521..e6de2aee1 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -9,20 +9,20 @@ use syn::{ parse::{Error, ParseStream}, punctuated::Punctuated, spanned::Spanned, - Attribute, Ident, Lit, Meta, MetaNameValue, Path, Result, Token, Type, + Attribute, Expr, Ident, Meta, MetaNameValue, Path, Result, Token, Type, }; /// Enum type for optional flags in qproperty macro /// Can contain a string identifier of a custom getter, setter or notifier #[derive(Debug, Eq, PartialEq, Hash)] pub enum QPropertyFlag { - Read(Option), - Write(Option), - Notify(Option), + Read(Option), + Write(Option), + Notify(Option), } impl QPropertyFlag { - fn from_string(identifier: Ident, value: Option) -> Result { + fn from_ident(identifier: Ident, value: Option) -> Result { return match identifier.to_string().as_str() { "read" => Ok(QPropertyFlag::Read(value)), "write" => Ok(QPropertyFlag::Write(value)), @@ -36,10 +36,10 @@ impl QPropertyFlag { fn from_meta(meta_value: Meta) -> Result { match meta_value { - Meta::Path(path) => Ok(Self::from_string(parse_path_to_ident(&path), None)?), + Meta::Path(path) => Ok(Self::from_ident(parse_path_to_ident(&path), None)?), Meta::NameValue(name_value) => { let kv_pair = parse_meta_name_value(&name_value)?; - Ok(Self::from_string(kv_pair.0, Some(kv_pair.1))?) + Ok(Self::from_ident(kv_pair.0, Some(kv_pair.1))?) } _ => Err(Error::new( meta_value.span(), @@ -63,29 +63,21 @@ fn parse_path_to_ident(path: &Path) -> Ident { path.segments[0].ident.clone() } -fn parse_meta_name_value(name_value: &MetaNameValue) -> Result<(Ident, String)> { +fn parse_meta_name_value(name_value: &MetaNameValue) -> Result<(Ident, Ident)> { let ident = parse_path_to_ident(&name_value.path); let expr = &name_value.value; - let mut str_value: String = String::from(""); - match expr { - syn::Expr::Lit(literal) => match &literal.lit { - Lit::Str(str_lit) => str_value = str_lit.value(), - _ => { - return Err(Error::new( - expr.span(), - "Expression must be a string literal", - )) - } - }, - _ => { - return Err(Error::new( - expr.span(), - "Expression must be a string literal", - )) - } + let func_signature: Ident; + + if let Expr::Path(path_expr) = expr { + func_signature = parse_path_to_ident(&path_expr.path); + } else { + return Err(Error::new( + expr.span(), + "Function signature must be an identifier", + )); } - return Ok((ident, str_value)); + Ok((ident, func_signature)) } impl ParsedQProperty { @@ -120,6 +112,8 @@ impl ParsedQProperty { // TODO: later we'll need to parse setters and getters here // which are key-value, hence this not being parsed as a list + println!("{:?}", flag_set); + Ok(Self { ident, ty, @@ -136,6 +130,16 @@ mod tests { use quote::format_ident; use syn::{parse_quote, ItemStruct}; + #[test] + fn test_debug_structs() { + let mut input: ItemStruct = parse_quote! { + #[qproperty(T, name, read = my_getter, write, notify = my_notifier)] + struct MyStruct; + }; + let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); + println!("{:?}", property.flags) + } + #[test] fn test_parse_property() { let mut input: ItemStruct = parse_quote! { @@ -176,7 +180,7 @@ mod tests { #[test] fn test_parse_kwargs() { let mut input: ItemStruct = parse_quote! { - #[qproperty(T, name, read = "blah", write, notify = "blahblah")] + #[qproperty(T, name, read = blah, write, notify = blahblah)] struct MyStruct; }; let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); @@ -184,27 +188,17 @@ mod tests { assert_eq!(property.ty, parse_quote! { T }); assert!(property .flags - .contains(&QPropertyFlag::Read(Some(String::from("blah"))))); + .contains(&QPropertyFlag::Read(Some(format_ident!("blah"))))); assert!(property.flags.contains(&QPropertyFlag::Write(None))); assert!(property .flags - .contains(&QPropertyFlag::Notify(Some(String::from("blahblah"))))); + .contains(&QPropertyFlag::Notify(Some(format_ident!("blahblah"))))); } #[test] fn test_parse_invalid_flag() { let mut input: ItemStruct = parse_quote! { - #[qproperty(T, name, read = "blah", a, notify = "blahblah")] - struct MyStruct; - }; - let property = ParsedQProperty::parse(input.attrs.remove(0)); - assert!(property.is_err()) - } - - #[test] - fn test_parse_invalid_flag_value() { - let mut input: ItemStruct = parse_quote! { - #[qproperty(T, name, read = blah, write, notify = "blahblah")] + #[qproperty(T, name, read = blah, a, notify = blahblah)] struct MyStruct; }; let property = ParsedQProperty::parse(input.attrs.remove(0)); @@ -214,13 +208,13 @@ mod tests { #[test] fn test_parse_missing_flags() { let mut input: ItemStruct = parse_quote! { - #[qproperty(T, name, notify = "blahblah")] + #[qproperty(T, name, notify = blahblah)] struct MyStruct; }; let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); assert!(property .flags - .contains(&QPropertyFlag::Notify(Some(String::from("blahblah"))))) + .contains(&QPropertyFlag::Notify(Some(format_ident!("blahblah"))))) } #[test] From 1eead3f8e3936f4acebc5c946a3b831d7cfff1f6 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Thu, 11 Jul 2024 10:41:06 +0100 Subject: [PATCH 04/18] Make setter optional --- .../src/generator/cpp/property/meta.rs | 28 +++++-- .../src/generator/cpp/property/mod.rs | 82 +++++++++++++++---- .../src/generator/cpp/property/setter.rs | 8 +- .../src/generator/naming/property.rs | 57 +++++++++++-- .../src/generator/rust/property/setter.rs | 5 +- crates/cxx-qt-gen/src/parser/property.rs | 60 ++++++-------- 6 files changed, 169 insertions(+), 71 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs b/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs index 1b3a11602..d640c78f0 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs @@ -7,12 +7,28 @@ use crate::generator::naming::property::QPropertyNames; /// Generate the metaobject line for a given property pub fn generate(idents: &QPropertyNames, cxx_ty: &str) -> String { - format!( - "Q_PROPERTY({ty} {ident} READ {ident_getter} WRITE {ident_setter} NOTIFY {ident_notify})", + let mut output: String = format!( + "Q_PROPERTY({ty} {ident} READ {ident_getter} ", ty = cxx_ty, ident = idents.name.cxx_unqualified(), - ident_getter = idents.getter.cxx_unqualified(), - ident_setter = idents.setter.cxx_unqualified(), - ident_notify = idents.notify.cxx_unqualified() - ) + ident_getter = idents.getter.cxx_unqualified() + ); + // Write + if let Some(name) = &idents.setter { + output += format!("WRITE {} ", name.cxx_unqualified()).as_str(); + } + // Notify + output += format!("NOTIFY {}", idents.notify.cxx_unqualified()).as_str(); + + + // format!( + // "Q_PROPERTY({ty} {ident} READ {ident_getter} WRITE {ident_setter} NOTIFY {ident_notify})", + // ty = cxx_ty, + // ident = idents.name.cxx_unqualified(), + // ident_setter = ident_setter_value, + // ident_getter = idents.getter.cxx_unqualified(), + // ident_notify = idents.notify.cxx_unqualified() + // ) + + return output } diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs index ce3743d82..f38867b33 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs @@ -8,9 +8,12 @@ use crate::generator::{ naming::{property::QPropertyNames, qobject::QObjectNames}, }; use crate::{ - naming::cpp::syn_type_to_cpp_type, naming::TypeNames, parser::property::ParsedQProperty, + naming::cpp::syn_type_to_cpp_type, + naming::TypeNames, + parser::property::{ParsedQProperty, QPropertyFlag}, }; -use syn::Result; +use std::collections::HashSet; +use syn::{Error, Result}; mod getter; mod meta; @@ -27,24 +30,48 @@ pub fn generate_cpp_properties( let qobject_ident = qobject_idents.name.cxx_unqualified(); for property in properties { - // Cache the idents as they are used in multiple places + + // Cache the idents and flags as they are used in multiple places let idents = QPropertyNames::from(property); let cxx_ty = syn_type_to_cpp_type(&property.ty, type_names)?; + generated.metaobjects.push(meta::generate(&idents, &cxx_ty)); - generated - .methods - .push(getter::generate(&idents, &qobject_ident, &cxx_ty)); - generated - .private_methods - .push(getter::generate_wrapper(&idents, &cxx_ty)); - generated - .methods - .push(setter::generate(&idents, &qobject_ident, &cxx_ty)); - generated - .private_methods - .push(setter::generate_wrapper(&idents, &cxx_ty)); - signals.push(signal::generate(&idents, qobject_idents)); + + let mut includes_read = false; // If the HashSet includes entries read must be specified otherwise it is an error + + for flag in &property.flags { + match flag { + QPropertyFlag::Write(ref signature) => { + // Gen setters + generated + .methods + .push(setter::generate(&idents, &qobject_ident, &cxx_ty)); + generated + .private_methods + .push(setter::generate_wrapper(&idents, &cxx_ty)); + } + QPropertyFlag::Read(ref signature) => { + includes_read = true; + // Gen Getters + generated + .methods + .push(getter::generate(&idents, &qobject_ident, &cxx_ty)); + generated + .private_methods + .push(getter::generate_wrapper(&idents, &cxx_ty)); + } + QPropertyFlag::Notify(ref signature) => { + // Gen signal + signals.push(signal::generate(&idents, qobject_idents)); + } + } + } + + if !includes_read { + // TODO: Change to throwing an error, but no syn types present in this function + panic!("If flags are specified, read cannot be inferred and so must be specified") + } } generated.append(&mut generate_cpp_signals( @@ -65,7 +92,27 @@ mod tests { use indoc::indoc; use pretty_assertions::assert_str_eq; use quote::format_ident; - use syn::parse_quote; + use syn::{parse_quote, ItemStruct}; + + #[test] + fn test_optional_write() { + let mut input: ItemStruct = parse_quote! { + #[qproperty(i32, num, read)] + struct MyStruct; + }; + let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); + + let properties = vec![property]; + + let qobject_idents = create_qobjectname(); + + let mut type_names = TypeNames::mock(); + type_names.mock_insert("i32", None, None, None); + let generated = generate_cpp_properties(&properties, &qobject_idents, &type_names).unwrap(); + + println!("generated code: \n{:?}", generated.metaobjects); + println!("generated methods: \n{:?}", generated.methods); + } #[test] fn test_generate_cpp_properties() { @@ -81,6 +128,7 @@ mod tests { flags: Default::default(), }, ]; + let qobject_idents = create_qobjectname(); let mut type_names = TypeNames::mock(); diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs index db439907b..3ef0424e9 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs @@ -10,7 +10,7 @@ pub fn generate(idents: &QPropertyNames, qobject_ident: &str, cxx_ty: &str) -> C CppFragment::Pair { header: format!( "Q_SLOT void {ident_setter}({cxx_ty} const& value);", - ident_setter = idents.setter.cxx_unqualified(), + ident_setter = idents.setter.clone().expect("Setter was empty").cxx_unqualified(), ), source: formatdoc! { r#" @@ -21,8 +21,8 @@ pub fn generate(idents: &QPropertyNames, qobject_ident: &str, cxx_ty: &str) -> C {ident_setter_wrapper}(value); }} "#, - ident_setter = idents.setter.cxx_unqualified(), - ident_setter_wrapper = idents.setter_wrapper.cxx_unqualified(), + ident_setter = idents.setter.clone().expect("Setter was empty").cxx_unqualified(), + ident_setter_wrapper = idents.setter_wrapper.clone().expect("Setter was empty").cxx_unqualified(), }, } } @@ -32,6 +32,6 @@ pub fn generate_wrapper(idents: &QPropertyNames, cxx_ty: &str) -> CppFragment { // Note that we pass T not const T& to Rust so that it is by-value // https://github.com/KDAB/cxx-qt/issues/463 "void {ident_setter_wrapper}({cxx_ty} value) noexcept;", - ident_setter_wrapper = idents.setter_wrapper.cxx_unqualified() + ident_setter_wrapper = idents.setter_wrapper.clone().expect("Setter was empty").cxx_unqualified() )) } diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs index 5e577cea8..a6140912f 100644 --- a/crates/cxx-qt-gen/src/generator/naming/property.rs +++ b/crates/cxx-qt-gen/src/generator/naming/property.rs @@ -2,7 +2,7 @@ // SPDX-FileContributor: Andrew Hayzen // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::{naming::Name, parser::property::ParsedQProperty}; +use crate::{naming::Name, parser::property::{ParsedQProperty, QPropertyFlag}}; use convert_case::{Case, Casing}; use quote::format_ident; use syn::Ident; @@ -12,22 +12,61 @@ pub struct QPropertyNames { pub name: Name, pub getter: Name, pub getter_wrapper: Name, - pub setter: Name, - pub setter_wrapper: Name, + pub setter: Option, + pub setter_wrapper: Option, pub notify: Name, } impl From<&ParsedQProperty> for QPropertyNames { fn from(property: &ParsedQProperty) -> Self { let property_name = property_name_from_rust_name(property.ident.clone()); - let getter = getter_name_from_property(&property_name); - let setter = setter_name_from_property(&property_name); + + let mut getter: Name = Name::new(format_ident!("test_ident")); + let mut setter: Option = None; + let mut notify: Name = Name::new(format_ident!("test_ident")); + + for flag in &property.flags { + match flag { + QPropertyFlag::Write(ref signature) => { // TODO: remove if let blocks (passing custom func should not change name of getter, only its contents) + if let Some(ident) = signature { + setter = Some(Name::new(ident.clone())) + } + else { + setter = Some(setter_name_from_property(&property_name)) + } + }, + QPropertyFlag::Read(ref signature) => { + if let Some(ident) = signature { + getter = Name::new(ident.clone()) + } + else { + getter = getter_name_from_property(&property_name) + } + }, + QPropertyFlag::Notify(ref signature) => { + if let Some(ident) = signature { + notify = Name::new(ident.clone()) + } + else { + notify = notify_name_from_property(&property_name) + } + }, + } + } + let setter_wrapper: Option; + if let Some(name) = &setter { + setter_wrapper = Some(wrapper_name_from_function_name(&name)); + } + else { + setter_wrapper = None; + } + Self { getter_wrapper: wrapper_name_from_function_name(&getter), getter, - setter_wrapper: wrapper_name_from_function_name(&setter), + setter_wrapper, setter, - notify: notify_name_from_property(&property_name), + notify, name: property_name, } } @@ -98,9 +137,9 @@ pub mod tests { names.getter.rust_unqualified(), &format_ident!("my_property") ); - assert_eq!(names.setter.cxx_unqualified(), "setMyProperty"); + assert_eq!(names.setter.clone().expect("Setter was empty").cxx_unqualified(), "setMyProperty"); assert_eq!( - names.setter.rust_unqualified(), + names.setter.clone().expect("Setter was empty").rust_unqualified(), &format_ident!("set_my_property") ); assert_eq!(names.notify.cxx_unqualified(), "myPropertyChanged"); diff --git a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs index ce759e717..b7a9062d1 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs @@ -21,8 +21,9 @@ pub fn generate( type_names: &TypeNames, ) -> Result { let cpp_class_name_rust = &qobject_idents.name.rust_unqualified(); - let setter_wrapper_cpp = idents.setter_wrapper.cxx_unqualified(); - let setter_rust = &idents.setter.rust_unqualified(); + let setter_wrapper_cpp = idents.setter_wrapper.clone().expect("Setter was empty").cxx_unqualified(); + let setter_binding = idents.setter.clone().expect("Setter was empty"); + let setter_rust = setter_binding.rust_unqualified(); let ident = &idents.name.rust_unqualified(); let ident_str = ident.to_string(); let notify_ident = &idents.notify.rust_unqualified(); diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index e6de2aee1..dbfb4068d 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -87,38 +87,42 @@ impl ParsedQProperty { let _comma = input.parse::()?; let ident = input.parse()?; + let mut flag_set = HashSet::new(); + if input.is_empty() { - // No flags so return with empty HashSet + flag_set.insert(QPropertyFlag::Read(None)); + flag_set.insert(QPropertyFlag::Write(None)); + flag_set.insert(QPropertyFlag::Notify(None)); + + // No flags so fill with default options return Ok(Self { ident, ty, - flags: Default::default(), + flags: flag_set, }); } - let _comma = input.parse::()?; // Start of final identifiers - - let punctuated_flags: Punctuated = - Punctuated::parse_terminated(input)?; - - let flags: Vec = punctuated_flags.into_iter().collect(); // Removes the commas while collecting into Vec - - let mut flag_set: HashSet = HashSet::new(); - - for flag in flags { - flag_set.insert(QPropertyFlag::from_meta(flag)?); + else { + let _comma = input.parse::()?; // Start of final identifiers + + let punctuated_flags: Punctuated = + Punctuated::parse_terminated(input)?; + + let flags: Vec = punctuated_flags.into_iter().collect(); // Removes the commas while collecting into Vec + + let mut flag_set: HashSet = HashSet::new(); + + for flag in flags { + flag_set.insert(QPropertyFlag::from_meta(flag)?); + } + + Ok(Self { + ident, + ty, + flags: flag_set, + }) } - // TODO: later we'll need to parse setters and getters here - // which are key-value, hence this not being parsed as a list - - println!("{:?}", flag_set); - - Ok(Self { - ident, - ty, - flags: flag_set, - }) }) } } @@ -130,16 +134,6 @@ mod tests { use quote::format_ident; use syn::{parse_quote, ItemStruct}; - #[test] - fn test_debug_structs() { - let mut input: ItemStruct = parse_quote! { - #[qproperty(T, name, read = my_getter, write, notify = my_notifier)] - struct MyStruct; - }; - let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); - println!("{:?}", property.flags) - } - #[test] fn test_parse_property() { let mut input: ItemStruct = parse_quote! { From 50df978b97c92905ae16c6a21462bde0eeee9f7d Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Thu, 11 Jul 2024 11:24:00 +0100 Subject: [PATCH 05/18] Make Notify optional --- .../src/generator/cpp/property/meta.rs | 20 ++---- .../src/generator/cpp/property/mod.rs | 13 ++-- .../src/generator/cpp/property/setter.rs | 12 +++- .../src/generator/cpp/property/signal.rs | 7 +- .../src/generator/naming/property.rs | 66 ++++++++++++------- .../src/generator/rust/property/setter.rs | 9 ++- .../src/generator/rust/property/signal.rs | 7 +- crates/cxx-qt-gen/src/parser/property.rs | 21 +++--- 8 files changed, 90 insertions(+), 65 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs b/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs index d640c78f0..c32a45330 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs @@ -13,22 +13,16 @@ pub fn generate(idents: &QPropertyNames, cxx_ty: &str) -> String { ident = idents.name.cxx_unqualified(), ident_getter = idents.getter.cxx_unqualified() ); - // Write + // Write if let Some(name) = &idents.setter { output += format!("WRITE {} ", name.cxx_unqualified()).as_str(); - } + } // Notify - output += format!("NOTIFY {}", idents.notify.cxx_unqualified()).as_str(); - + if let Some(name) = &idents.notify { + output += format!("NOTIFY {} ", name.cxx_unqualified()).as_str(); + } - // format!( - // "Q_PROPERTY({ty} {ident} READ {ident_getter} WRITE {ident_setter} NOTIFY {ident_notify})", - // ty = cxx_ty, - // ident = idents.name.cxx_unqualified(), - // ident_setter = ident_setter_value, - // ident_getter = idents.getter.cxx_unqualified(), - // ident_notify = idents.notify.cxx_unqualified() - // ) + output += ")"; - return output + output } diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs index f38867b33..a5c5db720 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs @@ -12,8 +12,7 @@ use crate::{ naming::TypeNames, parser::property::{ParsedQProperty, QPropertyFlag}, }; -use std::collections::HashSet; -use syn::{Error, Result}; +use syn::Result; mod getter; mod meta; @@ -30,19 +29,17 @@ pub fn generate_cpp_properties( let qobject_ident = qobject_idents.name.cxx_unqualified(); for property in properties { - // Cache the idents and flags as they are used in multiple places let idents = QPropertyNames::from(property); let cxx_ty = syn_type_to_cpp_type(&property.ty, type_names)?; - generated.metaobjects.push(meta::generate(&idents, &cxx_ty)); let mut includes_read = false; // If the HashSet includes entries read must be specified otherwise it is an error for flag in &property.flags { match flag { - QPropertyFlag::Write(ref signature) => { + QPropertyFlag::Write(_) => { // Gen setters generated .methods @@ -51,7 +48,7 @@ pub fn generate_cpp_properties( .private_methods .push(setter::generate_wrapper(&idents, &cxx_ty)); } - QPropertyFlag::Read(ref signature) => { + QPropertyFlag::Read(_) => { includes_read = true; // Gen Getters generated @@ -61,7 +58,7 @@ pub fn generate_cpp_properties( .private_methods .push(getter::generate_wrapper(&idents, &cxx_ty)); } - QPropertyFlag::Notify(ref signature) => { + QPropertyFlag::Notify(_) => { // Gen signal signals.push(signal::generate(&idents, qobject_idents)); } @@ -97,7 +94,7 @@ mod tests { #[test] fn test_optional_write() { let mut input: ItemStruct = parse_quote! { - #[qproperty(i32, num, read)] + #[qproperty(i32, num, read, write, notify)] struct MyStruct; }; let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs index 3ef0424e9..ddbef1bb3 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs @@ -10,7 +10,11 @@ pub fn generate(idents: &QPropertyNames, qobject_ident: &str, cxx_ty: &str) -> C CppFragment::Pair { header: format!( "Q_SLOT void {ident_setter}({cxx_ty} const& value);", - ident_setter = idents.setter.clone().expect("Setter was empty").cxx_unqualified(), + ident_setter = idents + .setter + .clone() + .expect("Setter was empty") + .cxx_unqualified(), ), source: formatdoc! { r#" @@ -32,6 +36,10 @@ pub fn generate_wrapper(idents: &QPropertyNames, cxx_ty: &str) -> CppFragment { // Note that we pass T not const T& to Rust so that it is by-value // https://github.com/KDAB/cxx-qt/issues/463 "void {ident_setter_wrapper}({cxx_ty} value) noexcept;", - ident_setter_wrapper = idents.setter_wrapper.clone().expect("Setter was empty").cxx_unqualified() + ident_setter_wrapper = idents + .setter_wrapper + .clone() + .expect("Setter was empty") + .cxx_unqualified() )) } diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs b/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs index 2ef864052..c9bacacae 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs @@ -14,8 +14,9 @@ pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Parse // We build our signal in the generation phase as we need to use the naming // structs to build the signal name let cpp_class_rust = &qobject_idents.name.rust_unqualified(); - let notify_cpp = &idents.notify.cxx_unqualified(); - let notify_rust = idents.notify.rust_unqualified(); + let notify = &idents.notify.clone().expect("Notify was empty!"); + let notify_cpp = notify.cxx_unqualified(); + let notify_rust = notify.rust_unqualified(); let method: ForeignItemFn = syn::parse_quote! { #[doc = "Notify for the Q_PROPERTY"] #[cxx_name = #notify_cpp] @@ -23,7 +24,7 @@ pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Parse }; ParsedSignal::from_property_method( method, - idents.notify.clone(), + idents.notify.clone().expect("Notify was empty!"), qobject_idents.name.rust_unqualified().clone(), ) } diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs index a6140912f..06ebd0d75 100644 --- a/crates/cxx-qt-gen/src/generator/naming/property.rs +++ b/crates/cxx-qt-gen/src/generator/naming/property.rs @@ -2,7 +2,10 @@ // SPDX-FileContributor: Andrew Hayzen // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::{naming::Name, parser::property::{ParsedQProperty, QPropertyFlag}}; +use crate::{ + naming::Name, + parser::property::{ParsedQProperty, QPropertyFlag}, +}; use convert_case::{Case, Casing}; use quote::format_ident; use syn::Ident; @@ -14,7 +17,7 @@ pub struct QPropertyNames { pub getter_wrapper: Name, pub setter: Option, pub setter_wrapper: Option, - pub notify: Name, + pub notify: Option, } impl From<&ParsedQProperty> for QPropertyNames { @@ -23,41 +26,38 @@ impl From<&ParsedQProperty> for QPropertyNames { let mut getter: Name = Name::new(format_ident!("test_ident")); let mut setter: Option = None; - let mut notify: Name = Name::new(format_ident!("test_ident")); + let mut notify: Option = None; for flag in &property.flags { match flag { - QPropertyFlag::Write(ref signature) => { // TODO: remove if let blocks (passing custom func should not change name of getter, only its contents) + QPropertyFlag::Write(ref signature) => { + // TODO: remove if let blocks (passing custom func should not change name of getter, only its contents) if let Some(ident) = signature { setter = Some(Name::new(ident.clone())) - } - else { + } else { setter = Some(setter_name_from_property(&property_name)) } - }, + } QPropertyFlag::Read(ref signature) => { if let Some(ident) = signature { getter = Name::new(ident.clone()) - } - else { + } else { getter = getter_name_from_property(&property_name) } - }, + } QPropertyFlag::Notify(ref signature) => { if let Some(ident) = signature { - notify = Name::new(ident.clone()) + notify = Some(Name::new(ident.clone())) + } else { + notify = Some(notify_name_from_property(&property_name)) } - else { - notify = notify_name_from_property(&property_name) - } - }, + } } } let setter_wrapper: Option; if let Some(name) = &setter { - setter_wrapper = Some(wrapper_name_from_function_name(&name)); - } - else { + setter_wrapper = Some(wrapper_name_from_function_name(name)); + } else { setter_wrapper = None; } @@ -137,14 +137,36 @@ pub mod tests { names.getter.rust_unqualified(), &format_ident!("my_property") ); - assert_eq!(names.setter.clone().expect("Setter was empty").cxx_unqualified(), "setMyProperty"); assert_eq!( - names.setter.clone().expect("Setter was empty").rust_unqualified(), + names + .setter + .clone() + .expect("Setter was empty") + .cxx_unqualified(), + "setMyProperty" + ); + assert_eq!( + names + .setter + .clone() + .expect("Setter was empty") + .rust_unqualified(), &format_ident!("set_my_property") ); - assert_eq!(names.notify.cxx_unqualified(), "myPropertyChanged"); assert_eq!( - names.notify.rust_unqualified(), + names + .notify + .clone() + .expect("Notify was empty") + .cxx_unqualified(), + "myPropertyChanged" + ); + assert_eq!( + names + .notify + .clone() + .expect("Notify was empty") + .rust_unqualified(), &format_ident!("my_property_changed") ); } diff --git a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs index b7a9062d1..b4f493412 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs @@ -21,12 +21,17 @@ pub fn generate( type_names: &TypeNames, ) -> Result { let cpp_class_name_rust = &qobject_idents.name.rust_unqualified(); - let setter_wrapper_cpp = idents.setter_wrapper.clone().expect("Setter was empty").cxx_unqualified(); + let setter_wrapper_cpp = idents + .setter_wrapper + .clone() + .expect("Setter was empty") + .cxx_unqualified(); let setter_binding = idents.setter.clone().expect("Setter was empty"); let setter_rust = setter_binding.rust_unqualified(); let ident = &idents.name.rust_unqualified(); let ident_str = ident.to_string(); - let notify_ident = &idents.notify.rust_unqualified(); + let notify = &idents.notify.clone().expect("Notify was empty!"); + let notify_ident = notify.rust_unqualified(); let qualified_ty = syn_type_cxx_bridge_to_qualified(cxx_ty, type_names)?; let qualified_impl = type_names.rust_qualified(cpp_class_name_rust)?; diff --git a/crates/cxx-qt-gen/src/generator/rust/property/signal.rs b/crates/cxx-qt-gen/src/generator/rust/property/signal.rs index 8064036f5..994abb344 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/signal.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/signal.rs @@ -14,8 +14,9 @@ pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Parse // We build our signal in the generation phase as we need to use the naming // structs to build the signal name let cpp_class_rust = &qobject_idents.name.rust_unqualified(); - let notify_rust = &idents.notify.rust_unqualified(); - let notify_cpp_str = &idents.notify.cxx_unqualified(); + let notify = &idents.notify.clone().expect("Notify was empty!"); + let notify_rust = notify.rust_unqualified(); + let notify_cpp_str = notify.cxx_unqualified(); let method: ForeignItemFn = syn::parse_quote! { #[doc = "Notify for the Q_PROPERTY"] #[cxx_name = #notify_cpp_str] @@ -23,7 +24,7 @@ pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Parse }; ParsedSignal::from_property_method( method, - idents.notify.clone(), + idents.notify.clone().expect("Notify was empty"), qobject_idents.name.rust_unqualified().clone(), ) } diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index dbfb4068d..a947431d2 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -93,36 +93,33 @@ impl ParsedQProperty { flag_set.insert(QPropertyFlag::Read(None)); flag_set.insert(QPropertyFlag::Write(None)); flag_set.insert(QPropertyFlag::Notify(None)); - + // No flags so fill with default options - return Ok(Self { + Ok(Self { ident, ty, flags: flag_set, - }); - } - - else { + }) + } else { let _comma = input.parse::()?; // Start of final identifiers - + let punctuated_flags: Punctuated = Punctuated::parse_terminated(input)?; - + let flags: Vec = punctuated_flags.into_iter().collect(); // Removes the commas while collecting into Vec - + let mut flag_set: HashSet = HashSet::new(); - + for flag in flags { flag_set.insert(QPropertyFlag::from_meta(flag)?); } - + Ok(Self { ident, ty, flags: flag_set, }) } - }) } } From eb0bdb5f43002f8f67164715a8c3a4ebc3e1f173 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Fri, 12 Jul 2024 14:10:38 +0100 Subject: [PATCH 06/18] Improve error handling --- .../src/generator/cpp/property/mod.rs | 8 ++-- .../src/generator/cpp/property/setter.rs | 1 + .../src/generator/cpp/property/signal.rs | 14 ++++-- .../src/generator/naming/property.rs | 4 +- .../src/generator/rust/property/setter.rs | 43 +++++++++++++++---- .../src/generator/rust/property/signal.rs | 4 +- crates/cxx-qt-gen/src/naming/name.rs | 2 +- crates/cxx-qt-gen/src/parser/property.rs | 2 +- 8 files changed, 56 insertions(+), 22 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs index a5c5db720..a719225ac 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs @@ -12,7 +12,7 @@ use crate::{ naming::TypeNames, parser::property::{ParsedQProperty, QPropertyFlag}, }; -use syn::Result; +use syn::{Error, Result}; mod getter; mod meta; @@ -66,8 +66,10 @@ pub fn generate_cpp_properties( } if !includes_read { - // TODO: Change to throwing an error, but no syn types present in this function - panic!("If flags are specified, read cannot be inferred and so must be specified") + return Err(Error::new( + property.ident.span(), + "If other flags are specified, read must also be specified", + )); } } diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs index ddbef1bb3..4bd427f3a 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs @@ -6,6 +6,7 @@ use crate::generator::{cpp::fragment::CppFragment, naming::property::QPropertyNames}; use indoc::formatdoc; +// TODO: modify func to return result for proper erroring pub fn generate(idents: &QPropertyNames, qobject_ident: &str, cxx_ty: &str) -> CppFragment { CppFragment::Pair { header: format!( diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs b/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs index c9bacacae..11f2ad786 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs @@ -3,7 +3,7 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use syn::ForeignItemFn; +use syn::{spanned::Spanned, Error, ForeignItemFn}; use crate::{ generator::naming::{property::QPropertyNames, qobject::QObjectNames}, @@ -14,9 +14,15 @@ pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Parse // We build our signal in the generation phase as we need to use the naming // structs to build the signal name let cpp_class_rust = &qobject_idents.name.rust_unqualified(); - let notify = &idents.notify.clone().expect("Notify was empty!"); - let notify_cpp = notify.cxx_unqualified(); - let notify_rust = notify.rust_unqualified(); + let notify_binding = &idents.notify.clone().expect("Notify was empty!"); + // TODO: modify the func to return result + // let notify_binding = match &idents.notify { + // Some(notify) => notift, + // _ => return Err(Error::new(cxx_ty.span(), "Property did not include a notify field")) + // }; + + let notify_cpp = notify_binding.cxx_unqualified(); + let notify_rust = notify_binding.rust_unqualified(); let method: ForeignItemFn = syn::parse_quote! { #[doc = "Notify for the Q_PROPERTY"] #[cxx_name = #notify_cpp] diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs index 06ebd0d75..178f0092b 100644 --- a/crates/cxx-qt-gen/src/generator/naming/property.rs +++ b/crates/cxx-qt-gen/src/generator/naming/property.rs @@ -24,7 +24,7 @@ impl From<&ParsedQProperty> for QPropertyNames { fn from(property: &ParsedQProperty) -> Self { let property_name = property_name_from_rust_name(property.ident.clone()); - let mut getter: Name = Name::new(format_ident!("test_ident")); + let mut getter: Name = getter_name_from_property(&property_name); //TODO: potentially declaring this twice, might be fixed when tackling other todo in this file let mut setter: Option = None; let mut notify: Option = None; @@ -124,7 +124,7 @@ pub mod tests { ty, flags: Default::default(), }; - QPropertyNames::from(&property) + QPropertyNames::from(&property) // Doesn't account for emtpy flags, maybe change this to the quote macro } #[test] diff --git a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs index b4f493412..294dd07d2 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs @@ -12,7 +12,7 @@ use crate::{ naming::TypeNames, }; use quote::quote; -use syn::{Result, Type}; +use syn::{spanned::Spanned, Error, Result, Type}; pub fn generate( idents: &QPropertyNames, @@ -21,17 +21,42 @@ pub fn generate( type_names: &TypeNames, ) -> Result { let cpp_class_name_rust = &qobject_idents.name.rust_unqualified(); - let setter_wrapper_cpp = idents - .setter_wrapper - .clone() - .expect("Setter was empty") - .cxx_unqualified(); - let setter_binding = idents.setter.clone().expect("Setter was empty"); + + let setter_wrapper_cpp = match &idents.setter_wrapper { + Some(wrapper) => wrapper.cxx_unqualified(), + _ => { + return Err(Error::new( + cxx_ty.span(), + "Property did not include a setter wrapper", + )) + } + }; + + let setter_binding = match &idents.setter { + Some(setter) => setter, + _ => { + return Err(Error::new( + cxx_ty.span(), + "Property did not include a setter", + )) + } + }; + let setter_rust = setter_binding.rust_unqualified(); let ident = &idents.name.rust_unqualified(); let ident_str = ident.to_string(); - let notify = &idents.notify.clone().expect("Notify was empty!"); - let notify_ident = notify.rust_unqualified(); + + let notify_binding = match &idents.notify { + Some(notify) => notify, + _ => { + return Err(Error::new( + cxx_ty.span(), + "Property did not include a notify field", + )) + } + }; + + let notify_ident = notify_binding.rust_unqualified(); let qualified_ty = syn_type_cxx_bridge_to_qualified(cxx_ty, type_names)?; let qualified_impl = type_names.rust_qualified(cpp_class_name_rust)?; diff --git a/crates/cxx-qt-gen/src/generator/rust/property/signal.rs b/crates/cxx-qt-gen/src/generator/rust/property/signal.rs index 994abb344..1888f004f 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/signal.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/signal.rs @@ -14,7 +14,7 @@ pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Parse // We build our signal in the generation phase as we need to use the naming // structs to build the signal name let cpp_class_rust = &qobject_idents.name.rust_unqualified(); - let notify = &idents.notify.clone().expect("Notify was empty!"); + let notify = &idents.notify.clone().expect("Notify was empty!"); //TODO: Throw error instead let notify_rust = notify.rust_unqualified(); let notify_cpp_str = notify.cxx_unqualified(); let method: ForeignItemFn = syn::parse_quote! { @@ -24,7 +24,7 @@ pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Parse }; ParsedSignal::from_property_method( method, - idents.notify.clone().expect("Notify was empty"), + notify.clone(), qobject_idents.name.rust_unqualified().clone(), ) } diff --git a/crates/cxx-qt-gen/src/naming/name.rs b/crates/cxx-qt-gen/src/naming/name.rs index cef617a96..d1d191b00 100644 --- a/crates/cxx-qt-gen/src/naming/name.rs +++ b/crates/cxx-qt-gen/src/naming/name.rs @@ -177,7 +177,7 @@ impl Name { /// Get the unqualified name of the type in Rust. /// This is either; /// - The rust_name attribute value, if one is provided - /// - The original ident, of no rust_name was provided + /// - The original ident, if no rust_name was provided pub fn rust_unqualified(&self) -> &Ident { &self.rust } diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index a947431d2..34763d750 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -43,7 +43,7 @@ impl QPropertyFlag { } _ => Err(Error::new( meta_value.span(), - "Invalid syntax, flags must be specified as either `read` or `read = 'my_getter'`", + "Invalid syntax, flags must be specified as either `read` or `read = my_getter`", )), } } From fe3ecfdcecd123061c60b72896de022f8deb1078 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Thu, 11 Jul 2024 14:21:41 +0100 Subject: [PATCH 07/18] Misc changes before QPropertyFlag rewrite --- .../src/generator/cpp/property/meta.rs | 2 +- .../src/generator/cpp/property/mod.rs | 62 ++++++++++++------- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs b/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs index c32a45330..3ddbcff0c 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs @@ -19,7 +19,7 @@ pub fn generate(idents: &QPropertyNames, cxx_ty: &str) -> String { } // Notify if let Some(name) = &idents.notify { - output += format!("NOTIFY {} ", name.cxx_unqualified()).as_str(); + output += format!("NOTIFY {}", name.cxx_unqualified()).as_str(); } output += ")"; diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs index a719225ac..291b0f740 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs @@ -37,17 +37,8 @@ pub fn generate_cpp_properties( let mut includes_read = false; // If the HashSet includes entries read must be specified otherwise it is an error - for flag in &property.flags { + for flag in property.flags.iter() { match flag { - QPropertyFlag::Write(_) => { - // Gen setters - generated - .methods - .push(setter::generate(&idents, &qobject_ident, &cxx_ty)); - generated - .private_methods - .push(setter::generate_wrapper(&idents, &cxx_ty)); - } QPropertyFlag::Read(_) => { includes_read = true; // Gen Getters @@ -58,6 +49,15 @@ pub fn generate_cpp_properties( .private_methods .push(getter::generate_wrapper(&idents, &cxx_ty)); } + QPropertyFlag::Write(_) => { + // Gen setters + generated + .methods + .push(setter::generate(&idents, &qobject_ident, &cxx_ty)); + generated + .private_methods + .push(setter::generate_wrapper(&idents, &cxx_ty)); + } QPropertyFlag::Notify(_) => { // Gen signal signals.push(signal::generate(&idents, qobject_idents)); @@ -109,23 +109,37 @@ mod tests { type_names.mock_insert("i32", None, None, None); let generated = generate_cpp_properties(&properties, &qobject_idents, &type_names).unwrap(); - println!("generated code: \n{:?}", generated.metaobjects); - println!("generated methods: \n{:?}", generated.methods); + println!("generated code: \n{:?}\n", generated.metaobjects); + println!("generated methods: \n{:?}\n\n", generated.methods); } #[test] fn test_generate_cpp_properties() { + // let properties = vec![ + // ParsedQProperty { + // ident: format_ident!("trivial_property"), + // ty: parse_quote! { i32 }, + // flags: Default::default(), + // }, + // ParsedQProperty { + // ident: format_ident!("opaque_property"), + // ty: parse_quote! { UniquePtr }, + // flags: Default::default(), + // }, + // ]; + let mut input1: ItemStruct = parse_quote! { + #[qproperty(i32, trivial_property, read, write, notify)] + struct MyStruct; + }; + + let mut input2: ItemStruct = parse_quote! { + #[qproperty(UniquePtr, opaque_property)] + struct MyStruct; + }; + let properties = vec![ - ParsedQProperty { - ident: format_ident!("trivial_property"), - ty: parse_quote! { i32 }, - flags: Default::default(), - }, - ParsedQProperty { - ident: format_ident!("opaque_property"), - ty: parse_quote! { UniquePtr }, - flags: Default::default(), - }, + ParsedQProperty::parse(input1.attrs.remove(0)).unwrap(), + ParsedQProperty::parse(input2.attrs.remove(0)).unwrap(), ]; let qobject_idents = create_qobjectname(); @@ -134,6 +148,8 @@ mod tests { type_names.mock_insert("QColor", None, None, None); let generated = generate_cpp_properties(&properties, &qobject_idents, &type_names).unwrap(); + println!("\ngenerated meta: {:?}\n\ngenerated methods: {:?}\n\n", generated.metaobjects, generated.methods); + // metaobjects assert_eq!(generated.metaobjects.len(), 2); assert_str_eq!(generated.metaobjects[0], "Q_PROPERTY(::std::int32_t trivialProperty READ getTrivialProperty WRITE setTrivialProperty NOTIFY trivialPropertyChanged)"); @@ -146,6 +162,7 @@ mod tests { } else { panic!("Expected pair!") }; + assert_str_eq!(header, "::std::int32_t const& getTrivialProperty() const;"); assert_str_eq!( source, @@ -185,6 +202,7 @@ mod tests { } else { panic!("Expected pair!") }; + assert_str_eq!( header, "::std::unique_ptr const& getOpaqueProperty() const;" From 1fa65fa2f6e271d0a15306215a23289d0602b8b8 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Thu, 11 Jul 2024 17:05:16 +0100 Subject: [PATCH 08/18] Refactor to QPropertyFlags struct with Options<> instead of a HashSet. --- .../src/generator/cpp/property/mod.rs | 58 ++++---- .../src/generator/naming/property.rs | 56 ++++---- .../src/generator/rust/property/mod.rs | 7 +- crates/cxx-qt-gen/src/parser/property.rs | 136 +++++++++++------- 4 files changed, 149 insertions(+), 108 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs index 291b0f740..25d94dcb1 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs @@ -10,7 +10,7 @@ use crate::generator::{ use crate::{ naming::cpp::syn_type_to_cpp_type, naming::TypeNames, - parser::property::{ParsedQProperty, QPropertyFlag}, + parser::property::{ParsedQProperty, QPropertyFlags}, }; use syn::{Error, Result}; @@ -37,33 +37,33 @@ pub fn generate_cpp_properties( let mut includes_read = false; // If the HashSet includes entries read must be specified otherwise it is an error - for flag in property.flags.iter() { - match flag { - QPropertyFlag::Read(_) => { - includes_read = true; - // Gen Getters - generated - .methods - .push(getter::generate(&idents, &qobject_ident, &cxx_ty)); - generated - .private_methods - .push(getter::generate_wrapper(&idents, &cxx_ty)); - } - QPropertyFlag::Write(_) => { - // Gen setters - generated - .methods - .push(setter::generate(&idents, &qobject_ident, &cxx_ty)); - generated - .private_methods - .push(setter::generate_wrapper(&idents, &cxx_ty)); - } - QPropertyFlag::Notify(_) => { - // Gen signal - signals.push(signal::generate(&idents, qobject_idents)); - } - } - } + // for flag in property.flags.iter() { + // match flag { + // QPropertyFlag::Read(_) => { + // includes_read = true; + // // Gen Getters + // generated + // .methods + // .push(getter::generate(&idents, &qobject_ident, &cxx_ty)); + // generated + // .private_methods + // .push(getter::generate_wrapper(&idents, &cxx_ty)); + // } + // QPropertyFlag::Write(_) => { + // // Gen setters + // generated + // .methods + // .push(setter::generate(&idents, &qobject_ident, &cxx_ty)); + // generated + // .private_methods + // .push(setter::generate_wrapper(&idents, &cxx_ty)); + // } + // QPropertyFlag::Notify(_) => { + // // Gen signal + // signals.push(signal::generate(&idents, qobject_idents)); + // } + // } + // } if !includes_read { return Err(Error::new( @@ -425,7 +425,7 @@ mod tests { let properties = vec![ParsedQProperty { ident: format_ident!("mapped_property"), ty: parse_quote! { A }, - flags: Default::default(), + flags: QPropertyFlags::new(), }]; let qobject_idents = create_qobjectname(); diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs index 178f0092b..7d642c0fa 100644 --- a/crates/cxx-qt-gen/src/generator/naming/property.rs +++ b/crates/cxx-qt-gen/src/generator/naming/property.rs @@ -4,7 +4,7 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 use crate::{ naming::Name, - parser::property::{ParsedQProperty, QPropertyFlag}, + parser::property::{ParsedQProperty, QPropertyFlags}, }; use convert_case::{Case, Casing}; use quote::format_ident; @@ -28,32 +28,32 @@ impl From<&ParsedQProperty> for QPropertyNames { let mut setter: Option = None; let mut notify: Option = None; - for flag in &property.flags { - match flag { - QPropertyFlag::Write(ref signature) => { - // TODO: remove if let blocks (passing custom func should not change name of getter, only its contents) - if let Some(ident) = signature { - setter = Some(Name::new(ident.clone())) - } else { - setter = Some(setter_name_from_property(&property_name)) - } - } - QPropertyFlag::Read(ref signature) => { - if let Some(ident) = signature { - getter = Name::new(ident.clone()) - } else { - getter = getter_name_from_property(&property_name) - } - } - QPropertyFlag::Notify(ref signature) => { - if let Some(ident) = signature { - notify = Some(Name::new(ident.clone())) - } else { - notify = Some(notify_name_from_property(&property_name)) - } - } - } - } + // for flag in &property.flags { + // match flag { + // QPropertyFlag::Write(ref signature) => { + // // TODO: remove if let blocks (passing custom func should not change name of getter, only its contents) + // if let Some(ident) = signature { + // setter = Some(Name::new(ident.clone())) + // } else { + // setter = Some(setter_name_from_property(&property_name)) + // } + // } + // QPropertyFlag::Read(ref signature) => { + // if let Some(ident) = signature { + // getter = Name::new(ident.clone()) + // } else { + // getter = getter_name_from_property(&property_name) + // } + // } + // QPropertyFlag::Notify(ref signature) => { + // if let Some(ident) = signature { + // notify = Some(Name::new(ident.clone())) + // } else { + // notify = Some(notify_name_from_property(&property_name)) + // } + // } + // } + // } let setter_wrapper: Option; if let Some(name) = &setter { setter_wrapper = Some(wrapper_name_from_function_name(name)); @@ -122,7 +122,7 @@ pub mod tests { let property = ParsedQProperty { ident: format_ident!("my_property"), ty, - flags: Default::default(), + flags: QPropertyFlags::new(), }; QPropertyNames::from(&property) // Doesn't account for emtpy flags, maybe change this to the quote macro } diff --git a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs index 4026bc3aa..64eba3291 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs @@ -70,6 +70,7 @@ mod tests { use crate::{generator::naming::qobject::tests::create_qobjectname, tests::assert_tokens_eq}; use quote::format_ident; use syn::parse_quote; + use crate::parser::property::QPropertyFlags; #[test] fn test_generate_rust_properties() { @@ -77,17 +78,17 @@ mod tests { ParsedQProperty { ident: format_ident!("trivial_property"), ty: parse_quote! { i32 }, - flags: Default::default(), + flags: QPropertyFlags::new(), }, ParsedQProperty { ident: format_ident!("opaque_property"), ty: parse_quote! { UniquePtr }, - flags: Default::default(), + flags: QPropertyFlags::new(), }, ParsedQProperty { ident: format_ident!("unsafe_property"), ty: parse_quote! { *mut T }, - flags: Default::default(), + flags: QPropertyFlags::new(), }, ]; let qobject_idents = create_qobjectname(); diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index 34763d750..08989b183 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -12,40 +12,75 @@ use syn::{ Attribute, Expr, Ident, Meta, MetaNameValue, Path, Result, Token, Type, }; -/// Enum type for optional flags in qproperty macro -/// Can contain a string identifier of a custom getter, setter or notifier -#[derive(Debug, Eq, PartialEq, Hash)] -pub enum QPropertyFlag { - Read(Option), - Write(Option), - Notify(Option), +use quote::format_ident; + +/// An optional identifier of the functions passed with flags +/// e.g. read = my_getter, IdentFlag would be used to store my_getter +type IdentFlag = Option; + +/// Struct for storing the flags provided for a QProperty and their optional identifiers ([IdentFlag]) +#[derive(Debug)] +pub struct QPropertyFlags { + read: Option, // TODO: maybe change this to better represent the data + write: Option, + notify: Option, } -impl QPropertyFlag { - fn from_ident(identifier: Ident, value: Option) -> Result { - return match identifier.to_string().as_str() { - "read" => Ok(QPropertyFlag::Read(value)), - "write" => Ok(QPropertyFlag::Write(value)), - "notify" => Ok(QPropertyFlag::Notify(value)), - _ => Err(Error::new( - identifier.span(), - "Flags must be one of read, write or notify", - )), +impl QPropertyFlags { + pub fn new() -> Self { + Self { + read: None, + write: None, + notify: None, + } + } + + pub fn all_flags() -> Self { + Self { + read: None, + write: Some(None), + notify: Some(None), + } + } + + fn set_field_by_string(&mut self, key: String, value: IdentFlag) -> Result<()> { + match key.as_str() { + "read" => self.read = Some(value), + "write" => self.write = Some(value), + "notify" => self.notify = Some(value), + _ => { + return Err(Error::new( + format_ident!("{}", key).span(), // TODO: check if this is an acceptable form of erroring for non Syn functions + "Invalid flag passed, must be one of read, write, notify", + )); + } }; + Ok(()) } - fn from_meta(meta_value: Meta) -> Result { + fn add_from_meta(&mut self, meta_value: Meta) -> Result<()> { match meta_value { - Meta::Path(path) => Ok(Self::from_ident(parse_path_to_ident(&path), None)?), + Meta::Path(path) => { + let ident: String = parse_path_to_ident(&path).to_string(); + + self.set_field_by_string(ident, None)?; + Ok(()) + } Meta::NameValue(name_value) => { let kv_pair = parse_meta_name_value(&name_value)?; - Ok(Self::from_ident(kv_pair.0, Some(kv_pair.1))?) + + let ident: String = kv_pair.0.to_string(); + let value: IdentFlag = Some(kv_pair.1); + + self.set_field_by_string(ident, value)?; + Ok(()) } _ => Err(Error::new( meta_value.span(), "Invalid syntax, flags must be specified as either `read` or `read = my_getter`", )), - } + }; + Ok(()) } } @@ -55,14 +90,15 @@ pub struct ParsedQProperty { pub ident: Ident, /// The [syn::Type] of the property pub ty: Type, - /// HashSet of [QPropertyFlag]s which were specified - pub flags: HashSet, + /// Property flag collection + pub flags: QPropertyFlags, } fn parse_path_to_ident(path: &Path) -> Ident { path.segments[0].ident.clone() } +// Returning struct instead of tuple might be more descriptive fn parse_meta_name_value(name_value: &MetaNameValue) -> Result<(Ident, Ident)> { let ident = parse_path_to_ident(&name_value.path); let expr = &name_value.value; @@ -87,18 +123,12 @@ impl ParsedQProperty { let _comma = input.parse::()?; let ident = input.parse()?; - let mut flag_set = HashSet::new(); - if input.is_empty() { - flag_set.insert(QPropertyFlag::Read(None)); - flag_set.insert(QPropertyFlag::Write(None)); - flag_set.insert(QPropertyFlag::Notify(None)); - // No flags so fill with default options Ok(Self { ident, ty, - flags: flag_set, + flags: QPropertyFlags::all_flags(), }) } else { let _comma = input.parse::()?; // Start of final identifiers @@ -108,16 +138,25 @@ impl ParsedQProperty { let flags: Vec = punctuated_flags.into_iter().collect(); // Removes the commas while collecting into Vec - let mut flag_set: HashSet = HashSet::new(); + let mut passed_flags = QPropertyFlags::new(); for flag in flags { - flag_set.insert(QPropertyFlag::from_meta(flag)?); + passed_flags.add_from_meta(flag)?; + } + + println!("Final flags: {:?}", passed_flags); + + if passed_flags.read.is_none() { + return Err(Error::new( + input.span(), + "If flags are passed, read must be explicitly specified", + )); } Ok(Self { ident, ty, - flags: flag_set, + flags: passed_flags, }) } }) @@ -151,7 +190,7 @@ mod tests { let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); assert_eq!(property.ident, format_ident!("name")); assert_eq!(property.ty, parse_quote! { T }); - assert!(property.flags.contains(&QPropertyFlag::Read(None))); + // assert!(property.flags.contains(&QPropertyFlags::Read(None))); } #[test] @@ -163,9 +202,9 @@ mod tests { let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); assert_eq!(property.ident, format_ident!("name")); assert_eq!(property.ty, parse_quote! { T }); - assert!(property.flags.contains(&QPropertyFlag::Read(None))); - assert!(property.flags.contains(&QPropertyFlag::Write(None))); - assert!(property.flags.contains(&QPropertyFlag::Notify(None))); + // assert!(property.flags.contains(&QPropertyFlags::Read(None))); + // assert!(property.flags.contains(&QPropertyFlags::Write(None))); + // assert!(property.flags.contains(&QPropertyFlags::Notify(None))); } #[test] @@ -177,13 +216,13 @@ mod tests { let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); assert_eq!(property.ident, format_ident!("name")); assert_eq!(property.ty, parse_quote! { T }); - assert!(property - .flags - .contains(&QPropertyFlag::Read(Some(format_ident!("blah"))))); - assert!(property.flags.contains(&QPropertyFlag::Write(None))); - assert!(property - .flags - .contains(&QPropertyFlag::Notify(Some(format_ident!("blahblah"))))); + // assert!(property + // .flags + // .contains(&QPropertyFlag::Read(Some(format_ident!("blah"))))); + // assert!(property.flags.contains(&QPropertyFlag::Write(None))); + // assert!(property + // .flags + // .contains(&QPropertyFlag::Notify(Some(format_ident!("blahblah"))))); } #[test] @@ -202,10 +241,11 @@ mod tests { #[qproperty(T, name, notify = blahblah)] struct MyStruct; }; - let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); - assert!(property - .flags - .contains(&QPropertyFlag::Notify(Some(format_ident!("blahblah"))))) + let property = ParsedQProperty::parse(input.attrs.remove(0)); + assert!(property.is_err()); + // assert!(property + // .flags + // .contains(&QPropertyFlag::Notify(Some(format_ident!("blahblah"))))) } #[test] From 865d7fed5524696568da96e728d0a1f04b985840 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Fri, 12 Jul 2024 10:05:06 +0100 Subject: [PATCH 09/18] Rewrite property parser with new layout for storing flags --- crates/cxx-qt-gen/src/parser/property.rs | 64 +++++++++++------------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index 08989b183..536303562 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -21,7 +21,7 @@ type IdentFlag = Option; /// Struct for storing the flags provided for a QProperty and their optional identifiers ([IdentFlag]) #[derive(Debug)] pub struct QPropertyFlags { - read: Option, // TODO: maybe change this to better represent the data + read: IdentFlag, // TODO: maybe change this to better represent the data write: Option, notify: Option, } @@ -43,9 +43,9 @@ impl QPropertyFlags { } } - fn set_field_by_string(&mut self, key: String, value: IdentFlag) -> Result<()> { + fn set_field_by_string(&mut self, key: String, value: IdentFlag) -> Result { match key.as_str() { - "read" => self.read = Some(value), + "read" => self.read = value, "write" => self.write = Some(value), "notify" => self.notify = Some(value), _ => { @@ -55,16 +55,15 @@ impl QPropertyFlags { )); } }; - Ok(()) + Ok(key) // Return the string back up for checking which flags were set } - fn add_from_meta(&mut self, meta_value: Meta) -> Result<()> { + fn add_from_meta(&mut self, meta_value: Meta) -> Result { match meta_value { Meta::Path(path) => { let ident: String = parse_path_to_ident(&path).to_string(); - self.set_field_by_string(ident, None)?; - Ok(()) + Ok(self.set_field_by_string(ident, None)?) } Meta::NameValue(name_value) => { let kv_pair = parse_meta_name_value(&name_value)?; @@ -72,15 +71,13 @@ impl QPropertyFlags { let ident: String = kv_pair.0.to_string(); let value: IdentFlag = Some(kv_pair.1); - self.set_field_by_string(ident, value)?; - Ok(()) + Ok(self.set_field_by_string(ident, value)?) } _ => Err(Error::new( meta_value.span(), "Invalid syntax, flags must be specified as either `read` or `read = my_getter`", )), - }; - Ok(()) + } } } @@ -140,13 +137,14 @@ impl ParsedQProperty { let mut passed_flags = QPropertyFlags::new(); + let mut flag_strings: Vec = Vec::new(); + for flag in flags { - passed_flags.add_from_meta(flag)?; + let flag_string: String = passed_flags.add_from_meta(flag)?; + flag_strings.push(flag_string); } - println!("Final flags: {:?}", passed_flags); - - if passed_flags.read.is_none() { + if !flag_strings.contains(&String::from("read")) { return Err(Error::new( input.span(), "If flags are passed, read must be explicitly specified", @@ -182,7 +180,7 @@ mod tests { } #[test] - fn test_parse_read_flag() { + fn test_parse_flags_read() { let mut input: ItemStruct = parse_quote! { #[qproperty(T, name, read)] struct MyStruct; @@ -190,11 +188,10 @@ mod tests { let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); assert_eq!(property.ident, format_ident!("name")); assert_eq!(property.ty, parse_quote! { T }); - // assert!(property.flags.contains(&QPropertyFlags::Read(None))); } #[test] - fn test_parse_all_flags() { + fn test_parse_flags_all() { let mut input: ItemStruct = parse_quote! { #[qproperty(T, name, read, write, notify)] struct MyStruct; @@ -202,13 +199,12 @@ mod tests { let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); assert_eq!(property.ident, format_ident!("name")); assert_eq!(property.ty, parse_quote! { T }); - // assert!(property.flags.contains(&QPropertyFlags::Read(None))); - // assert!(property.flags.contains(&QPropertyFlags::Write(None))); - // assert!(property.flags.contains(&QPropertyFlags::Notify(None))); + assert!(property.flags.write.is_some()); + assert!(property.flags.notify.is_some()); } #[test] - fn test_parse_kwargs() { + fn test_parse_flags_kw() { let mut input: ItemStruct = parse_quote! { #[qproperty(T, name, read = blah, write, notify = blahblah)] struct MyStruct; @@ -216,17 +212,20 @@ mod tests { let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); assert_eq!(property.ident, format_ident!("name")); assert_eq!(property.ty, parse_quote! { T }); - // assert!(property - // .flags - // .contains(&QPropertyFlag::Read(Some(format_ident!("blah"))))); - // assert!(property.flags.contains(&QPropertyFlag::Write(None))); - // assert!(property - // .flags - // .contains(&QPropertyFlag::Notify(Some(format_ident!("blahblah"))))); + + assert!(property.flags.read.is_some()); + assert_eq!(property.flags.read.unwrap(), format_ident!("blah")); + + assert!(property.flags.write.is_some()); + + assert!(property.flags.notify.is_some()); + let notify = property.flags.notify.unwrap(); + assert!(notify.is_some()); + assert_eq!(notify.unwrap(), format_ident!("blahblah")); } #[test] - fn test_parse_invalid_flag() { + fn test_parse_flags_invalid() { let mut input: ItemStruct = parse_quote! { #[qproperty(T, name, read = blah, a, notify = blahblah)] struct MyStruct; @@ -243,9 +242,6 @@ mod tests { }; let property = ParsedQProperty::parse(input.attrs.remove(0)); assert!(property.is_err()); - // assert!(property - // .flags - // .contains(&QPropertyFlag::Notify(Some(format_ident!("blahblah"))))) } #[test] @@ -291,7 +287,7 @@ mod tests { #[test] fn test_parse_property_no_type() { let mut input: ItemStruct = parse_quote! { - #[qproperty(T)] + #[qproperty(num)] struct MyStruct; }; let property = ParsedQProperty::parse(input.attrs.remove(0)); From 209bff4ca6f1cf22d729eaf8a7fff18971ba989d Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Tue, 16 Jul 2024 12:04:56 +0100 Subject: [PATCH 10/18] Improve Error checking and refactor the generators --- .../src/generator/cpp/property/meta.rs | 21 ++-- .../src/generator/cpp/property/mod.rs | 104 +++++++--------- .../src/generator/cpp/property/setter.rs | 66 +++++----- .../src/generator/cpp/property/signal.rs | 38 +++--- .../src/generator/naming/property.rs | 97 ++++++--------- .../src/generator/rust/property/mod.rs | 69 +++++++---- .../src/generator/rust/property/setter.rs | 116 ++++++++---------- .../src/generator/rust/property/signal.rs | 33 ++--- crates/cxx-qt-gen/src/parser/property.rs | 65 +++++----- crates/cxx-qt-gen/test_inputs/properties.rs | 1 + crates/cxx-qt-gen/test_outputs/properties.cpp | 58 +++++++++ crates/cxx-qt-gen/test_outputs/properties.h | 17 +++ crates/cxx-qt-gen/test_outputs/properties.rs | 100 +++++++++++++++ 13 files changed, 464 insertions(+), 321 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs b/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs index 3ddbcff0c..8ea5af1aa 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs @@ -7,22 +7,23 @@ use crate::generator::naming::property::QPropertyNames; /// Generate the metaobject line for a given property pub fn generate(idents: &QPropertyNames, cxx_ty: &str) -> String { - let mut output: String = format!( - "Q_PROPERTY({ty} {ident} READ {ident_getter} ", - ty = cxx_ty, - ident = idents.name.cxx_unqualified(), + let mut parts = vec![format!( + "READ {ident_getter}", ident_getter = idents.getter.cxx_unqualified() - ); + )]; // Write if let Some(name) = &idents.setter { - output += format!("WRITE {} ", name.cxx_unqualified()).as_str(); + parts.push(format!("WRITE {}", name.cxx_unqualified())); } // Notify if let Some(name) = &idents.notify { - output += format!("NOTIFY {}", name.cxx_unqualified()).as_str(); + parts.push(format!("NOTIFY {}", name.cxx_unqualified())); } - output += ")"; - - output + format!( + "Q_PROPERTY({ty} {ident} {meta_parts})", + ty = cxx_ty, + ident = idents.name.cxx_unqualified(), + meta_parts = parts.join(" ") + ) } diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs index 25d94dcb1..a9d971569 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs @@ -8,11 +8,9 @@ use crate::generator::{ naming::{property::QPropertyNames, qobject::QObjectNames}, }; use crate::{ - naming::cpp::syn_type_to_cpp_type, - naming::TypeNames, - parser::property::{ParsedQProperty, QPropertyFlags}, + naming::cpp::syn_type_to_cpp_type, naming::TypeNames, parser::property::ParsedQProperty, }; -use syn::{Error, Result}; +use syn::Result; mod getter; mod meta; @@ -33,43 +31,44 @@ pub fn generate_cpp_properties( let idents = QPropertyNames::from(property); let cxx_ty = syn_type_to_cpp_type(&property.ty, type_names)?; + let flags = &property.flags; + generated.metaobjects.push(meta::generate(&idents, &cxx_ty)); - let mut includes_read = false; // If the HashSet includes entries read must be specified otherwise it is an error - - // for flag in property.flags.iter() { - // match flag { - // QPropertyFlag::Read(_) => { - // includes_read = true; - // // Gen Getters - // generated - // .methods - // .push(getter::generate(&idents, &qobject_ident, &cxx_ty)); - // generated - // .private_methods - // .push(getter::generate_wrapper(&idents, &cxx_ty)); - // } - // QPropertyFlag::Write(_) => { - // // Gen setters - // generated - // .methods - // .push(setter::generate(&idents, &qobject_ident, &cxx_ty)); - // generated - // .private_methods - // .push(setter::generate_wrapper(&idents, &cxx_ty)); - // } - // QPropertyFlag::Notify(_) => { - // // Gen signal - // signals.push(signal::generate(&idents, qobject_idents)); - // } - // } - // } - - if !includes_read { - return Err(Error::new( - property.ident.span(), - "If other flags are specified, read must also be specified", - )); + // None indicates no custom identifier was provided + if flags.read.is_none() { + generated + .methods + .push(getter::generate(&idents, &qobject_ident, &cxx_ty)); + generated + .private_methods + .push(getter::generate_wrapper(&idents, &cxx_ty)); + } + + // Checking custom setter wasn't provided + if flags + .write + .clone() + .is_some_and(|ident_option| ident_option.is_none()) + { + if let Some(setter) = setter::generate(&idents, &qobject_ident, &cxx_ty) { + generated.methods.push(setter) + } + + if let Some(setter_wrapper) = setter::generate_wrapper(&idents, &cxx_ty) { + generated.private_methods.push(setter_wrapper) + } + } + + // Checking custom notifier wasn't provided + if flags + .notify + .clone() + .is_some_and(|ident_option| ident_option.is_none()) + { + if let Some(notify) = signal::generate(&idents, qobject_idents) { + signals.push(notify) + } } } @@ -86,6 +85,8 @@ pub fn generate_cpp_properties( mod tests { use super::*; + use crate::parser::property::QPropertyFlags; + use crate::generator::naming::qobject::tests::create_qobjectname; use crate::CppFragment; use indoc::indoc; @@ -105,28 +106,13 @@ mod tests { let qobject_idents = create_qobjectname(); - let mut type_names = TypeNames::mock(); - type_names.mock_insert("i32", None, None, None); - let generated = generate_cpp_properties(&properties, &qobject_idents, &type_names).unwrap(); - - println!("generated code: \n{:?}\n", generated.metaobjects); - println!("generated methods: \n{:?}\n\n", generated.methods); + let type_names = TypeNames::mock(); + let _generated = + generate_cpp_properties(&properties, &qobject_idents, &type_names).unwrap(); } #[test] fn test_generate_cpp_properties() { - // let properties = vec![ - // ParsedQProperty { - // ident: format_ident!("trivial_property"), - // ty: parse_quote! { i32 }, - // flags: Default::default(), - // }, - // ParsedQProperty { - // ident: format_ident!("opaque_property"), - // ty: parse_quote! { UniquePtr }, - // flags: Default::default(), - // }, - // ]; let mut input1: ItemStruct = parse_quote! { #[qproperty(i32, trivial_property, read, write, notify)] struct MyStruct; @@ -148,8 +134,6 @@ mod tests { type_names.mock_insert("QColor", None, None, None); let generated = generate_cpp_properties(&properties, &qobject_idents, &type_names).unwrap(); - println!("\ngenerated meta: {:?}\n\ngenerated methods: {:?}\n\n", generated.metaobjects, generated.methods); - // metaobjects assert_eq!(generated.metaobjects.len(), 2); assert_str_eq!(generated.metaobjects[0], "Q_PROPERTY(::std::int32_t trivialProperty READ getTrivialProperty WRITE setTrivialProperty NOTIFY trivialPropertyChanged)"); @@ -425,7 +409,7 @@ mod tests { let properties = vec![ParsedQProperty { ident: format_ident!("mapped_property"), ty: parse_quote! { A }, - flags: QPropertyFlags::new(), + flags: QPropertyFlags::default(), }]; let qobject_idents = create_qobjectname(); diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs index 4bd427f3a..23422102b 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs @@ -6,41 +6,39 @@ use crate::generator::{cpp::fragment::CppFragment, naming::property::QPropertyNames}; use indoc::formatdoc; -// TODO: modify func to return result for proper erroring -pub fn generate(idents: &QPropertyNames, qobject_ident: &str, cxx_ty: &str) -> CppFragment { - CppFragment::Pair { - header: format!( - "Q_SLOT void {ident_setter}({cxx_ty} const& value);", - ident_setter = idents - .setter - .clone() - .expect("Setter was empty") - .cxx_unqualified(), - ), - source: formatdoc! { - r#" - void - {qobject_ident}::{ident_setter}({cxx_ty} const& value) - {{ - const ::rust::cxxqt1::MaybeLockGuard<{qobject_ident}> guard(*this); - {ident_setter_wrapper}(value); - }} - "#, - ident_setter = idents.setter.clone().expect("Setter was empty").cxx_unqualified(), - ident_setter_wrapper = idents.setter_wrapper.clone().expect("Setter was empty").cxx_unqualified(), - }, +pub fn generate(idents: &QPropertyNames, qobject_ident: &str, cxx_ty: &str) -> Option { + // Checking whether a setter should be generated based off if it has a name + if let (Some(setter), Some(setter_wrapper)) = (&idents.setter, &idents.setter_wrapper) { + Some(CppFragment::Pair { + header: format!( + "Q_SLOT void {ident_setter}({cxx_ty} const& value);", + ident_setter = setter.cxx_unqualified(), + ), + source: formatdoc! { + r#" + void + {qobject_ident}::{ident_setter}({cxx_ty} const& value) + {{ + const ::rust::cxxqt1::MaybeLockGuard<{qobject_ident}> guard(*this); + {ident_setter_wrapper}(value); + }} + "#, + ident_setter = setter.cxx_unqualified(), + ident_setter_wrapper = setter_wrapper.cxx_unqualified(), + }, + }) + } else { + None } } -pub fn generate_wrapper(idents: &QPropertyNames, cxx_ty: &str) -> CppFragment { - CppFragment::Header(format!( - // Note that we pass T not const T& to Rust so that it is by-value - // https://github.com/KDAB/cxx-qt/issues/463 - "void {ident_setter_wrapper}({cxx_ty} value) noexcept;", - ident_setter_wrapper = idents - .setter_wrapper - .clone() - .expect("Setter was empty") - .cxx_unqualified() - )) +pub fn generate_wrapper(idents: &QPropertyNames, cxx_ty: &str) -> Option { + idents.setter_wrapper.as_ref().map(|setter_wrapper| { + CppFragment::Header(format!( + // Note that we pass T not const T& to Rust so that it is by-value + // https://github.com/KDAB/cxx-qt/issues/463 + "void {ident_setter_wrapper}({cxx_ty} value) noexcept;", + ident_setter_wrapper = setter_wrapper.cxx_unqualified() + )) + }) } diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs b/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs index 11f2ad786..a5d090852 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs @@ -3,34 +3,32 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use syn::{spanned::Spanned, Error, ForeignItemFn}; +use syn::ForeignItemFn; use crate::{ generator::naming::{property::QPropertyNames, qobject::QObjectNames}, parser::signals::ParsedSignal, }; -pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> ParsedSignal { +pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Option { // We build our signal in the generation phase as we need to use the naming // structs to build the signal name let cpp_class_rust = &qobject_idents.name.rust_unqualified(); - let notify_binding = &idents.notify.clone().expect("Notify was empty!"); - // TODO: modify the func to return result - // let notify_binding = match &idents.notify { - // Some(notify) => notift, - // _ => return Err(Error::new(cxx_ty.span(), "Property did not include a notify field")) - // }; + if let Some(notify) = &idents.notify { + let notify_cpp = notify.cxx_unqualified(); + let notify_rust = notify.rust_unqualified(); + let method: ForeignItemFn = syn::parse_quote! { + #[doc = "Notify for the Q_PROPERTY"] + #[cxx_name = #notify_cpp] + fn #notify_rust(self: Pin<&mut #cpp_class_rust>); + }; - let notify_cpp = notify_binding.cxx_unqualified(); - let notify_rust = notify_binding.rust_unqualified(); - let method: ForeignItemFn = syn::parse_quote! { - #[doc = "Notify for the Q_PROPERTY"] - #[cxx_name = #notify_cpp] - fn #notify_rust(self: Pin<&mut #cpp_class_rust>); - }; - ParsedSignal::from_property_method( - method, - idents.notify.clone().expect("Notify was empty!"), - qobject_idents.name.rust_unqualified().clone(), - ) + Some(ParsedSignal::from_property_method( + method, + notify.clone(), + qobject_idents.name.rust_unqualified().clone(), + )) + } else { + None + } } diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs index 7d642c0fa..73d7e8ef1 100644 --- a/crates/cxx-qt-gen/src/generator/naming/property.rs +++ b/crates/cxx-qt-gen/src/generator/naming/property.rs @@ -2,10 +2,7 @@ // SPDX-FileContributor: Andrew Hayzen // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::{ - naming::Name, - parser::property::{ParsedQProperty, QPropertyFlags}, -}; +use crate::{naming::Name, parser::property::ParsedQProperty}; use convert_case::{Case, Casing}; use quote::format_ident; use syn::Ident; @@ -24,42 +21,33 @@ impl From<&ParsedQProperty> for QPropertyNames { fn from(property: &ParsedQProperty) -> Self { let property_name = property_name_from_rust_name(property.ident.clone()); - let mut getter: Name = getter_name_from_property(&property_name); //TODO: potentially declaring this twice, might be fixed when tackling other todo in this file - let mut setter: Option = None; - let mut notify: Option = None; - - // for flag in &property.flags { - // match flag { - // QPropertyFlag::Write(ref signature) => { - // // TODO: remove if let blocks (passing custom func should not change name of getter, only its contents) - // if let Some(ident) = signature { - // setter = Some(Name::new(ident.clone())) - // } else { - // setter = Some(setter_name_from_property(&property_name)) - // } - // } - // QPropertyFlag::Read(ref signature) => { - // if let Some(ident) = signature { - // getter = Name::new(ident.clone()) - // } else { - // getter = getter_name_from_property(&property_name) - // } - // } - // QPropertyFlag::Notify(ref signature) => { - // if let Some(ident) = signature { - // notify = Some(Name::new(ident.clone())) - // } else { - // notify = Some(notify_name_from_property(&property_name)) - // } - // } - // } - // } - let setter_wrapper: Option; - if let Some(name) = &setter { - setter_wrapper = Some(wrapper_name_from_function_name(name)); - } else { - setter_wrapper = None; - } + let flags = &property.flags; + + // Match for custom name or autogenerate one + let getter = match &flags.read { + Some(ident) => Name::new(ident.clone()), + None => getter_name_from_property(&property_name), + }; + + // Match for custom name, autogenerate, or None + // TODO: Refactor to use an enum type to represent these 3 states better + let setter = match &flags.write { + Some(ident_option) => match ident_option { + Some(ident) => Some(Name::new(ident.clone())), + None => Some(setter_name_from_property(&property_name)), + }, + None => None, + }; + + let notify = match &flags.notify { + Some(ident_option) => match ident_option { + Some(ident) => Some(Name::new(ident.clone())), + None => Some(notify_name_from_property(&property_name)), + }, + None => None, + }; + + let setter_wrapper = setter.as_ref().map(wrapper_name_from_function_name); Self { getter_wrapper: wrapper_name_from_function_name(&getter), @@ -116,15 +104,16 @@ pub mod tests { use syn::parse_quote; use super::*; + use crate::parser::property::QPropertyFlags; pub fn create_i32_qpropertyname() -> QPropertyNames { let ty: syn::Type = parse_quote! { i32 }; let property = ParsedQProperty { ident: format_ident!("my_property"), ty, - flags: QPropertyFlags::new(), + flags: QPropertyFlags::default(), }; - QPropertyNames::from(&property) // Doesn't account for emtpy flags, maybe change this to the quote macro + QPropertyNames::from(&property) } #[test] @@ -138,35 +127,19 @@ pub mod tests { &format_ident!("my_property") ); assert_eq!( - names - .setter - .clone() - .expect("Setter was empty") - .cxx_unqualified(), + names.setter.clone().unwrap().cxx_unqualified(), "setMyProperty" ); assert_eq!( - names - .setter - .clone() - .expect("Setter was empty") - .rust_unqualified(), + names.setter.clone().unwrap().rust_unqualified(), &format_ident!("set_my_property") ); assert_eq!( - names - .notify - .clone() - .expect("Notify was empty") - .cxx_unqualified(), + names.notify.clone().unwrap().cxx_unqualified(), "myPropertyChanged" ); assert_eq!( - names - .notify - .clone() - .expect("Notify was empty") - .rust_unqualified(), + names.notify.clone().unwrap().rust_unqualified(), &format_ident!("my_property_changed") ); } diff --git a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs index 64eba3291..ed07cd7c3 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs @@ -31,26 +31,47 @@ pub fn generate_rust_properties( for property in properties { let idents = QPropertyNames::from(property); - // Getters - let getter = getter::generate(&idents, qobject_idents, &property.ty, type_names)?; - generated - .cxx_mod_contents - .append(&mut getter.cxx_bridge_as_items()?); - generated - .cxx_qt_mod_contents - .append(&mut getter.implementation_as_items()?); - - // Setters - let setter = setter::generate(&idents, qobject_idents, &property.ty, type_names)?; - generated - .cxx_mod_contents - .append(&mut setter.cxx_bridge_as_items()?); - generated - .cxx_qt_mod_contents - .append(&mut setter.implementation_as_items()?); - - // Signals - signals.push(signal::generate(&idents, qobject_idents)); + let flags = &property.flags; + + if flags.read.is_none() { + //gen getter and wrapper + let getter = getter::generate(&idents, qobject_idents, &property.ty, type_names)?; + generated + .cxx_mod_contents + .append(&mut getter.cxx_bridge_as_items()?); + generated + .cxx_qt_mod_contents + .append(&mut getter.implementation_as_items()?); + } + + // Checking that write flag was provided but no custom identifier + if flags + .write + .clone() + .is_some_and(|ident_option| ident_option.is_none()) + { + // gen setter and wrapper + if let Some(setter) = + setter::generate(&idents, qobject_idents, &property.ty, type_names)? + { + generated + .cxx_mod_contents + .append(&mut setter.cxx_bridge_as_items()?); + generated + .cxx_qt_mod_contents + .append(&mut setter.implementation_as_items()?); + } + } + + if flags + .notify + .clone() + .is_some_and(|ident_option| ident_option.is_none()) + { + if let Some(notify) = signal::generate(&idents, qobject_idents) { + signals.push(notify) + } + } } generated.append(&mut generate_rust_signals( @@ -67,10 +88,10 @@ pub fn generate_rust_properties( mod tests { use super::*; + use crate::parser::property::QPropertyFlags; use crate::{generator::naming::qobject::tests::create_qobjectname, tests::assert_tokens_eq}; use quote::format_ident; use syn::parse_quote; - use crate::parser::property::QPropertyFlags; #[test] fn test_generate_rust_properties() { @@ -78,17 +99,17 @@ mod tests { ParsedQProperty { ident: format_ident!("trivial_property"), ty: parse_quote! { i32 }, - flags: QPropertyFlags::new(), + flags: QPropertyFlags::default(), }, ParsedQProperty { ident: format_ident!("opaque_property"), ty: parse_quote! { UniquePtr }, - flags: QPropertyFlags::new(), + flags: QPropertyFlags::default(), }, ParsedQProperty { ident: format_ident!("unsafe_property"), ty: parse_quote! { *mut T }, - flags: QPropertyFlags::new(), + flags: QPropertyFlags::default(), }, ]; let qobject_idents = create_qobjectname(); diff --git a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs index 294dd07d2..d6649229c 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs @@ -12,84 +12,70 @@ use crate::{ naming::TypeNames, }; use quote::quote; -use syn::{spanned::Spanned, Error, Result, Type}; +use syn::{Result, Type}; pub fn generate( idents: &QPropertyNames, qobject_idents: &QObjectNames, cxx_ty: &Type, type_names: &TypeNames, -) -> Result { +) -> Result> { let cpp_class_name_rust = &qobject_idents.name.rust_unqualified(); - let setter_wrapper_cpp = match &idents.setter_wrapper { - Some(wrapper) => wrapper.cxx_unqualified(), - _ => { - return Err(Error::new( - cxx_ty.span(), - "Property did not include a setter wrapper", - )) - } - }; + if let (Some(setter), Some(setter_wrapper)) = (&idents.setter, &idents.setter_wrapper) { + let setter_wrapper_cpp = setter_wrapper.cxx_unqualified(); - let setter_binding = match &idents.setter { - Some(setter) => setter, - _ => { - return Err(Error::new( - cxx_ty.span(), - "Property did not include a setter", - )) - } - }; + let setter_rust = setter.rust_unqualified(); + let ident = &idents.name.rust_unqualified(); + let ident_str = ident.to_string(); - let setter_rust = setter_binding.rust_unqualified(); - let ident = &idents.name.rust_unqualified(); - let ident_str = ident.to_string(); - - let notify_binding = match &idents.notify { - Some(notify) => notify, - _ => { - return Err(Error::new( - cxx_ty.span(), - "Property did not include a notify field", - )) - } - }; + // Generate a notify name if it was provided, otherwise return empty + let notify_binding = match &idents.notify { + Some(notify) => { + let notify_ident = notify.rust_unqualified(); + quote! {self.as_mut().#notify_ident();} + } + None => quote! {}, + }; - let notify_ident = notify_binding.rust_unqualified(); - let qualified_ty = syn_type_cxx_bridge_to_qualified(cxx_ty, type_names)?; - let qualified_impl = type_names.rust_qualified(cpp_class_name_rust)?; + let qualified_ty = syn_type_cxx_bridge_to_qualified(cxx_ty, type_names)?; + let qualified_impl = type_names.rust_qualified(cpp_class_name_rust)?; - // Determine if unsafe is required due to an unsafe type - let has_unsafe = if syn_type_is_cxx_bridge_unsafe(cxx_ty) { - quote! { unsafe } - } else { - quote! {} - }; + // Determine if unsafe is required due to an unsafe type + let has_unsafe = if syn_type_is_cxx_bridge_unsafe(cxx_ty) { + quote! { unsafe } + } else { + quote! {} + }; - Ok(RustFragmentPair { - cxx_bridge: vec![quote! { - extern "Rust" { - #[cxx_name = #setter_wrapper_cpp] - // TODO: Add #[namespace] of the QObject to the declaration - #has_unsafe fn #setter_rust(self: Pin<&mut #cpp_class_name_rust>, value: #cxx_ty); - } - }], - implementation: vec![quote! { - impl #qualified_impl { - #[doc = "Setter for the Q_PROPERTY "] - #[doc = #ident_str] - pub fn #setter_rust(mut self: core::pin::Pin<&mut Self>, value: #qualified_ty) { - use cxx_qt::CxxQtType; - if self.#ident == value { - // don't want to set the value again and reemit the signal, - // as this can cause binding loops - return; + Ok(Some(RustFragmentPair { + cxx_bridge: vec![quote! { + extern "Rust" { + #[cxx_name = #setter_wrapper_cpp] + // TODO: Add #[namespace] of the QObject to the declaration + #has_unsafe fn #setter_rust(self: Pin<&mut #cpp_class_name_rust>, value: #cxx_ty); + } + }], + implementation: vec![quote! { + impl #qualified_impl { + #[doc = "Setter for the Q_PROPERTY "] + #[doc = #ident_str] + pub fn #setter_rust(mut self: core::pin::Pin<&mut Self>, value: #qualified_ty) { + use cxx_qt::CxxQtType; + //TODO: could be optimised to remove this condition? + if self.#ident == value { + // don't want to set the value again and reemit the signal, + // as this can cause binding loops + return; + } + self.as_mut().rust_mut().#ident = value; + // self.as_mut().#notify_ident(); + #notify_binding } - self.as_mut().rust_mut().#ident = value; - self.as_mut().#notify_ident(); } - } - }], - }) + }], + })) + } else { + Ok(None) + } } diff --git a/crates/cxx-qt-gen/src/generator/rust/property/signal.rs b/crates/cxx-qt-gen/src/generator/rust/property/signal.rs index 1888f004f..f9b63966a 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/signal.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/signal.rs @@ -10,21 +10,26 @@ use crate::{ parser::signals::ParsedSignal, }; -pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> ParsedSignal { +pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Option { // We build our signal in the generation phase as we need to use the naming // structs to build the signal name let cpp_class_rust = &qobject_idents.name.rust_unqualified(); - let notify = &idents.notify.clone().expect("Notify was empty!"); //TODO: Throw error instead - let notify_rust = notify.rust_unqualified(); - let notify_cpp_str = notify.cxx_unqualified(); - let method: ForeignItemFn = syn::parse_quote! { - #[doc = "Notify for the Q_PROPERTY"] - #[cxx_name = #notify_cpp_str] - fn #notify_rust(self: Pin<&mut #cpp_class_rust>); - }; - ParsedSignal::from_property_method( - method, - notify.clone(), - qobject_idents.name.rust_unqualified().clone(), - ) + if let Some(notify) = &idents.notify { + let notify_rust = notify.rust_unqualified(); + let notify_cpp_str = notify.cxx_unqualified(); + + let method: ForeignItemFn = syn::parse_quote! { + #[doc = "Notify for the Q_PROPERTY"] + #[cxx_name = #notify_cpp_str] + fn #notify_rust(self: Pin<&mut #cpp_class_rust>); + }; + + Some(ParsedSignal::from_property_method( + method, + notify.clone(), + qobject_idents.name.rust_unqualified().clone(), + )) + } else { + None + } } diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index 536303562..d53fa667e 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -3,8 +3,6 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use std::collections::HashSet; - use syn::{ parse::{Error, ParseStream}, punctuated::Punctuated, @@ -12,8 +10,6 @@ use syn::{ Attribute, Expr, Ident, Meta, MetaNameValue, Path, Result, Token, Type, }; -use quote::format_ident; - /// An optional identifier of the functions passed with flags /// e.g. read = my_getter, IdentFlag would be used to store my_getter type IdentFlag = Option; @@ -21,57 +17,58 @@ type IdentFlag = Option; /// Struct for storing the flags provided for a QProperty and their optional identifiers ([IdentFlag]) #[derive(Debug)] pub struct QPropertyFlags { - read: IdentFlag, // TODO: maybe change this to better represent the data - write: Option, - notify: Option, + // TODO: Refactor to use an enum type for representing all 3 states of write and notify + // Or use an enum for {Auto, Custom(Ident)}, this is optional for write and notify, but not read + pub(crate) read: IdentFlag, + pub(crate) write: Option, + pub(crate) notify: Option, } -impl QPropertyFlags { - pub fn new() -> Self { +impl Default for QPropertyFlags { + // Default represents the flags of the desugared version of #[qproperty(T, ident)] + fn default() -> Self { Self { read: None, - write: None, - notify: None, + write: Some(None), + notify: Some(None), } } +} - pub fn all_flags() -> Self { +impl QPropertyFlags { + // Doesn't really represent a realistic state, just used for collecting in the parser + pub fn new() -> Self { Self { read: None, - write: Some(None), - notify: Some(None), + write: None, + notify: None, } } - fn set_field_by_string(&mut self, key: String, value: IdentFlag) -> Result { - match key.as_str() { + /// Takes an [Ident] and matches it to the valid flags, setting fields if a match is found, otherwise will error + fn set_field_by_ident(&mut self, key: Ident, value: IdentFlag) -> Result { + match key.to_string().as_str() { "read" => self.read = value, "write" => self.write = Some(value), "notify" => self.notify = Some(value), _ => { return Err(Error::new( - format_ident!("{}", key).span(), // TODO: check if this is an acceptable form of erroring for non Syn functions + key.span(), // TODO: check if this is an acceptable form of erroring for non Syn functions "Invalid flag passed, must be one of read, write, notify", )); } }; - Ok(key) // Return the string back up for checking which flags were set + Ok(key.to_string()) // Return the string back up for checking which flags were set } fn add_from_meta(&mut self, meta_value: Meta) -> Result { match meta_value { - Meta::Path(path) => { - let ident: String = parse_path_to_ident(&path).to_string(); - - Ok(self.set_field_by_string(ident, None)?) - } + Meta::Path(path) => Ok(self.set_field_by_ident(parse_path_to_ident(&path), None)?), Meta::NameValue(name_value) => { let kv_pair = parse_meta_name_value(&name_value)?; - - let ident: String = kv_pair.0.to_string(); let value: IdentFlag = Some(kv_pair.1); - Ok(self.set_field_by_string(ident, value)?) + Ok(self.set_field_by_ident(kv_pair.0, value)?) } _ => Err(Error::new( meta_value.span(), @@ -92,10 +89,11 @@ pub struct ParsedQProperty { } fn parse_path_to_ident(path: &Path) -> Ident { + // when used, should only ever contain 1 path segment path.segments[0].ident.clone() } -// Returning struct instead of tuple might be more descriptive +// TODO: Returning struct instead of tuple might be more descriptive fn parse_meta_name_value(name_value: &MetaNameValue) -> Result<(Ident, Ident)> { let ident = parse_path_to_ident(&name_value.path); let expr = &name_value.value; @@ -121,11 +119,11 @@ impl ParsedQProperty { let ident = input.parse()?; if input.is_empty() { - // No flags so fill with default options + // No flags passed so desugar: #[qproperty(T, ident)] -> #[qproperty(T, ident, read, write, notify)] Ok(Self { ident, ty, - flags: QPropertyFlags::all_flags(), + flags: QPropertyFlags::default(), }) } else { let _comma = input.parse::()?; // Start of final identifiers @@ -133,10 +131,13 @@ impl ParsedQProperty { let punctuated_flags: Punctuated = Punctuated::parse_terminated(input)?; - let flags: Vec = punctuated_flags.into_iter().collect(); // Removes the commas while collecting into Vec + // Remove the commas and collect the individual meta items + let flags: Vec = punctuated_flags.clone().into_iter().collect(); let mut passed_flags = QPropertyFlags::new(); + // Used to check that if flags are specified, read is included + // Can be removed in lieu of checking fields if the enum approach is used let mut flag_strings: Vec = Vec::new(); for flag in flags { @@ -146,7 +147,7 @@ impl ParsedQProperty { if !flag_strings.contains(&String::from("read")) { return Err(Error::new( - input.span(), + punctuated_flags.span(), "If flags are passed, read must be explicitly specified", )); } @@ -217,7 +218,7 @@ mod tests { assert_eq!(property.flags.read.unwrap(), format_ident!("blah")); assert!(property.flags.write.is_some()); - + assert!(property.flags.notify.is_some()); let notify = property.flags.notify.unwrap(); assert!(notify.is_some()); diff --git a/crates/cxx-qt-gen/test_inputs/properties.rs b/crates/cxx-qt-gen/test_inputs/properties.rs index d7ec0204a..36cbf3b02 100644 --- a/crates/cxx-qt-gen/test_inputs/properties.rs +++ b/crates/cxx-qt-gen/test_inputs/properties.rs @@ -11,6 +11,7 @@ mod ffi { #[derive(Default)] #[qproperty(i32, primitive)] #[qproperty(QPoint, trivial)] + #[qproperty(i32, test_name, read = my_getter, write = my_setter, notify)] type MyObject = super::MyObjectRust; } } diff --git a/crates/cxx-qt-gen/test_outputs/properties.cpp b/crates/cxx-qt-gen/test_outputs/properties.cpp index 578a0a3c9..6e9e6744f 100644 --- a/crates/cxx-qt-gen/test_outputs/properties.cpp +++ b/crates/cxx-qt-gen/test_outputs/properties.cpp @@ -116,6 +116,64 @@ MyObject_trivialChangedConnect( } } // namespace cxx_qt::my_object::rust::cxxqtgen1 +// Define namespace otherwise we hit a GCC bug +// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480 +namespace rust::cxxqt1 { +template<> +SignalHandler< + ::cxx_qt::my_object::rust::cxxqtgen1:: + MyObjectCxxQtSignalParamstestNameChanged*>::~SignalHandler() noexcept +{ + if (data[0] == nullptr && data[1] == nullptr) { + return; + } + + drop_MyObject_signal_handler_testNameChanged(::std::move(*this)); +} + +template<> +template<> +void +SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1:: + MyObjectCxxQtSignalParamstestNameChanged*>:: +operator()(cxx_qt::my_object::MyObject& self) +{ + call_MyObject_signal_handler_testNameChanged(*this, self); +} + +static_assert( + alignof(SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1:: + MyObjectCxxQtSignalParamstestNameChanged*>) <= + alignof(::std::size_t), + "unexpected aligment"); +static_assert( + sizeof(SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1:: + MyObjectCxxQtSignalParamstestNameChanged*>) == + sizeof(::std::size_t[2]), + "unexpected size"); +} // namespace rust::cxxqt1 + +namespace cxx_qt::my_object::rust::cxxqtgen1 { +::QMetaObject::Connection +MyObject_testNameChangedConnect( + cxx_qt::my_object::MyObject& self, + ::cxx_qt::my_object::rust::cxxqtgen1:: + MyObjectCxxQtSignalHandlertestNameChanged closure, + ::Qt::ConnectionType type) +{ + return ::QObject::connect( + &self, + &cxx_qt::my_object::MyObject::testNameChanged, + &self, + [&, closure = ::std::move(closure)]() mutable { + const ::rust::cxxqt1::MaybeLockGuard guard( + self); + closure.template operator()(self); + }, + type); +} +} // namespace cxx_qt::my_object::rust::cxxqtgen1 + namespace cxx_qt::my_object { ::std::int32_t const& MyObject::getPrimitive() const diff --git a/crates/cxx-qt-gen/test_outputs/properties.h b/crates/cxx-qt-gen/test_outputs/properties.h index 7efa9d489..b67d65968 100644 --- a/crates/cxx-qt-gen/test_outputs/properties.h +++ b/crates/cxx-qt-gen/test_outputs/properties.h @@ -21,6 +21,11 @@ using MyObjectCxxQtSignalHandlertrivialChanged = ::rust::cxxqt1::SignalHandler< struct MyObjectCxxQtSignalParamstrivialChanged*>; } // namespace cxx_qt::my_object::rust::cxxqtgen1 +namespace cxx_qt::my_object::rust::cxxqtgen1 { +using MyObjectCxxQtSignalHandlertestNameChanged = ::rust::cxxqt1::SignalHandler< + struct MyObjectCxxQtSignalParamstestNameChanged*>; +} // namespace cxx_qt::my_object::rust::cxxqtgen1 + #include "cxx-qt-gen/ffi.cxx.h" namespace cxx_qt::my_object::rust::cxxqtgen1 { @@ -41,6 +46,15 @@ MyObject_trivialChangedConnect( ::Qt::ConnectionType type); } // namespace cxx_qt::my_object::rust::cxxqtgen1 +namespace cxx_qt::my_object::rust::cxxqtgen1 { +::QMetaObject::Connection +MyObject_testNameChangedConnect( + cxx_qt::my_object::MyObject& self, + ::cxx_qt::my_object::rust::cxxqtgen1:: + MyObjectCxxQtSignalHandlertestNameChanged closure, + ::Qt::ConnectionType type); +} // namespace cxx_qt::my_object::rust::cxxqtgen1 + namespace cxx_qt::my_object { class MyObject : public QObject @@ -53,6 +67,8 @@ class MyObject NOTIFY primitiveChanged) Q_PROPERTY( QPoint trivial READ getTrivial WRITE setTrivial NOTIFY trivialChanged) + Q_PROPERTY(::std::int32_t testName READ my_getter WRITE my_setter NOTIFY + testNameChanged) virtual ~MyObject() = default; @@ -63,6 +79,7 @@ class MyObject Q_SLOT void setTrivial(QPoint const& value); Q_SIGNAL void primitiveChanged(); Q_SIGNAL void trivialChanged(); + Q_SIGNAL void testNameChanged(); explicit MyObject(QObject* parent = nullptr); private: diff --git a/crates/cxx-qt-gen/test_outputs/properties.rs b/crates/cxx-qt-gen/test_outputs/properties.rs index 68c14a2f6..b3be4d5aa 100644 --- a/crates/cxx-qt-gen/test_outputs/properties.rs +++ b/crates/cxx-qt-gen/test_outputs/properties.rs @@ -113,6 +113,38 @@ mod ffi { self_value: Pin<&mut MyObject>, ); } + unsafe extern "C++" { + #[doc = "Notify for the Q_PROPERTY"] + #[cxx_name = "testNameChanged"] + fn test_name_changed(self: Pin<&mut MyObject>); + } + unsafe extern "C++" { + #[doc(hidden)] + #[namespace = "cxx_qt::my_object::rust::cxxqtgen1"] + type MyObjectCxxQtSignalHandlertestNameChanged = cxx_qt::signalhandler::CxxQtSignalHandler< + super::MyObjectCxxQtSignalClosuretestNameChanged, + >; + #[doc(hidden)] + #[namespace = "cxx_qt::my_object::rust::cxxqtgen1"] + #[cxx_name = "MyObject_testNameChangedConnect"] + fn MyObject_connect_test_name_changed( + self_value: Pin<&mut MyObject>, + signal_handler: MyObjectCxxQtSignalHandlertestNameChanged, + conn_type: CxxQtConnectionType, + ) -> CxxQtQMetaObjectConnection; + } + #[namespace = "cxx_qt::my_object::rust::cxxqtgen1"] + extern "Rust" { + #[doc(hidden)] + fn drop_MyObject_signal_handler_testNameChanged( + handler: MyObjectCxxQtSignalHandlertestNameChanged, + ); + #[doc(hidden)] + fn call_MyObject_signal_handler_testNameChanged( + handler: &mut MyObjectCxxQtSignalHandlertestNameChanged, + self_value: Pin<&mut MyObject>, + ); + } extern "Rust" { #[cxx_name = "createRs"] #[namespace = "cxx_qt::my_object::cxx_qt_my_object"] @@ -285,6 +317,74 @@ cxx_qt::static_assertions::assert_eq_size!( cxx_qt::signalhandler::CxxQtSignalHandler, [usize; 2] ); +impl ffi::MyObject { + #[doc = "Connect the given function pointer to the signal "] + #[doc = "testNameChanged"] + #[doc = ", so that when the signal is emitted the function pointer is executed."] + pub fn connect_test_name_changed) + 'static>( + self: core::pin::Pin<&mut ffi::MyObject>, + mut closure: F, + conn_type: cxx_qt::ConnectionType, + ) -> cxx_qt::QMetaObjectConnectionGuard { + cxx_qt::QMetaObjectConnectionGuard::from( + ffi::MyObject_connect_test_name_changed( + self, + cxx_qt::signalhandler::CxxQtSignalHandler::< + MyObjectCxxQtSignalClosuretestNameChanged, + >::new(Box::new(closure)), + conn_type, + ), + ) + } +} +impl ffi::MyObject { + #[doc = "Connect the given function pointer to the signal "] + #[doc = "testNameChanged"] + #[doc = ", so that when the signal is emitted the function pointer is executed."] + #[doc = "\n"] + #[doc = "Note that this method uses a AutoConnection connection type."] + pub fn on_test_name_changed) + 'static>( + self: core::pin::Pin<&mut ffi::MyObject>, + mut closure: F, + ) -> cxx_qt::QMetaObjectConnectionGuard { + cxx_qt::QMetaObjectConnectionGuard::from( + ffi::MyObject_connect_test_name_changed( + self, + cxx_qt::signalhandler::CxxQtSignalHandler::< + MyObjectCxxQtSignalClosuretestNameChanged, + >::new(Box::new(closure)), + cxx_qt::ConnectionType::AutoConnection, + ), + ) + } +} +#[doc(hidden)] +pub struct MyObjectCxxQtSignalClosuretestNameChanged {} +impl cxx_qt::signalhandler::CxxQtSignalHandlerClosure + for MyObjectCxxQtSignalClosuretestNameChanged +{ + type Id = cxx::type_id!( + "::cxx_qt::my_object::rust::cxxqtgen1::MyObjectCxxQtSignalHandlertestNameChanged" + ); + type FnType = dyn FnMut(core::pin::Pin<&mut ffi::MyObject>); +} +use core::mem::drop as drop_MyObject_signal_handler_testNameChanged; +fn call_MyObject_signal_handler_testNameChanged( + handler: &mut cxx_qt::signalhandler::CxxQtSignalHandler< + MyObjectCxxQtSignalClosuretestNameChanged, + >, + self_value: core::pin::Pin<&mut ffi::MyObject>, +) { + handler.closure()(self_value); +} +cxx_qt::static_assertions::assert_eq_align!( + cxx_qt::signalhandler::CxxQtSignalHandler, + usize +); +cxx_qt::static_assertions::assert_eq_size!( + cxx_qt::signalhandler::CxxQtSignalHandler, + [usize; 2] +); impl cxx_qt::Locking for ffi::MyObject {} #[doc(hidden)] pub fn create_rs_my_object_rust() -> std::boxed::Box { From eeb2ba6b4c81a42f5f75069be4d3f71240bf8217 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Wed, 17 Jul 2024 11:22:07 +0100 Subject: [PATCH 11/18] Fix Issues raised in code review, including refactor to use Enum in QPropertyFlags --- .../src/generator/cpp/property/mod.rs | 53 +++-- .../src/generator/naming/property.rs | 32 +-- .../src/generator/rust/property/mod.rs | 14 +- .../src/generator/rust/property/setter.rs | 3 +- crates/cxx-qt-gen/src/parser/property.rs | 182 ++++++++++-------- crates/cxx-qt-gen/test_inputs/properties.rs | 4 +- crates/cxx-qt-gen/test_outputs/properties.cpp | 49 +++-- crates/cxx-qt-gen/test_outputs/properties.h | 24 ++- crates/cxx-qt-gen/test_outputs/properties.rs | 99 +++++++--- 9 files changed, 287 insertions(+), 173 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs index a9d971569..2e595299f 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs @@ -36,7 +36,7 @@ pub fn generate_cpp_properties( generated.metaobjects.push(meta::generate(&idents, &cxx_ty)); // None indicates no custom identifier was provided - if flags.read.is_none() { + if flags.read.is_auto() { generated .methods .push(getter::generate(&idents, &qobject_ident, &cxx_ty)); @@ -46,11 +46,7 @@ pub fn generate_cpp_properties( } // Checking custom setter wasn't provided - if flags - .write - .clone() - .is_some_and(|ident_option| ident_option.is_none()) - { + if flags.write.clone().is_some_and(|state| state.is_auto()) { if let Some(setter) = setter::generate(&idents, &qobject_ident, &cxx_ty) { generated.methods.push(setter) } @@ -61,11 +57,7 @@ pub fn generate_cpp_properties( } // Checking custom notifier wasn't provided - if flags - .notify - .clone() - .is_some_and(|ident_option| ident_option.is_none()) - { + if flags.notify.clone().is_some_and(|state| state.is_auto()) { if let Some(notify) = signal::generate(&idents, qobject_idents) { signals.push(notify) } @@ -95,9 +87,9 @@ mod tests { use syn::{parse_quote, ItemStruct}; #[test] - fn test_optional_write() { + fn test_custom_setter() { let mut input: ItemStruct = parse_quote! { - #[qproperty(i32, num, read, write, notify)] + #[qproperty(i32, num, read, write = mySetter)] struct MyStruct; }; let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); @@ -107,8 +99,39 @@ mod tests { let qobject_idents = create_qobjectname(); let type_names = TypeNames::mock(); - let _generated = - generate_cpp_properties(&properties, &qobject_idents, &type_names).unwrap(); + let generated = generate_cpp_properties(&properties, &qobject_idents, &type_names).unwrap(); + // TODO: write better tests using the string comparisons + + // Meta + println!("Generated meta: {:?}\n\n", generated.metaobjects); + println!("Generated methods: {:?}\n\n", generated.methods); + + assert_eq!(generated.metaobjects.len(), 1); + assert_str_eq!( + generated.metaobjects[0], + "Q_PROPERTY(::std::int32_t num READ getNum WRITE mySetter)" + ); + + // Methods + assert_eq!(generated.methods.len(), 1); + let (header, source) = if let CppFragment::Pair { header, source } = &generated.methods[0] { + (header, source) + } else { + panic!("Expected pair!") + }; + + assert_str_eq!(header, "::std::int32_t const& getNum() const;"); + assert_str_eq!( + source, + indoc! {r#" + ::std::int32_t const& + MyObject::getNum() const + { + const ::rust::cxxqt1::MaybeLockGuard guard(*this); + return getNumWrapper(); + } + "#} + ); } #[test] diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs index 73d7e8ef1..8a9cfba27 100644 --- a/crates/cxx-qt-gen/src/generator/naming/property.rs +++ b/crates/cxx-qt-gen/src/generator/naming/property.rs @@ -2,7 +2,10 @@ // SPDX-FileContributor: Andrew Hayzen // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::{naming::Name, parser::property::ParsedQProperty}; +use crate::{ + naming::Name, + parser::property::{FlagState, ParsedQProperty}, +}; use convert_case::{Case, Casing}; use quote::format_ident; use syn::Ident; @@ -23,26 +26,23 @@ impl From<&ParsedQProperty> for QPropertyNames { let flags = &property.flags; - // Match for custom name or autogenerate one let getter = match &flags.read { - Some(ident) => Name::new(ident.clone()), - None => getter_name_from_property(&property_name), + FlagState::Auto => getter_name_from_property(&property_name), + FlagState::Custom(ident) => Name::new(ident.clone()), }; - // Match for custom name, autogenerate, or None - // TODO: Refactor to use an enum type to represent these 3 states better let setter = match &flags.write { - Some(ident_option) => match ident_option { - Some(ident) => Some(Name::new(ident.clone())), - None => Some(setter_name_from_property(&property_name)), + Some(state) => match state { + FlagState::Auto => Some(setter_name_from_property(&property_name)), + FlagState::Custom(ident) => Some(Name::new(ident.clone())), }, None => None, }; let notify = match &flags.notify { - Some(ident_option) => match ident_option { - Some(ident) => Some(Name::new(ident.clone())), - None => Some(notify_name_from_property(&property_name)), + Some(state) => match state { + FlagState::Auto => Some(notify_name_from_property(&property_name)), + FlagState::Custom(ident) => Some(Name::new(ident.clone())), }, None => None, }; @@ -127,19 +127,19 @@ pub mod tests { &format_ident!("my_property") ); assert_eq!( - names.setter.clone().unwrap().cxx_unqualified(), + names.setter.as_ref().unwrap().cxx_unqualified(), "setMyProperty" ); assert_eq!( - names.setter.clone().unwrap().rust_unqualified(), + names.setter.as_ref().unwrap().rust_unqualified(), &format_ident!("set_my_property") ); assert_eq!( - names.notify.clone().unwrap().cxx_unqualified(), + names.notify.as_ref().unwrap().cxx_unqualified(), "myPropertyChanged" ); assert_eq!( - names.notify.clone().unwrap().rust_unqualified(), + names.notify.as_ref().unwrap().rust_unqualified(), &format_ident!("my_property_changed") ); } diff --git a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs index ed07cd7c3..cc7a79268 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs @@ -33,7 +33,7 @@ pub fn generate_rust_properties( let flags = &property.flags; - if flags.read.is_none() { + if flags.read.is_auto() { //gen getter and wrapper let getter = getter::generate(&idents, qobject_idents, &property.ty, type_names)?; generated @@ -45,11 +45,7 @@ pub fn generate_rust_properties( } // Checking that write flag was provided but no custom identifier - if flags - .write - .clone() - .is_some_and(|ident_option| ident_option.is_none()) - { + if flags.write.clone().is_some_and(|state| state.is_auto()) { // gen setter and wrapper if let Some(setter) = setter::generate(&idents, qobject_idents, &property.ty, type_names)? @@ -63,11 +59,7 @@ pub fn generate_rust_properties( } } - if flags - .notify - .clone() - .is_some_and(|ident_option| ident_option.is_none()) - { + if flags.notify.clone().is_some_and(|state| state.is_auto()) { if let Some(notify) = signal::generate(&idents, qobject_idents) { signals.push(notify) } diff --git a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs index d6649229c..a4745f43b 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs @@ -62,14 +62,13 @@ pub fn generate( #[doc = #ident_str] pub fn #setter_rust(mut self: core::pin::Pin<&mut Self>, value: #qualified_ty) { use cxx_qt::CxxQtType; - //TODO: could be optimised to remove this condition? + if self.#ident == value { // don't want to set the value again and reemit the signal, // as this can cause binding loops return; } self.as_mut().rust_mut().#ident = value; - // self.as_mut().#notify_ident(); #notify_binding } } diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index d53fa667e..a9c127ac3 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -7,30 +7,37 @@ use syn::{ parse::{Error, ParseStream}, punctuated::Punctuated, spanned::Spanned, - Attribute, Expr, Ident, Meta, MetaNameValue, Path, Result, Token, Type, + Attribute, Expr, Ident, Meta, MetaNameValue, Result, Token, Type, }; -/// An optional identifier of the functions passed with flags -/// e.g. read = my_getter, IdentFlag would be used to store my_getter -type IdentFlag = Option; -/// Struct for storing the flags provided for a QProperty and their optional identifiers ([IdentFlag]) +#[derive(Debug, Clone)] +pub enum FlagState { + Auto, // Might need to refactor to also store the generated ident here + Custom(Ident), +} + +impl FlagState { + pub fn is_auto(&self) -> bool { + matches!(self, Self::Auto) + } +} + +/// Struct for storing the flags provided for a QProperty #[derive(Debug)] pub struct QPropertyFlags { - // TODO: Refactor to use an enum type for representing all 3 states of write and notify - // Or use an enum for {Auto, Custom(Ident)}, this is optional for write and notify, but not read - pub(crate) read: IdentFlag, - pub(crate) write: Option, - pub(crate) notify: Option, + pub(crate) read: FlagState, + pub(crate) write: Option, + pub(crate) notify: Option, } impl Default for QPropertyFlags { - // Default represents the flags of the desugared version of #[qproperty(T, ident)] + /// Default represents the flags of the desugared version of ```#[qproperty(T, ident)]``` fn default() -> Self { Self { - read: None, - write: Some(None), - notify: Some(None), + read: FlagState::Auto, + write: Some(FlagState::Auto), + notify: Some(FlagState::Auto), } } } @@ -39,43 +46,11 @@ impl QPropertyFlags { // Doesn't really represent a realistic state, just used for collecting in the parser pub fn new() -> Self { Self { - read: None, + read: FlagState::Auto, write: None, notify: None, } } - - /// Takes an [Ident] and matches it to the valid flags, setting fields if a match is found, otherwise will error - fn set_field_by_ident(&mut self, key: Ident, value: IdentFlag) -> Result { - match key.to_string().as_str() { - "read" => self.read = value, - "write" => self.write = Some(value), - "notify" => self.notify = Some(value), - _ => { - return Err(Error::new( - key.span(), // TODO: check if this is an acceptable form of erroring for non Syn functions - "Invalid flag passed, must be one of read, write, notify", - )); - } - }; - Ok(key.to_string()) // Return the string back up for checking which flags were set - } - - fn add_from_meta(&mut self, meta_value: Meta) -> Result { - match meta_value { - Meta::Path(path) => Ok(self.set_field_by_ident(parse_path_to_ident(&path), None)?), - Meta::NameValue(name_value) => { - let kv_pair = parse_meta_name_value(&name_value)?; - let value: IdentFlag = Some(kv_pair.1); - - Ok(self.set_field_by_ident(kv_pair.0, value)?) - } - _ => Err(Error::new( - meta_value.span(), - "Invalid syntax, flags must be specified as either `read` or `read = my_getter`", - )), - } - } } /// Describes a single Q_PROPERTY for a struct @@ -88,19 +63,14 @@ pub struct ParsedQProperty { pub flags: QPropertyFlags, } -fn parse_path_to_ident(path: &Path) -> Ident { - // when used, should only ever contain 1 path segment - path.segments[0].ident.clone() -} - // TODO: Returning struct instead of tuple might be more descriptive fn parse_meta_name_value(name_value: &MetaNameValue) -> Result<(Ident, Ident)> { - let ident = parse_path_to_ident(&name_value.path); + let ident = name_value.path.require_ident()?.clone(); let expr = &name_value.value; let func_signature: Ident; if let Expr::Path(path_expr) = expr { - func_signature = parse_path_to_ident(&path_expr.path); + func_signature = path_expr.path.require_ident()?.clone(); } else { return Err(Error::new( expr.span(), @@ -134,29 +104,67 @@ impl ParsedQProperty { // Remove the commas and collect the individual meta items let flags: Vec = punctuated_flags.clone().into_iter().collect(); - let mut passed_flags = QPropertyFlags::new(); - - // Used to check that if flags are specified, read is included - // Can be removed in lieu of checking fields if the enum approach is used - let mut flag_strings: Vec = Vec::new(); + let mut read = None; + let mut write = None; + let mut notify = None; + + // Create mutable closure to capture the variables for setting with the Meta values + let mut update_fields = |ident: &Ident, value: Option| -> Result<()> { + let variable = match ident.to_string().as_str() { + "read" => &mut read, + "write" => &mut write, + "notify" => &mut notify, + _ => { + return Err(Error::new( + ident.span(), + "Invalid flag passed, must be one of read, write, notify", + )); + } + }; + *variable = Some(value.map_or(FlagState::Auto, FlagState::Custom)); + + Ok(()) + }; for flag in flags { - let flag_string: String = passed_flags.add_from_meta(flag)?; - flag_strings.push(flag_string); + // Could maybe refactor parse_meta_name_value to parse_meta, + // which would return an (Ident, Option) and extract the logic below to a new fn + match flag { + Meta::Path(path) => { + // Set flag as auto + update_fields(path.require_ident()?, None)?; + }, + Meta::NameValue(name_value) => { + let kv_pair = parse_meta_name_value(&name_value)?; + let value = Some(kv_pair.1); + + // Set Flags with Custom value + update_fields(&kv_pair.0, value)?; + }, + _ => return Err(Error::new( + flag.span(), + "Invalid syntax, flags must be specified as either `read` or `read = my_getter`", + )), + }; } - if !flag_strings.contains(&String::from("read")) { - return Err(Error::new( + if let Some(read) = read { + Ok(Self { + ident, + ty, + flags: QPropertyFlags { + read, + write, + notify, + }, + }) + } + else { + Err(Error::new( punctuated_flags.span(), "If flags are passed, read must be explicitly specified", - )); + )) } - - Ok(Self { - ident, - ty, - flags: passed_flags, - }) } }) } @@ -200,29 +208,43 @@ mod tests { let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); assert_eq!(property.ident, format_ident!("name")); assert_eq!(property.ty, parse_quote! { T }); + + assert!(property.flags.read.is_auto()); + assert!(property.flags.write.is_some()); assert!(property.flags.notify.is_some()); + + assert!(property.flags.write.unwrap().is_auto()); + assert!(property.flags.notify.unwrap().is_auto()); } #[test] fn test_parse_flags_kw() { let mut input: ItemStruct = parse_quote! { - #[qproperty(T, name, read = blah, write, notify = blahblah)] + #[qproperty(T, name, read = my_getter, write, notify = my_notifier)] struct MyStruct; }; let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); assert_eq!(property.ident, format_ident!("name")); assert_eq!(property.ty, parse_quote! { T }); - assert!(property.flags.read.is_some()); - assert_eq!(property.flags.read.unwrap(), format_ident!("blah")); + // Can use assert_matches! when https://github.com/rust-lang/rust/issues/82775 gets stabilised + let expected_read = format_ident!("my_getter"); + assert!(matches!( + property.flags.read, + FlagState::Custom(ident) if ident == expected_read + )); assert!(property.flags.write.is_some()); + assert!(property.flags.write.unwrap().is_auto()); assert!(property.flags.notify.is_some()); - let notify = property.flags.notify.unwrap(); - assert!(notify.is_some()); - assert_eq!(notify.unwrap(), format_ident!("blahblah")); + + let expected_notify = format_ident!("my_notifier"); + assert!(matches!( + property.flags.notify, + Some(FlagState::Custom(ident)) if ident == expected_notify + )); } #[test] @@ -236,7 +258,7 @@ mod tests { } #[test] - fn test_parse_missing_flags() { + fn test_parse_flags_missing_read() { let mut input: ItemStruct = parse_quote! { #[qproperty(T, name, notify = blahblah)] struct MyStruct; @@ -246,7 +268,7 @@ mod tests { } #[test] - fn test_parse_invalid_literal() { + fn test_parse_flags_invalid_literal() { let mut input: ItemStruct = parse_quote! { #[qproperty(T, name, notify = 3)] struct MyStruct; @@ -286,9 +308,9 @@ mod tests { } #[test] - fn test_parse_property_no_type() { + fn test_parse_property_no_params() { let mut input: ItemStruct = parse_quote! { - #[qproperty(num)] + #[qproperty()] struct MyStruct; }; let property = ParsedQProperty::parse(input.attrs.remove(0)); diff --git a/crates/cxx-qt-gen/test_inputs/properties.rs b/crates/cxx-qt-gen/test_inputs/properties.rs index 36cbf3b02..3d385f88d 100644 --- a/crates/cxx-qt-gen/test_inputs/properties.rs +++ b/crates/cxx-qt-gen/test_inputs/properties.rs @@ -11,7 +11,9 @@ mod ffi { #[derive(Default)] #[qproperty(i32, primitive)] #[qproperty(QPoint, trivial)] - #[qproperty(i32, test_name, read = my_getter, write = my_setter, notify)] + #[qproperty(i32, custom_function_prop, read = my_getter, write = my_setter, notify)] + #[qproperty(i32, readonly_prop, read)] + #[qproperty(i32, custom_on_changed, read, write, notify = myOnChanged)] type MyObject = super::MyObjectRust; } } diff --git a/crates/cxx-qt-gen/test_outputs/properties.cpp b/crates/cxx-qt-gen/test_outputs/properties.cpp index 6e9e6744f..b1fe2f9cc 100644 --- a/crates/cxx-qt-gen/test_outputs/properties.cpp +++ b/crates/cxx-qt-gen/test_outputs/properties.cpp @@ -120,50 +120,52 @@ MyObject_trivialChangedConnect( // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480 namespace rust::cxxqt1 { template<> -SignalHandler< - ::cxx_qt::my_object::rust::cxxqtgen1:: - MyObjectCxxQtSignalParamstestNameChanged*>::~SignalHandler() noexcept +SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1:: + MyObjectCxxQtSignalParamscustomFunctionPropChanged*>:: + ~SignalHandler() noexcept { if (data[0] == nullptr && data[1] == nullptr) { return; } - drop_MyObject_signal_handler_testNameChanged(::std::move(*this)); + drop_MyObject_signal_handler_customFunctionPropChanged(::std::move(*this)); } template<> template<> void SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1:: - MyObjectCxxQtSignalParamstestNameChanged*>:: + MyObjectCxxQtSignalParamscustomFunctionPropChanged*>:: operator()(cxx_qt::my_object::MyObject& self) { - call_MyObject_signal_handler_testNameChanged(*this, self); + call_MyObject_signal_handler_customFunctionPropChanged(*this, self); } static_assert( - alignof(SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1:: - MyObjectCxxQtSignalParamstestNameChanged*>) <= + alignof( + SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1:: + MyObjectCxxQtSignalParamscustomFunctionPropChanged*>) <= alignof(::std::size_t), "unexpected aligment"); static_assert( - sizeof(SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1:: - MyObjectCxxQtSignalParamstestNameChanged*>) == + sizeof( + SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1:: + MyObjectCxxQtSignalParamscustomFunctionPropChanged*>) == sizeof(::std::size_t[2]), "unexpected size"); } // namespace rust::cxxqt1 namespace cxx_qt::my_object::rust::cxxqtgen1 { ::QMetaObject::Connection -MyObject_testNameChangedConnect( +MyObject_customFunctionPropChangedConnect( cxx_qt::my_object::MyObject& self, ::cxx_qt::my_object::rust::cxxqtgen1:: - MyObjectCxxQtSignalHandlertestNameChanged closure, + MyObjectCxxQtSignalHandlercustomFunctionPropChanged closure, ::Qt::ConnectionType type) { return ::QObject::connect( &self, - &cxx_qt::my_object::MyObject::testNameChanged, + &cxx_qt::my_object::MyObject::customFunctionPropChanged, &self, [&, closure = ::std::move(closure)]() mutable { const ::rust::cxxqt1::MaybeLockGuard guard( @@ -203,6 +205,27 @@ MyObject::setTrivial(QPoint const& value) setTrivialWrapper(value); } +::std::int32_t const& +MyObject::getReadonlyProp() const +{ + const ::rust::cxxqt1::MaybeLockGuard guard(*this); + return getReadonlyPropWrapper(); +} + +::std::int32_t const& +MyObject::getCustomOnChanged() const +{ + const ::rust::cxxqt1::MaybeLockGuard guard(*this); + return getCustomOnChangedWrapper(); +} + +void +MyObject::setCustomOnChanged(::std::int32_t const& value) +{ + const ::rust::cxxqt1::MaybeLockGuard guard(*this); + setCustomOnChangedWrapper(value); +} + MyObject::MyObject(QObject* parent) : QObject(parent) , ::rust::cxxqt1::CxxQtType( diff --git a/crates/cxx-qt-gen/test_outputs/properties.h b/crates/cxx-qt-gen/test_outputs/properties.h index b67d65968..b5e28e79b 100644 --- a/crates/cxx-qt-gen/test_outputs/properties.h +++ b/crates/cxx-qt-gen/test_outputs/properties.h @@ -22,8 +22,9 @@ using MyObjectCxxQtSignalHandlertrivialChanged = ::rust::cxxqt1::SignalHandler< } // namespace cxx_qt::my_object::rust::cxxqtgen1 namespace cxx_qt::my_object::rust::cxxqtgen1 { -using MyObjectCxxQtSignalHandlertestNameChanged = ::rust::cxxqt1::SignalHandler< - struct MyObjectCxxQtSignalParamstestNameChanged*>; +using MyObjectCxxQtSignalHandlercustomFunctionPropChanged = + ::rust::cxxqt1::SignalHandler< + struct MyObjectCxxQtSignalParamscustomFunctionPropChanged*>; } // namespace cxx_qt::my_object::rust::cxxqtgen1 #include "cxx-qt-gen/ffi.cxx.h" @@ -48,10 +49,10 @@ MyObject_trivialChangedConnect( namespace cxx_qt::my_object::rust::cxxqtgen1 { ::QMetaObject::Connection -MyObject_testNameChangedConnect( +MyObject_customFunctionPropChangedConnect( cxx_qt::my_object::MyObject& self, ::cxx_qt::my_object::rust::cxxqtgen1:: - MyObjectCxxQtSignalHandlertestNameChanged closure, + MyObjectCxxQtSignalHandlercustomFunctionPropChanged closure, ::Qt::ConnectionType type); } // namespace cxx_qt::my_object::rust::cxxqtgen1 @@ -67,8 +68,11 @@ class MyObject NOTIFY primitiveChanged) Q_PROPERTY( QPoint trivial READ getTrivial WRITE setTrivial NOTIFY trivialChanged) - Q_PROPERTY(::std::int32_t testName READ my_getter WRITE my_setter NOTIFY - testNameChanged) + Q_PROPERTY(::std::int32_t customFunctionProp READ my_getter WRITE my_setter + NOTIFY customFunctionPropChanged) + Q_PROPERTY(::std::int32_t readonlyProp READ getReadonlyProp) + Q_PROPERTY(::std::int32_t customOnChanged READ getCustomOnChanged WRITE + setCustomOnChanged NOTIFY myOnChanged) virtual ~MyObject() = default; @@ -77,9 +81,12 @@ class MyObject Q_SLOT void setPrimitive(::std::int32_t const& value); QPoint const& getTrivial() const; Q_SLOT void setTrivial(QPoint const& value); + ::std::int32_t const& getReadonlyProp() const; + ::std::int32_t const& getCustomOnChanged() const; + Q_SLOT void setCustomOnChanged(::std::int32_t const& value); Q_SIGNAL void primitiveChanged(); Q_SIGNAL void trivialChanged(); - Q_SIGNAL void testNameChanged(); + Q_SIGNAL void customFunctionPropChanged(); explicit MyObject(QObject* parent = nullptr); private: @@ -87,6 +94,9 @@ class MyObject void setPrimitiveWrapper(::std::int32_t value) noexcept; QPoint const& getTrivialWrapper() const noexcept; void setTrivialWrapper(QPoint value) noexcept; + ::std::int32_t const& getReadonlyPropWrapper() const noexcept; + ::std::int32_t const& getCustomOnChangedWrapper() const noexcept; + void setCustomOnChangedWrapper(::std::int32_t value) noexcept; }; static_assert(::std::is_base_of::value, diff --git a/crates/cxx-qt-gen/test_outputs/properties.rs b/crates/cxx-qt-gen/test_outputs/properties.rs index b3be4d5aa..9dda703a3 100644 --- a/crates/cxx-qt-gen/test_outputs/properties.rs +++ b/crates/cxx-qt-gen/test_outputs/properties.rs @@ -49,6 +49,18 @@ mod ffi { #[cxx_name = "setTrivialWrapper"] fn set_trivial(self: Pin<&mut MyObject>, value: QPoint); } + extern "Rust" { + #[cxx_name = "getReadonlyPropWrapper"] + unsafe fn readonly_prop<'a>(self: &'a MyObject) -> &'a i32; + } + extern "Rust" { + #[cxx_name = "getCustomOnChangedWrapper"] + unsafe fn custom_on_changed<'a>(self: &'a MyObject) -> &'a i32; + } + extern "Rust" { + #[cxx_name = "setCustomOnChangedWrapper"] + fn set_custom_on_changed(self: Pin<&mut MyObject>, value: i32); + } unsafe extern "C++" { #[doc = "Notify for the Q_PROPERTY"] #[cxx_name = "primitiveChanged"] @@ -115,33 +127,34 @@ mod ffi { } unsafe extern "C++" { #[doc = "Notify for the Q_PROPERTY"] - #[cxx_name = "testNameChanged"] - fn test_name_changed(self: Pin<&mut MyObject>); + #[cxx_name = "customFunctionPropChanged"] + fn custom_function_prop_changed(self: Pin<&mut MyObject>); } unsafe extern "C++" { #[doc(hidden)] #[namespace = "cxx_qt::my_object::rust::cxxqtgen1"] - type MyObjectCxxQtSignalHandlertestNameChanged = cxx_qt::signalhandler::CxxQtSignalHandler< - super::MyObjectCxxQtSignalClosuretestNameChanged, - >; + type MyObjectCxxQtSignalHandlercustomFunctionPropChanged = + cxx_qt::signalhandler::CxxQtSignalHandler< + super::MyObjectCxxQtSignalClosurecustomFunctionPropChanged, + >; #[doc(hidden)] #[namespace = "cxx_qt::my_object::rust::cxxqtgen1"] - #[cxx_name = "MyObject_testNameChangedConnect"] - fn MyObject_connect_test_name_changed( + #[cxx_name = "MyObject_customFunctionPropChangedConnect"] + fn MyObject_connect_custom_function_prop_changed( self_value: Pin<&mut MyObject>, - signal_handler: MyObjectCxxQtSignalHandlertestNameChanged, + signal_handler: MyObjectCxxQtSignalHandlercustomFunctionPropChanged, conn_type: CxxQtConnectionType, ) -> CxxQtQMetaObjectConnection; } #[namespace = "cxx_qt::my_object::rust::cxxqtgen1"] extern "Rust" { #[doc(hidden)] - fn drop_MyObject_signal_handler_testNameChanged( - handler: MyObjectCxxQtSignalHandlertestNameChanged, + fn drop_MyObject_signal_handler_customFunctionPropChanged( + handler: MyObjectCxxQtSignalHandlercustomFunctionPropChanged, ); #[doc(hidden)] - fn call_MyObject_signal_handler_testNameChanged( - handler: &mut MyObjectCxxQtSignalHandlertestNameChanged, + fn call_MyObject_signal_handler_customFunctionPropChanged( + handler: &mut MyObjectCxxQtSignalHandlercustomFunctionPropChanged, self_value: Pin<&mut MyObject>, ); } @@ -199,6 +212,32 @@ impl ffi::MyObject { self.as_mut().trivial_changed(); } } +impl ffi::MyObject { + #[doc = "Getter for the Q_PROPERTY "] + #[doc = "readonly_prop"] + pub fn readonly_prop(&self) -> &i32 { + &self.readonly_prop + } +} +impl ffi::MyObject { + #[doc = "Getter for the Q_PROPERTY "] + #[doc = "custom_on_changed"] + pub fn custom_on_changed(&self) -> &i32 { + &self.custom_on_changed + } +} +impl ffi::MyObject { + #[doc = "Setter for the Q_PROPERTY "] + #[doc = "custom_on_changed"] + pub fn set_custom_on_changed(mut self: core::pin::Pin<&mut Self>, value: i32) { + use cxx_qt::CxxQtType; + if self.custom_on_changed == value { + return; + } + self.as_mut().rust_mut().custom_on_changed = value; + self.as_mut().myOnChanged(); + } +} impl ffi::MyObject { #[doc = "Connect the given function pointer to the signal "] #[doc = "primitiveChanged"] @@ -319,18 +358,20 @@ cxx_qt::static_assertions::assert_eq_size!( ); impl ffi::MyObject { #[doc = "Connect the given function pointer to the signal "] - #[doc = "testNameChanged"] + #[doc = "customFunctionPropChanged"] #[doc = ", so that when the signal is emitted the function pointer is executed."] - pub fn connect_test_name_changed) + 'static>( + pub fn connect_custom_function_prop_changed< + F: FnMut(core::pin::Pin<&mut ffi::MyObject>) + 'static, + >( self: core::pin::Pin<&mut ffi::MyObject>, mut closure: F, conn_type: cxx_qt::ConnectionType, ) -> cxx_qt::QMetaObjectConnectionGuard { cxx_qt::QMetaObjectConnectionGuard::from( - ffi::MyObject_connect_test_name_changed( + ffi::MyObject_connect_custom_function_prop_changed( self, cxx_qt::signalhandler::CxxQtSignalHandler::< - MyObjectCxxQtSignalClosuretestNameChanged, + MyObjectCxxQtSignalClosurecustomFunctionPropChanged, >::new(Box::new(closure)), conn_type, ), @@ -339,19 +380,21 @@ impl ffi::MyObject { } impl ffi::MyObject { #[doc = "Connect the given function pointer to the signal "] - #[doc = "testNameChanged"] + #[doc = "customFunctionPropChanged"] #[doc = ", so that when the signal is emitted the function pointer is executed."] #[doc = "\n"] #[doc = "Note that this method uses a AutoConnection connection type."] - pub fn on_test_name_changed) + 'static>( + pub fn on_custom_function_prop_changed< + F: FnMut(core::pin::Pin<&mut ffi::MyObject>) + 'static, + >( self: core::pin::Pin<&mut ffi::MyObject>, mut closure: F, ) -> cxx_qt::QMetaObjectConnectionGuard { cxx_qt::QMetaObjectConnectionGuard::from( - ffi::MyObject_connect_test_name_changed( + ffi::MyObject_connect_custom_function_prop_changed( self, cxx_qt::signalhandler::CxxQtSignalHandler::< - MyObjectCxxQtSignalClosuretestNameChanged, + MyObjectCxxQtSignalClosurecustomFunctionPropChanged, >::new(Box::new(closure)), cxx_qt::ConnectionType::AutoConnection, ), @@ -359,30 +402,30 @@ impl ffi::MyObject { } } #[doc(hidden)] -pub struct MyObjectCxxQtSignalClosuretestNameChanged {} +pub struct MyObjectCxxQtSignalClosurecustomFunctionPropChanged {} impl cxx_qt::signalhandler::CxxQtSignalHandlerClosure - for MyObjectCxxQtSignalClosuretestNameChanged + for MyObjectCxxQtSignalClosurecustomFunctionPropChanged { type Id = cxx::type_id!( - "::cxx_qt::my_object::rust::cxxqtgen1::MyObjectCxxQtSignalHandlertestNameChanged" + "::cxx_qt::my_object::rust::cxxqtgen1::MyObjectCxxQtSignalHandlercustomFunctionPropChanged" ); type FnType = dyn FnMut(core::pin::Pin<&mut ffi::MyObject>); } -use core::mem::drop as drop_MyObject_signal_handler_testNameChanged; -fn call_MyObject_signal_handler_testNameChanged( +use core::mem::drop as drop_MyObject_signal_handler_customFunctionPropChanged; +fn call_MyObject_signal_handler_customFunctionPropChanged( handler: &mut cxx_qt::signalhandler::CxxQtSignalHandler< - MyObjectCxxQtSignalClosuretestNameChanged, + MyObjectCxxQtSignalClosurecustomFunctionPropChanged, >, self_value: core::pin::Pin<&mut ffi::MyObject>, ) { handler.closure()(self_value); } cxx_qt::static_assertions::assert_eq_align!( - cxx_qt::signalhandler::CxxQtSignalHandler, + cxx_qt::signalhandler::CxxQtSignalHandler, usize ); cxx_qt::static_assertions::assert_eq_size!( - cxx_qt::signalhandler::CxxQtSignalHandler, + cxx_qt::signalhandler::CxxQtSignalHandler, [usize; 2] ); impl cxx_qt::Locking for ffi::MyObject {} From 73e3cc90b46f8b857a39537a8f88a0a17cf93b7a Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Wed, 17 Jul 2024 15:10:42 +0100 Subject: [PATCH 12/18] Refactor to use a new NameState enum to simplify checks before generation --- .../src/generator/cpp/property/getter.rs | 49 ++++++---- .../src/generator/cpp/property/meta.rs | 8 +- .../src/generator/cpp/property/mod.rs | 56 +++++++---- .../src/generator/cpp/property/setter.rs | 9 +- .../src/generator/cpp/property/signal.rs | 7 +- .../src/generator/naming/property.rs | 96 ++++++++++++++----- .../src/generator/rust/property/getter.rs | 59 +++++++----- .../src/generator/rust/property/mod.rs | 68 ++++++++----- .../src/generator/rust/property/setter.rs | 9 +- .../src/generator/rust/property/signal.rs | 7 +- 10 files changed, 246 insertions(+), 122 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/getter.rs b/crates/cxx-qt-gen/src/generator/cpp/property/getter.rs index d9823cfe1..45b6dc50f 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/getter.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/getter.rs @@ -3,27 +3,38 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::generator::{cpp::fragment::CppFragment, naming::property::QPropertyNames}; +use crate::generator::{ + cpp::fragment::CppFragment, + naming::property::{NameState, QPropertyNames}, +}; use indoc::formatdoc; -pub fn generate(idents: &QPropertyNames, qobject_ident: &str, return_cxx_ty: &str) -> CppFragment { - CppFragment::Pair { - header: format!( - "{return_cxx_ty} const& {ident_getter}() const;", - ident_getter = idents.getter.cxx_unqualified() - ), - source: formatdoc!( - r#" - {return_cxx_ty} const& - {qobject_ident}::{ident_getter}() const - {{ - const ::rust::cxxqt1::MaybeLockGuard<{qobject_ident}> guard(*this); - return {ident_getter_wrapper}(); - }} - "#, - ident_getter = idents.getter.cxx_unqualified(), - ident_getter_wrapper = idents.getter_wrapper.cxx_unqualified(), - ), +pub fn generate( + idents: &QPropertyNames, + qobject_ident: &str, + return_cxx_ty: &str, +) -> Option { + if let NameState::Auto(name) = &idents.getter { + Some(CppFragment::Pair { + header: format!( + "{return_cxx_ty} const& {ident_getter}() const;", + ident_getter = name.cxx_unqualified() + ), + source: formatdoc!( + r#" + {return_cxx_ty} const& + {qobject_ident}::{ident_getter}() const + {{ + const ::rust::cxxqt1::MaybeLockGuard<{qobject_ident}> guard(*this); + return {ident_getter_wrapper}(); + }} + "#, + ident_getter = name.cxx_unqualified(), + ident_getter_wrapper = idents.getter_wrapper.cxx_unqualified(), + ), + }) + } else { + None } } diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs b/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs index 8ea5af1aa..d8f588358 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs @@ -12,12 +12,12 @@ pub fn generate(idents: &QPropertyNames, cxx_ty: &str) -> String { ident_getter = idents.getter.cxx_unqualified() )]; // Write - if let Some(name) = &idents.setter { - parts.push(format!("WRITE {}", name.cxx_unqualified())); + if let Some(setter) = &idents.setter { + parts.push(format!("WRITE {}", setter.cxx_unqualified())); } // Notify - if let Some(name) = &idents.notify { - parts.push(format!("NOTIFY {}", name.cxx_unqualified())); + if let Some(notify) = &idents.notify { + parts.push(format!("NOTIFY {}", notify.cxx_unqualified())); } format!( diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs index 2e595299f..dd4107c15 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs @@ -31,36 +31,56 @@ pub fn generate_cpp_properties( let idents = QPropertyNames::from(property); let cxx_ty = syn_type_to_cpp_type(&property.ty, type_names)?; - let flags = &property.flags; - generated.metaobjects.push(meta::generate(&idents, &cxx_ty)); + // let flags = &property.flags; + // None indicates no custom identifier was provided - if flags.read.is_auto() { - generated - .methods - .push(getter::generate(&idents, &qobject_ident, &cxx_ty)); + // if flags.read.is_auto() { + // generated + // .methods + // .push(getter::generate(&idents, &qobject_ident, &cxx_ty)); + // generated + // .private_methods + // .push(getter::generate_wrapper(&idents, &cxx_ty)); + // } + + // // Checking custom setter wasn't provided + // if flags.write.clone().is_some_and(|state| state.is_auto()) { + // if let Some(setter) = setter::generate(&idents, &qobject_ident, &cxx_ty) { + // generated.methods.push(setter) + // } + + // if let Some(setter_wrapper) = setter::generate_wrapper(&idents, &cxx_ty) { + // generated.private_methods.push(setter_wrapper) + // } + // } + + // // Checking custom notifier wasn't provided + // if flags.notify.clone().is_some_and(|state| state.is_auto()) { + // if let Some(notify) = signal::generate(&idents, qobject_idents) { + // signals.push(notify) + // } + // } + + if let Some(getter) = getter::generate(&idents, &qobject_ident, &cxx_ty) { + generated.methods.push(getter); generated .private_methods .push(getter::generate_wrapper(&idents, &cxx_ty)); } // Checking custom setter wasn't provided - if flags.write.clone().is_some_and(|state| state.is_auto()) { - if let Some(setter) = setter::generate(&idents, &qobject_ident, &cxx_ty) { - generated.methods.push(setter) - } + if let Some(setter) = setter::generate(&idents, &qobject_ident, &cxx_ty) { + generated.methods.push(setter) + } - if let Some(setter_wrapper) = setter::generate_wrapper(&idents, &cxx_ty) { - generated.private_methods.push(setter_wrapper) - } + if let Some(setter_wrapper) = setter::generate_wrapper(&idents, &cxx_ty) { + generated.private_methods.push(setter_wrapper) } - // Checking custom notifier wasn't provided - if flags.notify.clone().is_some_and(|state| state.is_auto()) { - if let Some(notify) = signal::generate(&idents, qobject_idents) { - signals.push(notify) - } + if let Some(notify) = signal::generate(&idents, qobject_idents) { + signals.push(notify) } } diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs index 23422102b..0ae2d58c7 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs @@ -3,12 +3,17 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::generator::{cpp::fragment::CppFragment, naming::property::QPropertyNames}; +use crate::generator::{ + cpp::fragment::CppFragment, + naming::property::{NameState, QPropertyNames}, +}; use indoc::formatdoc; pub fn generate(idents: &QPropertyNames, qobject_ident: &str, cxx_ty: &str) -> Option { // Checking whether a setter should be generated based off if it has a name - if let (Some(setter), Some(setter_wrapper)) = (&idents.setter, &idents.setter_wrapper) { + if let (Some(NameState::Auto(setter)), Some(setter_wrapper)) = + (&idents.setter, &idents.setter_wrapper) + { Some(CppFragment::Pair { header: format!( "Q_SLOT void {ident_setter}({cxx_ty} const& value);", diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs b/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs index a5d090852..da77f4f2d 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs @@ -6,7 +6,10 @@ use syn::ForeignItemFn; use crate::{ - generator::naming::{property::QPropertyNames, qobject::QObjectNames}, + generator::naming::{ + property::{NameState, QPropertyNames}, + qobject::QObjectNames, + }, parser::signals::ParsedSignal, }; @@ -14,7 +17,7 @@ pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Optio // We build our signal in the generation phase as we need to use the naming // structs to build the signal name let cpp_class_rust = &qobject_idents.name.rust_unqualified(); - if let Some(notify) = &idents.notify { + if let Some(NameState::Auto(notify)) = &idents.notify { let notify_cpp = notify.cxx_unqualified(); let notify_rust = notify.rust_unqualified(); let method: ForeignItemFn = syn::parse_quote! { diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs index 8a9cfba27..8ab627ffa 100644 --- a/crates/cxx-qt-gen/src/generator/naming/property.rs +++ b/crates/cxx-qt-gen/src/generator/naming/property.rs @@ -10,14 +10,42 @@ use convert_case::{Case, Casing}; use quote::format_ident; use syn::Ident; +use std::ops::Deref; + +#[derive(Debug)] +pub enum NameState { + Auto(Name), + Custom(Name), +} + +impl Deref for NameState { + type Target = Name; + + fn deref(&self) -> &Self::Target { + match self { + Self::Auto(name) => name, + Self::Custom(name) => name, + } + } +} + +impl NameState { + pub fn from_flag_with_auto_fn(state: &FlagState, auto_fn: impl Fn() -> Name) -> Self { + match state { + FlagState::Auto => Self::Auto(auto_fn()), + FlagState::Custom(ident) => Self::Custom(Name::new(ident.clone())), + } + } +} + /// Names for parts of a Q_PROPERTY pub struct QPropertyNames { pub name: Name, - pub getter: Name, + pub getter: NameState, pub getter_wrapper: Name, - pub setter: Option, + pub setter: Option, pub setter_wrapper: Option, - pub notify: Option, + pub notify: Option, } impl From<&ParsedQProperty> for QPropertyNames { @@ -26,29 +54,49 @@ impl From<&ParsedQProperty> for QPropertyNames { let flags = &property.flags; - let getter = match &flags.read { - FlagState::Auto => getter_name_from_property(&property_name), - FlagState::Custom(ident) => Name::new(ident.clone()), - }; - - let setter = match &flags.write { - Some(state) => match state { - FlagState::Auto => Some(setter_name_from_property(&property_name)), - FlagState::Custom(ident) => Some(Name::new(ident.clone())), - }, - None => None, - }; - - let notify = match &flags.notify { - Some(state) => match state { - FlagState::Auto => Some(notify_name_from_property(&property_name)), - FlagState::Custom(ident) => Some(Name::new(ident.clone())), - }, - None => None, + // let getter = match &flags.read { + // FlagState::Auto => getter_name_from_property(&property_name), + // FlagState::Custom(ident) => Name::new(ident.clone()), + // }; + + // let setter = match &flags.write { + // Some(state) => match state { + // FlagState::Auto => Some(setter_name_from_property(&property_name)), + // FlagState::Custom(ident) => Some(Name::new(ident.clone())), + // }, + // None => None, + // }; + + // let notify = match &flags.notify { + // Some(state) => match state { + // FlagState::Auto => Some(notify_name_from_property(&property_name)), + // FlagState::Custom(ident) => Some(Name::new(ident.clone())), + // }, + // None => None, + // }; + + let getter = NameState::from_flag_with_auto_fn(&flags.read, || { + getter_name_from_property(&property_name) + }); + + let setter = flags.write.as_ref().map(|setter| { + NameState::from_flag_with_auto_fn(setter, || setter_name_from_property(&property_name)) + }); + + let notify = flags.notify.as_ref().map(|notify| { + NameState::from_flag_with_auto_fn(notify, || notify_name_from_property(&property_name)) + }); + + // let setter_wrapper = setter + // .as_ref() + // .map(|state| wrapper_name_from_function_name(state)); + + let setter_wrapper = if let Some(NameState::Auto(ref setter)) = setter { + Some(wrapper_name_from_function_name(setter)) + } else { + None }; - let setter_wrapper = setter.as_ref().map(wrapper_name_from_function_name); - Self { getter_wrapper: wrapper_name_from_function_name(&getter), getter, diff --git a/crates/cxx-qt-gen/src/generator/rust/property/getter.rs b/crates/cxx-qt-gen/src/generator/rust/property/getter.rs index a80731b2e..a23438198 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/getter.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/getter.rs @@ -5,7 +5,10 @@ use crate::{ generator::{ - naming::{property::QPropertyNames, qobject::QObjectNames}, + naming::{ + property::{NameState, QPropertyNames}, + qobject::QObjectNames, + }, rust::fragment::RustFragmentPair, }, naming::rust::syn_type_cxx_bridge_to_qualified, @@ -19,31 +22,35 @@ pub fn generate( qobject_idents: &QObjectNames, cxx_ty: &Type, type_names: &TypeNames, -) -> Result { - let cpp_class_name_rust = &qobject_idents.name.rust_unqualified(); - let getter_wrapper_cpp = idents.getter_wrapper.cxx_unqualified(); - let getter_rust = idents.getter.rust_unqualified(); - let ident = idents.name.rust_unqualified(); - let ident_str = ident.to_string(); - let qualified_ty = syn_type_cxx_bridge_to_qualified(cxx_ty, type_names)?; - let qualified_impl = type_names.rust_qualified(cpp_class_name_rust)?; +) -> Result> { + if let NameState::Auto(getter) = &idents.getter { + let cpp_class_name_rust = &qobject_idents.name.rust_unqualified(); + let getter_wrapper_cpp = idents.getter_wrapper.cxx_unqualified(); + let getter_rust = getter.rust_unqualified(); + let ident = idents.name.rust_unqualified(); + let ident_str = ident.to_string(); + let qualified_ty = syn_type_cxx_bridge_to_qualified(cxx_ty, type_names)?; + let qualified_impl = type_names.rust_qualified(cpp_class_name_rust)?; - Ok(RustFragmentPair { - cxx_bridge: vec![quote! { - extern "Rust" { - #[cxx_name = #getter_wrapper_cpp] - // TODO: Add #[namespace] of the QObject to the declaration - unsafe fn #getter_rust<'a>(self: &'a #cpp_class_name_rust) -> &'a #cxx_ty; - } - }], - implementation: vec![quote! { - impl #qualified_impl { - #[doc = "Getter for the Q_PROPERTY "] - #[doc = #ident_str] - pub fn #getter_rust(&self) -> &#qualified_ty { - &self.#ident + Ok(Some(RustFragmentPair { + cxx_bridge: vec![quote! { + extern "Rust" { + #[cxx_name = #getter_wrapper_cpp] + // TODO: Add #[namespace] of the QObject to the declaration + unsafe fn #getter_rust<'a>(self: &'a #cpp_class_name_rust) -> &'a #cxx_ty; } - } - }], - }) + }], + implementation: vec![quote! { + impl #qualified_impl { + #[doc = "Getter for the Q_PROPERTY "] + #[doc = #ident_str] + pub fn #getter_rust(&self) -> &#qualified_ty { + &self.#ident + } + } + }], + })) + } else { + Ok(None) + } } diff --git a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs index cc7a79268..551c87ccd 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs @@ -31,38 +31,60 @@ pub fn generate_rust_properties( for property in properties { let idents = QPropertyNames::from(property); - let flags = &property.flags; - - if flags.read.is_auto() { - //gen getter and wrapper - let getter = getter::generate(&idents, qobject_idents, &property.ty, type_names)?; + // let flags = &property.flags; + + // if flags.read.is_auto() { + // //gen getter and wrapper + // let getter = getter::generate(&idents, qobject_idents, &property.ty, type_names)?; + // generated + // .cxx_mod_contents + // .append(&mut getter.cxx_bridge_as_items()?); + // generated + // .cxx_qt_mod_contents + // .append(&mut getter.implementation_as_items()?); + // } + + // // Checking that write flag was provided but no custom identifier + // if flags.write.clone().is_some_and(|state| state.is_auto()) { + // // gen setter and wrapper + // if let Some(setter) = + // setter::generate(&idents, qobject_idents, &property.ty, type_names)? + // { + // generated + // .cxx_mod_contents + // .append(&mut setter.cxx_bridge_as_items()?); + // generated + // .cxx_qt_mod_contents + // .append(&mut setter.implementation_as_items()?); + // } + // } + + // if flags.notify.clone().is_some_and(|state| state.is_auto()) { + // if let Some(notify) = signal::generate(&idents, qobject_idents) { + // signals.push(notify) + // } + // } + + if let Some(getter) = getter::generate(&idents, qobject_idents, &property.ty, type_names)? { generated .cxx_mod_contents .append(&mut getter.cxx_bridge_as_items()?); generated .cxx_qt_mod_contents .append(&mut getter.implementation_as_items()?); - } + }; - // Checking that write flag was provided but no custom identifier - if flags.write.clone().is_some_and(|state| state.is_auto()) { - // gen setter and wrapper - if let Some(setter) = - setter::generate(&idents, qobject_idents, &property.ty, type_names)? - { - generated - .cxx_mod_contents - .append(&mut setter.cxx_bridge_as_items()?); - generated - .cxx_qt_mod_contents - .append(&mut setter.implementation_as_items()?); - } + if let Some(setter) = setter::generate(&idents, qobject_idents, &property.ty, type_names)? { + generated + .cxx_mod_contents + .append(&mut setter.cxx_bridge_as_items()?); + generated + .cxx_qt_mod_contents + .append(&mut setter.implementation_as_items()?); } - if flags.notify.clone().is_some_and(|state| state.is_auto()) { - if let Some(notify) = signal::generate(&idents, qobject_idents) { - signals.push(notify) - } + if let Some(notify) = signal::generate(&idents, qobject_idents) { + signals.push(notify) } } diff --git a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs index a4745f43b..7f6bd2f1e 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs @@ -5,7 +5,10 @@ use crate::{ generator::{ - naming::{property::QPropertyNames, qobject::QObjectNames}, + naming::{ + property::{NameState, QPropertyNames}, + qobject::QObjectNames, + }, rust::fragment::RustFragmentPair, }, naming::rust::{syn_type_cxx_bridge_to_qualified, syn_type_is_cxx_bridge_unsafe}, @@ -22,7 +25,9 @@ pub fn generate( ) -> Result> { let cpp_class_name_rust = &qobject_idents.name.rust_unqualified(); - if let (Some(setter), Some(setter_wrapper)) = (&idents.setter, &idents.setter_wrapper) { + if let (Some(NameState::Auto(setter)), Some(setter_wrapper)) = + (&idents.setter, &idents.setter_wrapper) + { let setter_wrapper_cpp = setter_wrapper.cxx_unqualified(); let setter_rust = setter.rust_unqualified(); diff --git a/crates/cxx-qt-gen/src/generator/rust/property/signal.rs b/crates/cxx-qt-gen/src/generator/rust/property/signal.rs index f9b63966a..5070d39be 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/signal.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/signal.rs @@ -6,7 +6,10 @@ use syn::ForeignItemFn; use crate::{ - generator::naming::{property::QPropertyNames, qobject::QObjectNames}, + generator::naming::{ + property::{NameState, QPropertyNames}, + qobject::QObjectNames, + }, parser::signals::ParsedSignal, }; @@ -14,7 +17,7 @@ pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Optio // We build our signal in the generation phase as we need to use the naming // structs to build the signal name let cpp_class_rust = &qobject_idents.name.rust_unqualified(); - if let Some(notify) = &idents.notify { + if let Some(NameState::Auto(notify)) = &idents.notify { let notify_rust = notify.rust_unqualified(); let notify_cpp_str = notify.cxx_unqualified(); From 5dfe239bb1cdb77fb0af5020ef9b60047e0ce174 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Wed, 17 Jul 2024 17:07:40 +0100 Subject: [PATCH 13/18] Cleanup and comments --- .../src/generator/cpp/property/meta.rs | 4 +- .../src/generator/cpp/property/mod.rs | 40 ++----------------- .../src/generator/cpp/property/setter.rs | 2 +- .../src/generator/naming/property.rs | 28 +------------ .../src/generator/rust/property/mod.rs | 34 ---------------- crates/cxx-qt-gen/src/parser/property.rs | 11 +++-- 6 files changed, 13 insertions(+), 106 deletions(-) diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs b/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs index d8f588358..4527c60fe 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/meta.rs @@ -11,11 +11,11 @@ pub fn generate(idents: &QPropertyNames, cxx_ty: &str) -> String { "READ {ident_getter}", ident_getter = idents.getter.cxx_unqualified() )]; - // Write + if let Some(setter) = &idents.setter { parts.push(format!("WRITE {}", setter.cxx_unqualified())); } - // Notify + if let Some(notify) = &idents.notify { parts.push(format!("NOTIFY {}", notify.cxx_unqualified())); } diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs index dd4107c15..f14a3d4b2 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs @@ -27,42 +27,12 @@ pub fn generate_cpp_properties( let qobject_ident = qobject_idents.name.cxx_unqualified(); for property in properties { - // Cache the idents and flags as they are used in multiple places + // Cache the idents as they are used in multiple places let idents = QPropertyNames::from(property); let cxx_ty = syn_type_to_cpp_type(&property.ty, type_names)?; generated.metaobjects.push(meta::generate(&idents, &cxx_ty)); - // let flags = &property.flags; - - // None indicates no custom identifier was provided - // if flags.read.is_auto() { - // generated - // .methods - // .push(getter::generate(&idents, &qobject_ident, &cxx_ty)); - // generated - // .private_methods - // .push(getter::generate_wrapper(&idents, &cxx_ty)); - // } - - // // Checking custom setter wasn't provided - // if flags.write.clone().is_some_and(|state| state.is_auto()) { - // if let Some(setter) = setter::generate(&idents, &qobject_ident, &cxx_ty) { - // generated.methods.push(setter) - // } - - // if let Some(setter_wrapper) = setter::generate_wrapper(&idents, &cxx_ty) { - // generated.private_methods.push(setter_wrapper) - // } - // } - - // // Checking custom notifier wasn't provided - // if flags.notify.clone().is_some_and(|state| state.is_auto()) { - // if let Some(notify) = signal::generate(&idents, qobject_idents) { - // signals.push(notify) - // } - // } - if let Some(getter) = getter::generate(&idents, &qobject_ident, &cxx_ty) { generated.methods.push(getter); generated @@ -70,7 +40,8 @@ pub fn generate_cpp_properties( .push(getter::generate_wrapper(&idents, &cxx_ty)); } - // Checking custom setter wasn't provided + // Optional generations, returning None if flags specified not to generate anything + if let Some(setter) = setter::generate(&idents, &qobject_ident, &cxx_ty) { generated.methods.push(setter) } @@ -120,11 +91,6 @@ mod tests { let type_names = TypeNames::mock(); let generated = generate_cpp_properties(&properties, &qobject_idents, &type_names).unwrap(); - // TODO: write better tests using the string comparisons - - // Meta - println!("Generated meta: {:?}\n\n", generated.metaobjects); - println!("Generated methods: {:?}\n\n", generated.methods); assert_eq!(generated.metaobjects.len(), 1); assert_str_eq!( diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs index 0ae2d58c7..c9f9878c4 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs @@ -10,7 +10,7 @@ use crate::generator::{ use indoc::formatdoc; pub fn generate(idents: &QPropertyNames, qobject_ident: &str, cxx_ty: &str) -> Option { - // Checking whether a setter should be generated based off if it has a name + // Only generates setter code if the state provided is Auto (not custom provided by user) if let (Some(NameState::Auto(setter)), Some(setter_wrapper)) = (&idents.setter, &idents.setter_wrapper) { diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs index 8ab627ffa..70c75e1ea 100644 --- a/crates/cxx-qt-gen/src/generator/naming/property.rs +++ b/crates/cxx-qt-gen/src/generator/naming/property.rs @@ -33,7 +33,7 @@ impl NameState { pub fn from_flag_with_auto_fn(state: &FlagState, auto_fn: impl Fn() -> Name) -> Self { match state { FlagState::Auto => Self::Auto(auto_fn()), - FlagState::Custom(ident) => Self::Custom(Name::new(ident.clone())), + FlagState::Custom(ident) => Self::Custom(Name::new(ident.clone())), // TODO: replace this with some sort of type / method name lookup } } } @@ -52,29 +52,9 @@ impl From<&ParsedQProperty> for QPropertyNames { fn from(property: &ParsedQProperty) -> Self { let property_name = property_name_from_rust_name(property.ident.clone()); + // Cache flags as they are accessed multiple times let flags = &property.flags; - // let getter = match &flags.read { - // FlagState::Auto => getter_name_from_property(&property_name), - // FlagState::Custom(ident) => Name::new(ident.clone()), - // }; - - // let setter = match &flags.write { - // Some(state) => match state { - // FlagState::Auto => Some(setter_name_from_property(&property_name)), - // FlagState::Custom(ident) => Some(Name::new(ident.clone())), - // }, - // None => None, - // }; - - // let notify = match &flags.notify { - // Some(state) => match state { - // FlagState::Auto => Some(notify_name_from_property(&property_name)), - // FlagState::Custom(ident) => Some(Name::new(ident.clone())), - // }, - // None => None, - // }; - let getter = NameState::from_flag_with_auto_fn(&flags.read, || { getter_name_from_property(&property_name) }); @@ -87,10 +67,6 @@ impl From<&ParsedQProperty> for QPropertyNames { NameState::from_flag_with_auto_fn(notify, || notify_name_from_property(&property_name)) }); - // let setter_wrapper = setter - // .as_ref() - // .map(|state| wrapper_name_from_function_name(state)); - let setter_wrapper = if let Some(NameState::Auto(ref setter)) = setter { Some(wrapper_name_from_function_name(setter)) } else { diff --git a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs index 551c87ccd..4b557548d 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs @@ -31,40 +31,6 @@ pub fn generate_rust_properties( for property in properties { let idents = QPropertyNames::from(property); - // let flags = &property.flags; - - // if flags.read.is_auto() { - // //gen getter and wrapper - // let getter = getter::generate(&idents, qobject_idents, &property.ty, type_names)?; - // generated - // .cxx_mod_contents - // .append(&mut getter.cxx_bridge_as_items()?); - // generated - // .cxx_qt_mod_contents - // .append(&mut getter.implementation_as_items()?); - // } - - // // Checking that write flag was provided but no custom identifier - // if flags.write.clone().is_some_and(|state| state.is_auto()) { - // // gen setter and wrapper - // if let Some(setter) = - // setter::generate(&idents, qobject_idents, &property.ty, type_names)? - // { - // generated - // .cxx_mod_contents - // .append(&mut setter.cxx_bridge_as_items()?); - // generated - // .cxx_qt_mod_contents - // .append(&mut setter.implementation_as_items()?); - // } - // } - - // if flags.notify.clone().is_some_and(|state| state.is_auto()) { - // if let Some(notify) = signal::generate(&idents, qobject_idents) { - // signals.push(notify) - // } - // } - if let Some(getter) = getter::generate(&idents, qobject_idents, &property.ty, type_names)? { generated .cxx_mod_contents diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index a9c127ac3..03a02db86 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -10,7 +10,6 @@ use syn::{ Attribute, Expr, Ident, Meta, MetaNameValue, Result, Token, Type, }; - #[derive(Debug, Clone)] pub enum FlagState { Auto, // Might need to refactor to also store the generated ident here @@ -44,7 +43,7 @@ impl Default for QPropertyFlags { impl QPropertyFlags { // Doesn't really represent a realistic state, just used for collecting in the parser - pub fn new() -> Self { + fn new() -> Self { Self { read: FlagState::Auto, write: None, @@ -89,7 +88,7 @@ impl ParsedQProperty { let ident = input.parse()?; if input.is_empty() { - // No flags passed so desugar: #[qproperty(T, ident)] -> #[qproperty(T, ident, read, write, notify)] + // No flags passed so desugar: ```#[qproperty(T, ident)]``` -> ```#[qproperty(T, ident, read, write, notify)]``` Ok(Self { ident, ty, @@ -122,12 +121,12 @@ impl ParsedQProperty { } }; *variable = Some(value.map_or(FlagState::Auto, FlagState::Custom)); - + Ok(()) }; for flag in flags { - // Could maybe refactor parse_meta_name_value to parse_meta, + // Could maybe refactor parse_meta_name_value to parse_meta, // which would return an (Ident, Option) and extract the logic below to a new fn match flag { Meta::Path(path) => { @@ -180,7 +179,7 @@ mod tests { #[test] fn test_parse_property() { let mut input: ItemStruct = parse_quote! { - #[qproperty(T, name)] + #[qproperty(T, name, read, write = myGetter,)] struct MyStruct; }; let property = ParsedQProperty::parse(input.attrs.remove(0)).unwrap(); From dd5097854fd5eccce70af8aa97976c54d6f3fbb2 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Fri, 19 Jul 2024 15:33:33 +0100 Subject: [PATCH 14/18] Fix nitpicks in PR and update docs and changelog with changes --- CHANGELOG.md | 1 + book/src/bridge/extern_rustqt.md | 14 +++- .../src/generator/cpp/property/getter.rs | 17 ++-- .../src/generator/cpp/property/mod.rs | 7 +- .../src/generator/cpp/property/signal.rs | 2 +- .../src/generator/naming/property.rs | 10 ++- .../src/generator/rust/property/getter.rs | 6 +- .../src/generator/rust/property/setter.rs | 1 - crates/cxx-qt-gen/src/parser/property.rs | 82 +++++++++---------- 9 files changed, 77 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44f42ad5f..4de769269 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased](https://github.com/KDAB/cxx-qt/compare/v0.6.1...HEAD) +- Add support for specifying read write and notify in qproperty macro, including support for custom user defined functions ### Added diff --git a/book/src/bridge/extern_rustqt.md b/book/src/bridge/extern_rustqt.md index a3bbe2ae8..46c728243 100644 --- a/book/src/bridge/extern_rustqt.md +++ b/book/src/bridge/extern_rustqt.md @@ -131,7 +131,19 @@ Where `` is the name of the property. These setters and getters assure that the changed signal is emitted every time the property is edited. -> Note that in the future it will be possible to specify custom getters and setters +It is also possible to specify custom getters, setters and on-changed functions, using flags passed like so: +`#[qproperty(TYPE, NAME, read = myGetter, write = mySetter, notify = myOnChanged)]` + +> Note: currently TypeName lookups are not currently supported, so either the name must be camel case or specified like `#[cxx_name = "my_getter"]` + +It is also possible to use any combination of custom functions or omitting entirely, but if flags are specified, read must be one of them as all properties need to be able to be read. + +### Examples: +- `#[qproperty(TYPE, NAME, read)]` A read only prop +- `#[qproperty(TYPE, NAME, read = myGetter, write, notify)]` custom getter provided but will generate setter and on-changed +- `#[qproperty(TYPE, NAME)]` is syntactic sugar for `#[qproperty(TYPE, NAME, read, write, notify)]` +- `#[qproperty(TYPE, NAME, write)]` is an error as read was not passed + ## Methods diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/getter.rs b/crates/cxx-qt-gen/src/generator/cpp/property/getter.rs index 45b6dc50f..f209b5e52 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/getter.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/getter.rs @@ -14,7 +14,8 @@ pub fn generate( qobject_ident: &str, return_cxx_ty: &str, ) -> Option { - if let NameState::Auto(name) = &idents.getter { + if let (NameState::Auto(name), Some(getter_wrapper)) = (&idents.getter, &idents.getter_wrapper) + { Some(CppFragment::Pair { header: format!( "{return_cxx_ty} const& {ident_getter}() const;", @@ -30,7 +31,7 @@ pub fn generate( }} "#, ident_getter = name.cxx_unqualified(), - ident_getter_wrapper = idents.getter_wrapper.cxx_unqualified(), + ident_getter_wrapper = getter_wrapper.cxx_unqualified(), ), }) } else { @@ -38,9 +39,11 @@ pub fn generate( } } -pub fn generate_wrapper(idents: &QPropertyNames, cxx_ty: &str) -> CppFragment { - CppFragment::Header(format!( - "{cxx_ty} const& {ident_getter_wrapper}() const noexcept;", - ident_getter_wrapper = idents.getter_wrapper.cxx_unqualified() - )) +pub fn generate_wrapper(idents: &QPropertyNames, cxx_ty: &str) -> Option { + idents.getter_wrapper.as_ref().map(|name| { + CppFragment::Header(format!( + "{cxx_ty} const& {ident_getter_wrapper}() const noexcept;", + ident_getter_wrapper = name.cxx_unqualified() + )) + }) } diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs index f14a3d4b2..847c1850b 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs @@ -35,12 +35,11 @@ pub fn generate_cpp_properties( if let Some(getter) = getter::generate(&idents, &qobject_ident, &cxx_ty) { generated.methods.push(getter); - generated - .private_methods - .push(getter::generate_wrapper(&idents, &cxx_ty)); } - // Optional generations, returning None if flags specified not to generate anything + if let Some(getter_wrapper) = getter::generate_wrapper(&idents, &cxx_ty) { + generated.private_methods.push(getter_wrapper); + } if let Some(setter) = setter::generate(&idents, &qobject_ident, &cxx_ty) { generated.methods.push(setter) diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs b/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs index da77f4f2d..bc2ccbac1 100644 --- a/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs +++ b/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs @@ -16,8 +16,8 @@ use crate::{ pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Option { // We build our signal in the generation phase as we need to use the naming // structs to build the signal name - let cpp_class_rust = &qobject_idents.name.rust_unqualified(); if let Some(NameState::Auto(notify)) = &idents.notify { + let cpp_class_rust = &qobject_idents.name.rust_unqualified(); let notify_cpp = notify.cxx_unqualified(); let notify_rust = notify.rust_unqualified(); let method: ForeignItemFn = syn::parse_quote! { diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs index 70c75e1ea..160343106 100644 --- a/crates/cxx-qt-gen/src/generator/naming/property.rs +++ b/crates/cxx-qt-gen/src/generator/naming/property.rs @@ -42,7 +42,7 @@ impl NameState { pub struct QPropertyNames { pub name: Name, pub getter: NameState, - pub getter_wrapper: Name, + pub getter_wrapper: Option, pub setter: Option, pub setter_wrapper: Option, pub notify: Option, @@ -73,8 +73,14 @@ impl From<&ParsedQProperty> for QPropertyNames { None }; + let getter_wrapper = if let NameState::Auto(ref getter) = getter { + Some(wrapper_name_from_function_name(getter)) + } else { + None + }; + Self { - getter_wrapper: wrapper_name_from_function_name(&getter), + getter_wrapper, getter, setter_wrapper, setter, diff --git a/crates/cxx-qt-gen/src/generator/rust/property/getter.rs b/crates/cxx-qt-gen/src/generator/rust/property/getter.rs index a23438198..0bfc369af 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/getter.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/getter.rs @@ -23,9 +23,11 @@ pub fn generate( cxx_ty: &Type, type_names: &TypeNames, ) -> Result> { - if let NameState::Auto(getter) = &idents.getter { + if let (NameState::Auto(getter), Some(getter_wrapper)) = + (&idents.getter, &idents.getter_wrapper) + { let cpp_class_name_rust = &qobject_idents.name.rust_unqualified(); - let getter_wrapper_cpp = idents.getter_wrapper.cxx_unqualified(); + let getter_wrapper_cpp = getter_wrapper.cxx_unqualified(); let getter_rust = getter.rust_unqualified(); let ident = idents.name.rust_unqualified(); let ident_str = ident.to_string(); diff --git a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs index 7f6bd2f1e..499bc236b 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/setter.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/setter.rs @@ -67,7 +67,6 @@ pub fn generate( #[doc = #ident_str] pub fn #setter_rust(mut self: core::pin::Pin<&mut Self>, value: #qualified_ty) { use cxx_qt::CxxQtType; - if self.#ident == value { // don't want to set the value again and reemit the signal, // as this can cause binding loops diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index 03a02db86..9f708737e 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -10,13 +10,14 @@ use syn::{ Attribute, Expr, Ident, Meta, MetaNameValue, Result, Token, Type, }; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum FlagState { Auto, // Might need to refactor to also store the generated ident here Custom(Ident), } impl FlagState { + #[cfg(test)] pub fn is_auto(&self) -> bool { matches!(self, Self::Auto) } @@ -41,17 +42,6 @@ impl Default for QPropertyFlags { } } -impl QPropertyFlags { - // Doesn't really represent a realistic state, just used for collecting in the parser - fn new() -> Self { - Self { - read: FlagState::Auto, - write: None, - notify: None, - } - } -} - /// Describes a single Q_PROPERTY for a struct pub struct ParsedQProperty { /// The [syn::Ident] of the property @@ -62,7 +52,6 @@ pub struct ParsedQProperty { pub flags: QPropertyFlags, } -// TODO: Returning struct instead of tuple might be more descriptive fn parse_meta_name_value(name_value: &MetaNameValue) -> Result<(Ident, Ident)> { let ident = name_value.path.require_ident()?.clone(); let expr = &name_value.value; @@ -80,6 +69,20 @@ fn parse_meta_name_value(name_value: &MetaNameValue) -> Result<(Ident, Ident)> { Ok((ident, func_signature)) } +fn parse_meta(meta: Meta) -> Result<(Ident, Option)> { + match meta { + Meta::Path(path) => Ok((path.require_ident()?.clone(), None)), + Meta::NameValue(name_value) => { + let (field, ident) = parse_meta_name_value(&name_value)?; + Ok((field, Some(ident))) + } + _ => Err(Error::new( + meta.span(), + "Invalid syntax, flags must be specified as either `read` or `read = my_getter`", + )), + } +} + impl ParsedQProperty { pub fn parse(attr: Attribute) -> Result { attr.parse_args_with(|input: ParseStream| -> Result { @@ -88,7 +91,7 @@ impl ParsedQProperty { let ident = input.parse()?; if input.is_empty() { - // No flags passed so desugar: ```#[qproperty(T, ident)]``` -> ```#[qproperty(T, ident, read, write, notify)]``` + // No flags passed so desugar: #[qproperty(T, ident)] -> #[qproperty(T, ident, read, write, notify)] Ok(Self { ident, ty, @@ -126,25 +129,8 @@ impl ParsedQProperty { }; for flag in flags { - // Could maybe refactor parse_meta_name_value to parse_meta, - // which would return an (Ident, Option) and extract the logic below to a new fn - match flag { - Meta::Path(path) => { - // Set flag as auto - update_fields(path.require_ident()?, None)?; - }, - Meta::NameValue(name_value) => { - let kv_pair = parse_meta_name_value(&name_value)?; - let value = Some(kv_pair.1); - - // Set Flags with Custom value - update_fields(&kv_pair.0, value)?; - }, - _ => return Err(Error::new( - flag.span(), - "Invalid syntax, flags must be specified as either `read` or `read = my_getter`", - )), - }; + let (field, maybe_value) = parse_meta(flag)?; + update_fields(&field, maybe_value)?; } if let Some(read) = read { @@ -157,8 +143,7 @@ impl ParsedQProperty { notify, }, }) - } - else { + } else { Err(Error::new( punctuated_flags.span(), "If flags are passed, read must be explicitly specified", @@ -228,22 +213,29 @@ mod tests { assert_eq!(property.ty, parse_quote! { T }); // Can use assert_matches! when https://github.com/rust-lang/rust/issues/82775 gets stabilised - let expected_read = format_ident!("my_getter"); - assert!(matches!( + // let expected_read = format_ident!("my_getter"); + // assert!(matches!( + // property.flags.read, + // FlagState::Custom(ident) if ident == expected_read + // )); + assert_eq!( property.flags.read, - FlagState::Custom(ident) if ident == expected_read - )); + FlagState::Custom(format_ident!("my_getter")) + ); - assert!(property.flags.write.is_some()); - assert!(property.flags.write.unwrap().is_auto()); + assert_eq!(property.flags.write, Some(FlagState::Auto)); assert!(property.flags.notify.is_some()); - let expected_notify = format_ident!("my_notifier"); - assert!(matches!( + // let expected_notify = format_ident!("my_notifier"); + // assert!(matches!( + // property.flags.notify, + // Some(FlagState::Custom(ident)) if ident == expected_notify + // )); + assert_eq!( property.flags.notify, - Some(FlagState::Custom(ident)) if ident == expected_notify - )); + Some(FlagState::Custom(format_ident!("my_notifier"))) + ); } #[test] From 4c3b3fd39c303bb5579ac37e68bae62f83637745 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Fri, 19 Jul 2024 15:59:07 +0100 Subject: [PATCH 15/18] Fix md lints --- CHANGELOG.md | 1 + book/src/bridge/extern_rustqt.md | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4de769269..acbfb6abf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased](https://github.com/KDAB/cxx-qt/compare/v0.6.1...HEAD) + - Add support for specifying read write and notify in qproperty macro, including support for custom user defined functions ### Added diff --git a/book/src/bridge/extern_rustqt.md b/book/src/bridge/extern_rustqt.md index 46c728243..fc4b120a0 100644 --- a/book/src/bridge/extern_rustqt.md +++ b/book/src/bridge/extern_rustqt.md @@ -138,13 +138,14 @@ It is also possible to specify custom getters, setters and on-changed functions, It is also possible to use any combination of custom functions or omitting entirely, but if flags are specified, read must be one of them as all properties need to be able to be read. -### Examples: +### Examples + - `#[qproperty(TYPE, NAME, read)]` A read only prop - `#[qproperty(TYPE, NAME, read = myGetter, write, notify)]` custom getter provided but will generate setter and on-changed + - `#[qproperty(TYPE, NAME)]` is syntactic sugar for `#[qproperty(TYPE, NAME, read, write, notify)]` - `#[qproperty(TYPE, NAME, write)]` is an error as read was not passed - ## Methods Any signature with a `self` parameter is interpreted as a Rust method and exposed to C++ method for the given type. From eea3a537cd1d81a7413db40245c885b8389adc30 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Mon, 22 Jul 2024 10:38:09 +0100 Subject: [PATCH 16/18] Improve docs and remove some commented code --- CHANGELOG.md | 3 +-- book/src/bridge/extern_rustqt.md | 13 ++++++++----- crates/cxx-qt-gen/src/parser/property.rs | 11 ----------- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index acbfb6abf..ba59bd515 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,8 +17,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/KDAB/cxx-qt/compare/v0.6.1...HEAD) -- Add support for specifying read write and notify in qproperty macro, including support for custom user defined functions - ### Added - Support for further types: `QLine`, `QLineF`, `QImage`, `QPainter`, `QFont`, `QPen`, `QPolygon`, `QPolygonF`, `QRegion` @@ -28,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add cxx-qt-lib-extras crate which contains: `QCommandLineOption`, `QCommandLineParser`, `QElapsedTimer`, `QApplication` - Serde support for `QString` (requires "serde" feature on cxx-qt-lib) - A new QuickControls module, which exposes `QQuickStyle`. This module is enabled by default and is behind the `qt_quickcontrols` feature. +- Add support for specifying read write and notify in qproperty macro, including support for custom user defined functions ### Changed diff --git a/book/src/bridge/extern_rustqt.md b/book/src/bridge/extern_rustqt.md index fc4b120a0..92c8398f5 100644 --- a/book/src/bridge/extern_rustqt.md +++ b/book/src/bridge/extern_rustqt.md @@ -131,20 +131,23 @@ Where `` is the name of the property. These setters and getters assure that the changed signal is emitted every time the property is edited. -It is also possible to specify custom getters, setters and on-changed functions, using flags passed like so: +It is also possible to specify custom getters, setters and notify signals, using flags passed like so: `#[qproperty(TYPE, NAME, read = myGetter, write = mySetter, notify = myOnChanged)]` -> Note: currently TypeName lookups are not currently supported, so either the name must be camel case or specified like `#[cxx_name = "my_getter"]` +> Note: currently the rust name must be in camel case or specified like `#[cxx_name = "my_getter"]` if not -It is also possible to use any combination of custom functions or omitting entirely, but if flags are specified, read must be one of them as all properties need to be able to be read. +It is also possible to use any combination of custom functions or omit them entirely, but if flags are specified, read must be included as all properties need to be able to be read. + +Using the read flag will cause cxx_qt to generate a getter function with an automatic name based off the property. e.g. `#[qproperty(i32, num, read)]` will have a getter function generated called get_num in Rust, and getNum in C++. + +If a custom function is specified, an implementation both in qobject::MyObject and and export in the bridge is expected. ### Examples - `#[qproperty(TYPE, NAME, read)]` A read only prop - `#[qproperty(TYPE, NAME, read = myGetter, write, notify)]` custom getter provided but will generate setter and on-changed - - `#[qproperty(TYPE, NAME)]` is syntactic sugar for `#[qproperty(TYPE, NAME, read, write, notify)]` -- `#[qproperty(TYPE, NAME, write)]` is an error as read was not passed +- `#[qproperty(TYPE, NAME, write)]` is an error as read was not explicitly passed ## Methods diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index 9f708737e..03068c723 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -212,12 +212,6 @@ mod tests { assert_eq!(property.ident, format_ident!("name")); assert_eq!(property.ty, parse_quote! { T }); - // Can use assert_matches! when https://github.com/rust-lang/rust/issues/82775 gets stabilised - // let expected_read = format_ident!("my_getter"); - // assert!(matches!( - // property.flags.read, - // FlagState::Custom(ident) if ident == expected_read - // )); assert_eq!( property.flags.read, FlagState::Custom(format_ident!("my_getter")) @@ -227,11 +221,6 @@ mod tests { assert!(property.flags.notify.is_some()); - // let expected_notify = format_ident!("my_notifier"); - // assert!(matches!( - // property.flags.notify, - // Some(FlagState::Custom(ident)) if ident == expected_notify - // )); assert_eq!( property.flags.notify, Some(FlagState::Custom(format_ident!("my_notifier"))) From ffccc51e3dd2c80a59682c22487bc196ba434bfe Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Mon, 22 Jul 2024 11:40:02 +0100 Subject: [PATCH 17/18] Fix typo in docs --- book/src/bridge/extern_rustqt.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/book/src/bridge/extern_rustqt.md b/book/src/bridge/extern_rustqt.md index 92c8398f5..5e30e268a 100644 --- a/book/src/bridge/extern_rustqt.md +++ b/book/src/bridge/extern_rustqt.md @@ -138,7 +138,7 @@ It is also possible to specify custom getters, setters and notify signals, using It is also possible to use any combination of custom functions or omit them entirely, but if flags are specified, read must be included as all properties need to be able to be read. -Using the read flag will cause cxx_qt to generate a getter function with an automatic name based off the property. e.g. `#[qproperty(i32, num, read)]` will have a getter function generated called get_num in Rust, and getNum in C++. +Using the read flag will cause CXX-Qt to generate a getter function with an automatic name based off the property. e.g. `#[qproperty(i32, num, read)]` will have a getter function generated called get_num in Rust, and getNum in C++. If a custom function is specified, an implementation both in qobject::MyObject and and export in the bridge is expected. From 5f4917f459f07121c63f078df7552901f11a39b9 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Mon, 22 Jul 2024 14:07:45 +0100 Subject: [PATCH 18/18] Fix asserts in tests --- crates/cxx-qt-gen/src/parser/property.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/crates/cxx-qt-gen/src/parser/property.rs b/crates/cxx-qt-gen/src/parser/property.rs index 03068c723..a1137c114 100644 --- a/crates/cxx-qt-gen/src/parser/property.rs +++ b/crates/cxx-qt-gen/src/parser/property.rs @@ -193,13 +193,10 @@ mod tests { assert_eq!(property.ident, format_ident!("name")); assert_eq!(property.ty, parse_quote! { T }); - assert!(property.flags.read.is_auto()); + assert_eq!(property.flags.read, FlagState::Auto); - assert!(property.flags.write.is_some()); - assert!(property.flags.notify.is_some()); - - assert!(property.flags.write.unwrap().is_auto()); - assert!(property.flags.notify.unwrap().is_auto()); + assert_eq!(property.flags.notify, Some(FlagState::Auto)); + assert_eq!(property.flags.write, Some(FlagState::Auto)); } #[test] @@ -219,8 +216,6 @@ mod tests { assert_eq!(property.flags.write, Some(FlagState::Auto)); - assert!(property.flags.notify.is_some()); - assert_eq!( property.flags.notify, Some(FlagState::Custom(format_ident!("my_notifier")))