cxx-qt-gen: remove wrapper method for C++ -> Rust properties

This then avoids us needing to generate Rust methods with fully qualified types on the Rust side and removes a load of generation. Related to #404
KDAB · Jul 19, 2023 · 9602329 · 9602329
1 parent 0805af7
commit 9602329
Show file tree

Hide file tree

Showing 13 changed files with 193 additions and 218 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
@@ -27,13 +27,22 @@ pub fn generate(
             {qobject_ident}::{ident_getter}() const
             {{
                 {rust_obj_guard}
-                return m_rustObj->{ident_getter}(*this);
+                return {ident_getter_wrapper}();
             }}
             "#,
             return_cxx_ty = cxx_ty.as_cxx_ty(),
             ident_getter = idents.getter.cpp.to_string(),
+            ident_getter_wrapper = idents.getter_wrapper.cpp.to_string(),
             qobject_ident = qobject_ident,
             rust_obj_guard = lock_guard.unwrap_or_default(),
         ),
     }
 }
+
+pub fn generate_wrapper(idents: &QPropertyName, cxx_ty: &CppType) -> CppFragment {
+    CppFragment::Header(format!(
+        "{return_cxx_ty} const& {ident_getter_wrapper}() const noexcept;",
+        return_cxx_ty = cxx_ty.as_cxx_ty(),
+        ident_getter_wrapper = idents.getter_wrapper.cpp
+    ))
+}
diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs
@@ -37,12 +37,18 @@ pub fn generate_cpp_properties(
             &cxx_ty,
             lock_guard,
         ));
+        generated
+            .private_methods
+            .push(getter::generate_wrapper(&idents, &cxx_ty));
         generated.methods.push(setter::generate(
             &idents,
             &qobject_ident,
             &cxx_ty,
             lock_guard,
         ));
+        generated
+            .private_methods
+            .push(setter::generate_wrapper(&idents, &cxx_ty));
         signals.push(signal::generate(&idents, qobject_idents));
     }
 
@@ -109,7 +115,7 @@ mod tests {
             MyObject::getTrivialProperty() const
             {
                 // ::std::lock_guard
-                return m_rustObj->getTrivialProperty(*this);
+                return getTrivialPropertyWrapper();
             }
             "#}
         );
@@ -130,7 +136,7 @@ mod tests {
                 MyObject::setTrivialProperty(::std::int32_t const& value)
                 {
                     // ::std::lock_guard
-                    m_rustObj->setTrivialProperty(*this, value);
+                    setTrivialPropertyWrapper(value);
                 }
                 "#}
         );
@@ -151,7 +157,7 @@ mod tests {
             MyObject::getOpaqueProperty() const
             {
                 // ::std::lock_guard
-                return m_rustObj->getOpaqueProperty(*this);
+                return getOpaquePropertyWrapper();
             }
             "#}
         );
@@ -172,7 +178,7 @@ mod tests {
             MyObject::setOpaqueProperty(::std::unique_ptr<QColor> const& value)
             {
                 // ::std::lock_guard
-                m_rustObj->setOpaqueProperty(*this, value);
+                setOpaquePropertyWrapper(value);
             }
             "#}
         );
@@ -242,6 +248,48 @@ mod tests {
             }
             "#}
         );
+
+        // private methods
+        assert_eq!(generated.private_methods.len(), 4);
+        let header = if let CppFragment::Header(header) = &generated.private_methods[0] {
+            header
+        } else {
+            panic!("Expected header")
+        };
+        assert_str_eq!(
+            header,
+            "::std::int32_t const& getTrivialPropertyWrapper() const noexcept;"
+        );
+
+        let header = if let CppFragment::Header(header) = &generated.private_methods[1] {
+            header
+        } else {
+            panic!("Expected header")
+        };
+        assert_str_eq!(
+            header,
+            "void setTrivialPropertyWrapper(::std::int32_t value) noexcept;"
+        );
+
+        let header = if let CppFragment::Header(header) = &generated.private_methods[2] {
+            header
+        } else {
+            panic!("Expected header")
+        };
+        assert_str_eq!(
+            header,
+            "::std::unique_ptr<QColor> const& getOpaquePropertyWrapper() const noexcept;"
+        );
+
+        let header = if let CppFragment::Header(header) = &generated.private_methods[3] {
+            header
+        } else {
+            panic!("Expected header")
+        };
+        assert_str_eq!(
+            header,
+            "void setOpaquePropertyWrapper(::std::unique_ptr<QColor> value) noexcept;"
+        );
     }
 
     #[test]
@@ -284,7 +332,7 @@ mod tests {
             MyObject::getMappedProperty() const
             {
                 // ::std::lock_guard
-                return m_rustObj->getMappedProperty(*this);
+                return getMappedPropertyWrapper();
             }
             "#}
         );
@@ -302,7 +350,7 @@ mod tests {
                 MyObject::setMappedProperty(A1 const& value)
                 {
                     // ::std::lock_guard
-                    m_rustObj->setMappedProperty(*this, value);
+                    setMappedPropertyWrapper(value);
                 }
                 "#}
         );
@@ -338,5 +386,24 @@ mod tests {
             }
             "#}
         );
