From 6611f4fa8fc9efab3512b79de0852dda57850268 Mon Sep 17 00:00:00 2001 From: Andrew Hayzen Date: Tue, 13 Jun 2023 15:14:07 +0100 Subject: [PATCH] cxx-qt-gen: remove mutable getter for properties As rust_mut is now safe. Related to #559 --- CHANGELOG.md | 1 + .../src/generator/naming/property.rs | 12 --- .../src/generator/rust/property/getter.rs | 16 ---- .../src/generator/rust/property/mod.rs | 79 ++++--------------- .../test_outputs/passthrough_and_naming.rs | 24 ------ crates/cxx-qt-gen/test_outputs/properties.rs | 24 ------ examples/qml_features/rust/src/containers.rs | 8 +- 7 files changed, 18 insertions(+), 146 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f90fa3ab2..a9be4b509 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Removed support for `cxx_type` and `cxx_return_type` and related conversion methods. - Removed `newCppObject` function that allowed creation of default-constructed QObject from Rust. - Generation of getter and setter for private Rust fields +- Generation of mutable getter for properties, instead use `rust_mut` ## [0.5.3](https://github.com/KDAB/cxx-qt/compare/v0.5.2...v0.5.3) - 2023-05-19 diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs index ae74d9fbc..9f88882f7 100644 --- a/crates/cxx-qt-gen/src/generator/naming/property.rs +++ b/crates/cxx-qt-gen/src/generator/naming/property.rs @@ -12,7 +12,6 @@ use syn::Ident; pub struct QPropertyName { pub name: CombinedIdent, pub getter: CombinedIdent, - pub getter_mutable: CombinedIdent, pub setter: CombinedIdent, pub notify: CombinedIdent, } @@ -22,7 +21,6 @@ impl From<&Ident> for QPropertyName { Self { name: CombinedIdent::from_property(ident.clone()), getter: CombinedIdent::getter_from_property(ident.clone()), - getter_mutable: CombinedIdent::getter_mutable_from_property(ident), setter: CombinedIdent::setter_from_property(ident), notify: CombinedIdent::notify_from_property(ident), } @@ -44,14 +42,6 @@ impl CombinedIdent { } } - /// For a given ident generate the Rust and C++ getter mutable names - fn getter_mutable_from_property(ident: &Ident) -> Self { - Self { - cpp: format_ident!("get{}Mut", ident.to_string().to_case(Case::Pascal)), - rust: format_ident!("{ident}_mut"), - } - } - /// For a given ident generate the Rust and C++ names fn from_property(ident: Ident) -> Self { Self { @@ -102,8 +92,6 @@ pub mod tests { assert_eq!(names.name.rust, format_ident!("my_property")); assert_eq!(names.getter.cpp, format_ident!("getMyProperty")); assert_eq!(names.getter.rust, format_ident!("my_property")); - assert_eq!(names.getter_mutable.cpp, format_ident!("getMyPropertyMut")); - assert_eq!(names.getter_mutable.rust, format_ident!("my_property_mut")); assert_eq!(names.setter.cpp, format_ident!("setMyProperty")); assert_eq!(names.setter.rust, format_ident!("set_my_property")); assert_eq!(names.notify.cpp, format_ident!("myPropertyChanged")); 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 795865e92..8217e4d97 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/getter.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/getter.rs @@ -19,10 +19,8 @@ pub fn generate( let rust_struct_name_rust = &qobject_idents.rust_struct.rust; let getter_cpp = idents.getter.cpp.to_string(); let getter_rust = &idents.getter.rust; - let getter_mutable_rust = &idents.getter_mutable.rust; let ident = &idents.name.rust; let ident_str = ident.to_string(); - let notify_ident_str = idents.notify.rust.to_string(); RustFragmentPair { cxx_bridge: vec![quote! { @@ -49,20 +47,6 @@ pub fn generate( } } }, - quote! { - impl #cpp_class_name_rust { - #[doc = "unsafe getter for the Q_PROPERTY "] - #[doc = #ident_str] - #[doc = "\n"] - #[doc = "This allows for modifying the Q_PROPERTY without calling the property changed Q_SIGNAL"] - #[doc = "\n"] - #[doc = "After modifying the property, make sure to call the corresponding changed signal: "] - #[doc = #notify_ident_str] - pub unsafe fn #getter_mutable_rust<'a>(self: Pin<&'a mut Self>) -> &'a mut #ty { - &mut self.rust_mut().get_unchecked_mut().#ident - } - } - }, ], } } 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 010e4bd68..360a5da3f 100644 --- a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs +++ b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs @@ -88,7 +88,7 @@ mod tests { // Check that we have the expected number of blocks assert_eq!(generated.cxx_mod_contents.len(), 12); - assert_eq!(generated.cxx_qt_mod_contents.len(), 18); + assert_eq!(generated.cxx_qt_mod_contents.len(), 15); // Trivial Property @@ -125,23 +125,6 @@ mod tests { } }, ); - assert_tokens_eq( - &generated.cxx_qt_mod_contents[2], - parse_quote! { - impl MyObjectQt { - #[doc = "unsafe getter for the Q_PROPERTY "] - #[doc = "trivial_property"] - #[doc = "\n"] - #[doc = "This allows for modifying the Q_PROPERTY without calling the property changed Q_SIGNAL"] - #[doc = "\n"] - #[doc = "After modifying the property, make sure to call the corresponding changed signal: "] - #[doc = "trivial_property_changed"] - pub unsafe fn trivial_property_mut<'a>(self: Pin<&'a mut Self>) -> &'a mut i32 { - &mut self.rust_mut().get_unchecked_mut().trivial_property - } - } - }, - ); // Setters assert_tokens_eq( @@ -154,7 +137,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[3], + &generated.cxx_qt_mod_contents[2], parse_quote! { impl MyObject { #[doc(hidden)] @@ -165,7 +148,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[4], + &generated.cxx_qt_mod_contents[3], parse_quote! { impl MyObjectQt { #[doc = "Setter for the Q_PROPERTY "] @@ -194,7 +177,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[5], + &generated.cxx_qt_mod_contents[4], parse_quote! { impl MyObject { #[doc(hidden)] @@ -205,7 +188,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[6], + &generated.cxx_qt_mod_contents[5], parse_quote! { impl MyObjectQt { #[doc = "Getter for the Q_PROPERTY "] @@ -216,23 +199,6 @@ mod tests { } }, ); - assert_tokens_eq( - &generated.cxx_qt_mod_contents[7], - parse_quote! { - impl MyObjectQt { - #[doc = "unsafe getter for the Q_PROPERTY "] - #[doc = "opaque_property"] - #[doc = "\n"] - #[doc = "This allows for modifying the Q_PROPERTY without calling the property changed Q_SIGNAL"] - #[doc = "\n"] - #[doc = "After modifying the property, make sure to call the corresponding changed signal: "] - #[doc = "opaque_property_changed"] - pub unsafe fn opaque_property_mut<'a>(self: Pin<&'a mut Self>) -> &'a mut UniquePtr { - &mut self.rust_mut().get_unchecked_mut().opaque_property - } - } - }, - ); // Setters assert_tokens_eq( @@ -245,7 +211,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[8], + &generated.cxx_qt_mod_contents[6], parse_quote! { impl MyObject { #[doc(hidden)] @@ -256,7 +222,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[9], + &generated.cxx_qt_mod_contents[7], parse_quote! { impl MyObjectQt { #[doc = "Setter for the Q_PROPERTY "] @@ -285,7 +251,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[10], + &generated.cxx_qt_mod_contents[8], parse_quote! { impl MyObject { #[doc(hidden)] @@ -296,7 +262,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[11], + &generated.cxx_qt_mod_contents[9], parse_quote! { impl MyObjectQt { #[doc = "Getter for the Q_PROPERTY "] @@ -307,23 +273,6 @@ mod tests { } }, ); - assert_tokens_eq( - &generated.cxx_qt_mod_contents[12], - parse_quote! { - impl MyObjectQt { - #[doc = "unsafe getter for the Q_PROPERTY "] - #[doc = "unsafe_property"] - #[doc = "\n"] - #[doc = "This allows for modifying the Q_PROPERTY without calling the property changed Q_SIGNAL"] - #[doc = "\n"] - #[doc = "After modifying the property, make sure to call the corresponding changed signal: "] - #[doc = "unsafe_property_changed"] - pub unsafe fn unsafe_property_mut<'a>(self: Pin<&'a mut Self>) -> &'a mut *mut T { - &mut self.rust_mut().get_unchecked_mut().unsafe_property - } - } - }, - ); // Setters assert_tokens_eq( @@ -336,7 +285,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[13], + &generated.cxx_qt_mod_contents[10], parse_quote! { impl MyObject { #[doc(hidden)] @@ -347,7 +296,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[14], + &generated.cxx_qt_mod_contents[11], parse_quote! { impl MyObjectQt { #[doc = "Setter for the Q_PROPERTY "] @@ -389,7 +338,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[15], + &generated.cxx_qt_mod_contents[12], parse_quote! { impl MyObjectQt { #[doc = "Connect the given function pointer to the signal "] @@ -430,7 +379,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[16], + &generated.cxx_qt_mod_contents[13], parse_quote! { impl MyObjectQt { #[doc = "Connect the given function pointer to the signal "] @@ -471,7 +420,7 @@ mod tests { }, ); assert_tokens_eq( - &generated.cxx_qt_mod_contents[17], + &generated.cxx_qt_mod_contents[14], parse_quote! { impl MyObjectQt { #[doc = "Connect the given function pointer to the signal "] diff --git a/crates/cxx-qt-gen/test_outputs/passthrough_and_naming.rs b/crates/cxx-qt-gen/test_outputs/passthrough_and_naming.rs index f55e27ef7..f31b65c9b 100644 --- a/crates/cxx-qt-gen/test_outputs/passthrough_and_naming.rs +++ b/crates/cxx-qt-gen/test_outputs/passthrough_and_naming.rs @@ -262,18 +262,6 @@ pub mod cxx_qt_ffi { &self.rust().property_name } } - impl MyObjectQt { - #[doc = "unsafe getter for the Q_PROPERTY "] - #[doc = "property_name"] - #[doc = "\n"] - #[doc = "This allows for modifying the Q_PROPERTY without calling the property changed Q_SIGNAL"] - #[doc = "\n"] - #[doc = "After modifying the property, make sure to call the corresponding changed signal: "] - #[doc = "property_name_changed"] - pub unsafe fn property_name_mut<'a>(self: Pin<&'a mut Self>) -> &'a mut i32 { - &mut self.rust_mut().get_unchecked_mut().property_name - } - } impl MyObject { #[doc(hidden)] pub fn set_property_name(&mut self, cpp: Pin<&mut MyObjectQt>, value: i32) { @@ -354,18 +342,6 @@ pub mod cxx_qt_ffi { &self.rust().property_name } } - impl SecondObjectQt { - #[doc = "unsafe getter for the Q_PROPERTY "] - #[doc = "property_name"] - #[doc = "\n"] - #[doc = "This allows for modifying the Q_PROPERTY without calling the property changed Q_SIGNAL"] - #[doc = "\n"] - #[doc = "After modifying the property, make sure to call the corresponding changed signal: "] - #[doc = "property_name_changed"] - pub unsafe fn property_name_mut<'a>(self: Pin<&'a mut Self>) -> &'a mut i32 { - &mut self.rust_mut().get_unchecked_mut().property_name - } - } impl SecondObject { #[doc(hidden)] pub fn set_property_name(&mut self, cpp: Pin<&mut SecondObjectQt>, value: i32) { diff --git a/crates/cxx-qt-gen/test_outputs/properties.rs b/crates/cxx-qt-gen/test_outputs/properties.rs index bbcdd8723..cac8ed76e 100644 --- a/crates/cxx-qt-gen/test_outputs/properties.rs +++ b/crates/cxx-qt-gen/test_outputs/properties.rs @@ -130,18 +130,6 @@ pub mod cxx_qt_ffi { &self.rust().primitive } } - impl MyObjectQt { - #[doc = "unsafe getter for the Q_PROPERTY "] - #[doc = "primitive"] - #[doc = "\n"] - #[doc = "This allows for modifying the Q_PROPERTY without calling the property changed Q_SIGNAL"] - #[doc = "\n"] - #[doc = "After modifying the property, make sure to call the corresponding changed signal: "] - #[doc = "primitive_changed"] - pub unsafe fn primitive_mut<'a>(self: Pin<&'a mut Self>) -> &'a mut i32 { - &mut self.rust_mut().get_unchecked_mut().primitive - } - } impl MyObject { #[doc(hidden)] pub fn set_primitive(&mut self, cpp: Pin<&mut MyObjectQt>, value: i32) { @@ -172,18 +160,6 @@ pub mod cxx_qt_ffi { &self.rust().trivial } } - impl MyObjectQt { - #[doc = "unsafe getter for the Q_PROPERTY "] - #[doc = "trivial"] - #[doc = "\n"] - #[doc = "This allows for modifying the Q_PROPERTY without calling the property changed Q_SIGNAL"] - #[doc = "\n"] - #[doc = "After modifying the property, make sure to call the corresponding changed signal: "] - #[doc = "trivial_changed"] - pub unsafe fn trivial_mut<'a>(self: Pin<&'a mut Self>) -> &'a mut QPoint { - &mut self.rust_mut().get_unchecked_mut().trivial - } - } impl MyObject { #[doc(hidden)] pub fn set_trivial(&mut self, cpp: Pin<&mut MyObjectQt>, value: QPoint) { diff --git a/examples/qml_features/rust/src/containers.rs b/examples/qml_features/rust/src/containers.rs index c692031a6..0dc469255 100644 --- a/examples/qml_features/rust/src/containers.rs +++ b/examples/qml_features/rust/src/containers.rs @@ -142,11 +142,9 @@ impl ffi::RustContainersQt { /// Insert the given string and variant to the map container fn insert_map(mut self: Pin<&mut Self>, key: QString, value: QVariant) { - // SAFETY: map is a Q_PROPERTY so ensure we manually trigger changed - unsafe { - self.as_mut().map_mut().insert(key, value); - self.as_mut().map_changed(); - } + // Note: map is a Q_PROPERTY so ensure we manually trigger changed + self.as_mut().rust_mut().map.insert(key, value); + self.as_mut().map_changed(); self.update_strings(); }