cxx-qt-gen: split locking generation and add unsafeRustLock method

This simplifies the C++ source files as they can use the helper method.

This allows later for a templated signal connect that can use the
public helper method to lock the QObject.

Related to KDAB#577

crates/cxx-qt-gen/src/generator/cpp/constructor.rs

@@ -143,7 +143,6 @@ mod tests {
             namespace_internals: "rust".to_string(),
             base_class: "BaseClass".to_string(),
             blocks: GeneratedCppQObjectBlocks::default(),
-            locking: true,
         }
     }
 

crates/cxx-qt-gen/src/generator/cpp/locking.rs

@@ -0,0 +1,97 @@
+// SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company <[email protected]>
+// SPDX-FileContributor: Andrew Hayzen <[email protected]>
+//
+// SPDX-License-Identifier: MIT OR Apache-2.0
+
+use crate::generator::{
+    cpp::{fragment::CppFragment, qobject::GeneratedCppQObjectBlocks},
+    naming::qobject::QObjectName,
+};
+use indoc::formatdoc;
+use syn::Result;
+
+pub fn generate(qobject_idents: &QObjectName) -> Result<(String, GeneratedCppQObjectBlocks)> {
+    let mut result = GeneratedCppQObjectBlocks::default();
+
+    let lock_guard_mutex = "::std::lock_guard<::std::recursive_mutex>";
+    let qobject_ident = qobject_idents.cpp_class.cpp.to_string();
+
+    result.includes.insert("#include <mutex>".to_owned());
+
+    result.private_methods.push(CppFragment::Pair {
+        header: format!("[[nodiscard]] {lock_guard_mutex} unsafeRustLock() const;"),
+        source: formatdoc! {
+            r#"
+            {lock_guard_mutex}
+            {qobject_ident}::unsafeRustLock() const
+            {{
+              return {lock_guard_mutex}(*m_rustObjMutex);
+            }}
+            "#
+        },
+    });
+
+    result
+        .members
+        .push("::std::shared_ptr<::std::recursive_mutex> m_rustObjMutex;".to_owned());
+
+    let member_initializer =
+        "m_rustObjMutex(::std::make_shared<::std::recursive_mutex>())".to_owned();
+
+    Ok((member_initializer, result))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use crate::generator::naming::qobject::tests::create_qobjectname;
+    use indoc::indoc;
+    use pretty_assertions::assert_str_eq;
+
+    #[test]
+    fn test_generate_cpp_locking() {
+        let qobject_idents = create_qobjectname();
+
+        let (initializer, generated) = generate(&qobject_idents).unwrap();
+
+        // includes
+        assert_eq!(generated.includes.len(), 1);
+        assert!(generated.includes.contains("#include <mutex>"));
+
+        // members
+        assert_eq!(generated.members.len(), 1);
+        assert_str_eq!(
+            &generated.members[0],
+            "::std::shared_ptr<::std::recursive_mutex> m_rustObjMutex;"
+        );
+        assert_str_eq!(
+            initializer,
+            "m_rustObjMutex(::std::make_shared<::std::recursive_mutex>())"
+        );
+
+        // private methods
+        assert_eq!(generated.private_methods.len(), 1);
+
+        let (header, source) =
+            if let CppFragment::Pair { header, source } = &generated.private_methods[0] {
+                (header, source)
+            } else {
+                panic!("Expected pair")
+            };
+        assert_str_eq!(
+            header,
+            "[[nodiscard]] ::std::lock_guard<::std::recursive_mutex> unsafeRustLock() const;"
+        );
+        assert_str_eq!(
+            source,
+            indoc! {r#"
+            ::std::lock_guard<::std::recursive_mutex>
+            MyObject::unsafeRustLock() const
+            {
+              return ::std::lock_guard<::std::recursive_mutex>(*m_rustObjMutex);
+            }
+            "#}
+        );
+    }
+}

crates/cxx-qt-gen/src/generator/cpp/mod.rs

@@ -6,6 +6,7 @@
 mod constructor;
 pub mod fragment;
 pub mod inherit;
+pub mod locking;
 pub mod method;
 pub mod property;
 pub mod qobject;

crates/cxx-qt-gen/src/generator/cpp/qobject.rs

@@ -5,12 +5,13 @@
 
 use crate::generator::{
     cpp::{
-        constructor, fragment::CppFragment, inherit, method::generate_cpp_methods,
+        constructor, fragment::CppFragment, inherit, locking, method::generate_cpp_methods,
         property::generate_cpp_properties, signal::generate_cpp_signals, threading,
     },
     naming::{namespace::NamespaceName, qobject::QObjectName},
 };
 use crate::parser::{cxxqtdata::ParsedCxxMappings, qobject::ParsedQObject};
+use std::collections::BTreeSet;
 use syn::Result;
 
 #[derive(Default)]
@@ -27,6 +28,8 @@ pub struct GeneratedCppQObjectBlocks {
     pub members: Vec<String>,
     /// List of deconstructor source
     pub deconstructors: Vec<String>,
+    /// List of includes
+    pub includes: BTreeSet<String>,
 }
 
 impl GeneratedCppQObjectBlocks {
@@ -37,6 +40,7 @@ impl GeneratedCppQObjectBlocks {
         self.private_methods.append(&mut other.private_methods);
         self.members.append(&mut other.members);
         self.deconstructors.append(&mut other.deconstructors);
+        self.includes.append(&mut other.includes);
     }
 
     pub fn from(qobject: &ParsedQObject) -> GeneratedCppQObjectBlocks {
@@ -77,8 +81,6 @@ pub struct GeneratedCppQObject {
     pub base_class: String,
     /// The blocks of the QObject
     pub blocks: GeneratedCppQObjectBlocks,
-    /// Whether locking is enabled
-    pub locking: bool,
 }
 
 impl GeneratedCppQObject {
@@ -98,10 +100,9 @@ impl GeneratedCppQObject {
                 .clone()
                 .unwrap_or_else(|| "QObject".to_string()),
             blocks: GeneratedCppQObjectBlocks::from(qobject),
-            locking: qobject.locking,
         };
         let lock_guard = if qobject.locking {
-            Some("const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex);")
+            Some("const auto guard = unsafeRustLock();")
         } else {
             None
         };
@@ -131,11 +132,15 @@ impl GeneratedCppQObject {
             cxx_mappings,
         )?);
 
-        let mut member_initializers = if qobject.locking {
-            vec!["m_rustObjMutex(::std::make_shared<::std::recursive_mutex>())".to_string()]
-        } else {
-            vec![]
-        };
+        let mut member_initializers = vec![];
+
+        // If this type has locking enabled then add generation
+        if qobject.locking {
+            let (initializer, mut blocks) = locking::generate(&qobject_idents)?;
+            generated.blocks.append(&mut blocks);
+            member_initializers.push(initializer);
+        }
+
         // If this type has threading enabled then add generation
         if qobject.threading {
             let (initializer, mut blocks) = threading::generate(&qobject_idents)?;

crates/cxx-qt-gen/src/writer/cpp/header.rs

@@ -3,6 +3,8 @@
 //
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
+use std::collections::BTreeSet;
+
 use crate::generator::cpp::{fragment::CppFragment, GeneratedCppBlocks};
 use crate::writer::cpp::namespace_pair;
 use indoc::formatdoc;
@@ -101,12 +103,6 @@ fn qobjects_header(generated: &GeneratedCppBlocks) -> Vec<String> {
                 format!("::rust::Box<{rust_ident}> m_rustObj;", rust_ident = qobject.rust_ident),
             ];
 
-            if qobject.locking {
-                members.extend(vec![
-                    "::std::shared_ptr<::std::recursive_mutex> m_rustObjMutex;".to_string(),
-                ]);
-            }
-
             members.extend(qobject.blocks.members.iter().cloned());
             members.join("\n  ")
         },
@@ -123,12 +119,11 @@ fn qobjects_header(generated: &GeneratedCppBlocks) -> Vec<String> {
 pub fn write_cpp_header(generated: &GeneratedCppBlocks) -> String {
     // Headers included:
     // <memory> - unique_ptr to the Rust object.
-    // <mutex> - used for mutex locking the rust object.
     formatdoc! {r#"
         #pragma once
 
         #include <memory>
-        #include <mutex>
+        {includes}
 
         namespace rust::cxxqtlib1 {{
         template<typename T>
@@ -143,6 +138,11 @@ pub fn write_cpp_header(generated: &GeneratedCppBlocks) -> String {
     cxx_file_stem = generated.cxx_file_stem,
     forward_declare = forward_declare(generated).join("\n"),
     qobjects = qobjects_header(generated).join("\n"),
+    includes = generated.qobjects.iter()
+    .fold(BTreeSet::<&String>::default(), |mut acc, qobject| {
+        acc.extend(qobject.blocks.includes.iter());
+        acc
+    }).into_iter().cloned().collect::<Vec<String>>().join("\n"),
     }
 }
 

crates/cxx-qt-gen/src/writer/cpp/mod.rs

@@ -42,6 +42,8 @@ pub fn write_cpp(generated: &GeneratedCppBlocks) -> CppFragment {
 
 #[cfg(test)]
 mod tests {
+    use std::collections::BTreeSet;
+
     use super::*;
 
     use crate::generator::cpp::qobject::{GeneratedCppQObject, GeneratedCppQObjectBlocks};
@@ -59,11 +61,15 @@ mod tests {
                     rust_ident: "MyObjectRust".to_owned(),
                     namespace_internals: "cxx_qt::my_object::cxx_qt_my_object".to_owned(),
                     base_class: "QStringListModel".to_owned(),
-                    locking: true,
                     blocks: GeneratedCppQObjectBlocks {
                         deconstructors: vec![],
                         forward_declares: vec![],
                         members: vec![],
+                        includes: {
+                          let mut includes = BTreeSet::<String>::default();
+                          includes.insert("#include <test>".to_owned());
+                          includes
+                        },
                         metaobjects: vec![
                             "Q_PROPERTY(int count READ count WRITE setCount NOTIFY countChanged)".to_owned(),
                             "Q_PROPERTY(bool longPropertyNameThatWrapsInClangFormat READ getToggle WRITE setToggle NOTIFY toggleChanged)"
@@ -177,11 +183,15 @@ mod tests {
                     rust_ident: "FirstObjectRust".to_owned(),
                     namespace_internals: "cxx_qt::cxx_qt_first_object".to_owned(),
                     base_class: "QStringListModel".to_owned(),
-                    locking: true,
                     blocks: GeneratedCppQObjectBlocks {
                         deconstructors: vec![],
                         forward_declares: vec![],
                         members: vec![],
+                        includes: {
+                          let mut includes = BTreeSet::<String>::default();
+                          includes.insert("#include <test>".to_owned());
+                          includes
+                        },
                         metaobjects: vec![
                             "Q_PROPERTY(int longPropertyNameThatWrapsInClangFormat READ count WRITE setCount NOTIFY countChanged)"
                                 .to_owned(),
@@ -218,11 +228,15 @@ mod tests {
                     rust_ident: "SecondObjectRust".to_owned(),
                     namespace_internals: "cxx_qt::cxx_qt_second_object".to_owned(),
                     base_class: "QStringListModel".to_owned(),
-                    locking: false,
                     blocks: GeneratedCppQObjectBlocks {
                         deconstructors: vec![],
                         forward_declares: vec![],
                         members: vec![],
+                        includes: {
+                          let mut includes = BTreeSet::<String>::default();
+                          includes.insert("#include <test>".to_owned());
+                          includes
+                        },
                         metaobjects: vec![
                             "Q_PROPERTY(int count READ count WRITE setCount NOTIFY countChanged)"
                                 .to_owned(),
@@ -280,7 +294,7 @@ mod tests {
         #pragma once
 
         #include <memory>
-        #include <mutex>
+        #include <test>
 
         namespace rust::cxxqtlib1 {
         template<typename T>
@@ -322,7 +336,6 @@ mod tests {
 
         private:
           ::rust::Box<MyObjectRust> m_rustObj;
-          ::std::shared_ptr<::std::recursive_mutex> m_rustObjMutex;
         };
 
         static_assert(::std::is_base_of<QObject, MyObject>::value, "MyObject must inherit from QObject");
@@ -339,7 +352,7 @@ mod tests {
         #pragma once
 
         #include <memory>
-        #include <mutex>
+        #include <test>
 
         namespace rust::cxxqtlib1 {
         template<typename T>
@@ -377,7 +390,6 @@ mod tests {
 
         private:
           ::rust::Box<FirstObjectRust> m_rustObj;
-          ::std::shared_ptr<::std::recursive_mutex> m_rustObjMutex;
         };
 
         static_assert(::std::is_base_of<QObject, FirstObject>::value, "FirstObject must inherit from QObject");
@@ -422,7 +434,7 @@ mod tests {
         #pragma once
 
         #include <memory>
-        #include <mutex>
+        #include <test>
 
         namespace rust::cxxqtlib1 {
         template<typename T>
@@ -464,7 +476,6 @@ mod tests {
 
         private:
           ::rust::Box<MyObjectRust> m_rustObj;
-          ::std::shared_ptr<::std::recursive_mutex> m_rustObjMutex;
         };
 
         static_assert(::std::is_base_of<QObject, MyObject>::value, "MyObject must inherit from QObject");

crates/cxx-qt-gen/test_outputs/inheritance.cpp

@@ -17,14 +17,14 @@ MyObject::unsafeRustMut()
 QVariant
 MyObject::data(QModelIndex const& _index, ::std::int32_t _role) const
 {
-  const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex);
+  const auto guard = unsafeRustLock();
   return dataWrapper(_index, _role);
 }
 
 bool
 MyObject::hasChildren(QModelIndex const& _parent) const
 {
-  const ::std::lock_guard<::std::recursive_mutex> guard(*m_rustObjMutex);
+  const auto guard = unsafeRustLock();
   return hasChildrenWrapper(_parent);
 }
 
@@ -34,3 +34,9 @@ MyObject::MyObject(QObject* parent)
   , m_rustObjMutex(::std::make_shared<::std::recursive_mutex>())
 {
 }
+
+::std::lock_guard<::std::recursive_mutex>
+MyObject::unsafeRustLock() const
+{
+  return ::std::lock_guard<::std::recursive_mutex>(*m_rustObjMutex);
+}

crates/cxx-qt-gen/test_outputs/inheritance.h

@@ -41,6 +41,8 @@ class MyObject : public QAbstractItemModel
   QVariant dataWrapper(QModelIndex const& _index,
                        ::std::int32_t _role) const noexcept;
   bool hasChildrenWrapper(QModelIndex const& _parent) const noexcept;
+  [[nodiscard]] ::std::lock_guard<::std::recursive_mutex> unsafeRustLock()
+    const;
 
 private:
   ::rust::Box<MyObjectRust> m_rustObj;