+
+        // private methods
+        assert_eq!(generated.private_methods.len(), 2);
+        let header = if let CppFragment::Header(header) = &generated.private_methods[0] {
+            header
+        } else {
+            panic!("Expected header")
+        };
+        assert_str_eq!(
+            header,
+            "A1 const& getMappedPropertyWrapper() const noexcept;"
+        );
+
+        let header = if let CppFragment::Header(header) = &generated.private_methods[1] {
+            header
+        } else {
+            panic!("Expected header")
+        };
+        assert_str_eq!(header, "void setMappedPropertyWrapper(A1 value) noexcept;");
     }
 }
diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs b/crates/cxx-qt-gen/src/generator/cpp/property/setter.rs
@@ -27,13 +27,24 @@ pub fn generate(
             {qobject_ident}::{ident_setter}({cxx_ty} const& value)
             {{
                 {rust_obj_guard}
-                m_rustObj->{ident_setter}(*this, value);
+                {ident_setter_wrapper}(value);
             }}
             "#,
             cxx_ty = cxx_ty.as_cxx_ty(),
             ident_setter = idents.setter.cpp,
+            ident_setter_wrapper = idents.setter_wrapper.cpp.to_string(),
             qobject_ident = qobject_ident,
             rust_obj_guard = lock_guard.unwrap_or_default(),
         },
     }
 }
+
+pub fn generate_wrapper(idents: &QPropertyName, cxx_ty: &CppType) -> 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;",
+        cxx_ty = cxx_ty.as_cxx_ty(),
+        ident_setter_wrapper = idents.setter_wrapper.cpp
+    ))
+}
diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs
@@ -12,16 +12,22 @@ use syn::Ident;
 pub struct QPropertyName {
     pub name: CombinedIdent,
     pub getter: CombinedIdent,
+    pub getter_wrapper: CombinedIdent,
     pub setter: CombinedIdent,
+    pub setter_wrapper: CombinedIdent,
     pub notify: CombinedIdent,
 }
 
 impl From<&Ident> for QPropertyName {
     fn from(ident: &Ident) -> Self {
+        let getter = CombinedIdent::getter_from_property(ident.clone());
+        let setter = CombinedIdent::setter_from_property(ident);
         Self {
             name: CombinedIdent::from_property(ident.clone()),
-            getter: CombinedIdent::getter_from_property(ident.clone()),
-            setter: CombinedIdent::setter_from_property(ident),
+            getter_wrapper: CombinedIdent::wrapper_from_combined_property(&getter),
+            getter,
+            setter_wrapper: CombinedIdent::wrapper_from_combined_property(&setter),
+            setter,
             notify: CombinedIdent::notify_from_property(ident),
         }
     }
@@ -67,6 +73,14 @@ impl CombinedIdent {
             rust: ident,
         }
     }
+
+    /// For a given ident generate the Rust and C++ wrapper names
+    fn wrapper_from_combined_property(ident: &CombinedIdent) -> Self {
+        Self {
+            cpp: format_ident!("{}Wrapper", ident.cpp),
+            rust: format_ident!("{}_wrapper", ident.rust),
+        }
+    }
 }
 
 #[cfg(test)]

diff --git a/crates/cxx-qt-gen/src/generator/rust/property/getter.rs b/crates/cxx-qt-gen/src/generator/rust/property/getter.rs
@@ -16,37 +16,26 @@ pub fn generate(
     ty: &Type,
 ) -> RustFragmentPair {
     let cpp_class_name_rust = &qobject_idents.cpp_class.rust;
-    let rust_struct_name_rust = &qobject_idents.rust_struct.rust;
-    let getter_cpp = idents.getter.cpp.to_string();
+    let getter_wrapper_cpp = idents.getter_wrapper.cpp.to_string();
     let getter_rust = &idents.getter.rust;
     let ident = &idents.name.rust;
     let ident_str = ident.to_string();
 
     RustFragmentPair {
         cxx_bridge: vec![quote! {
             extern "Rust" {
-                #[cxx_name = #getter_cpp]
-                unsafe fn #getter_rust<'a>(self: &'a #rust_struct_name_rust, cpp: &'a #cpp_class_name_rust) -> &'a #ty;
+                #[cxx_name = #getter_wrapper_cpp]
+                unsafe fn #getter_rust<'a>(self: &'a #cpp_class_name_rust) -> &'a #ty;
             }
         }],
-        implementation: vec![
-            quote! {
-                impl #rust_struct_name_rust {
-                    #[doc(hidden)]
-                    pub fn #getter_rust<'a>(&'a self, cpp: &'a #cpp_class_name_rust) -> &'a #ty {
-                        cpp.#getter_rust()
-                    }
+        implementation: vec![quote! {
+            impl #cpp_class_name_rust {
+                #[doc = "Getter for the Q_PROPERTY "]
+                #[doc = #ident_str]
+                pub fn #getter_rust(&self) -> &#ty {
+                    &self.#ident
                 }
-            },
-            quote! {
-                impl #cpp_class_name_rust {
-                    #[doc = "Getter for the Q_PROPERTY "]
-                    #[doc = #ident_str]
-                    pub fn #getter_rust(&self) -> &#ty {
-                        &self.#ident
-                    }
-                }
-            },
-        ],
+            }
+        }],
     }
 }