WIP: cxx-qt-gen: move signals away from an enum to an extern "C++" block

Related to KDAB#557

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

@@ -4,7 +4,7 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
 use crate::generator::{
-    cpp::{qobject::GeneratedCppQObjectBlocks, types::CppType},
+    cpp::{qobject::GeneratedCppQObjectBlocks, signal::generate_cpp_signals, types::CppType},
     naming::{property::QPropertyName, qobject::QObjectName},
 };
 use crate::parser::{cxxqtdata::ParsedCxxMappings, property::ParsedQProperty};
@@ -21,7 +21,9 @@ pub fn generate_cpp_properties(
     cxx_mappings: &ParsedCxxMappings,
 ) -> Result<GeneratedCppQObjectBlocks> {
     let mut generated = GeneratedCppQObjectBlocks::default();
+    let mut signals = vec![];
     let qobject_ident = qobject_idents.cpp_class.cpp.to_string();
+
     for property in properties {
         // Cache the idents as they are used in multiple places
         let idents = QPropertyName::from(property);
@@ -34,9 +36,15 @@ pub fn generate_cpp_properties(
         generated
             .methods
             .push(setter::generate(&idents, &qobject_ident, &cxx_ty));
-        generated.methods.push(signal::generate(&idents));
+        signals.push(signal::generate(&idents, qobject_idents));
     }
 
+    generated.append(&mut generate_cpp_signals(
+        &signals,
+        qobject_idents,
+        cxx_mappings,
+    )?);
+
     Ok(generated)
 }
 

crates/cxx-qt-gen/src/generator/cpp/property/signal.rs

@@ -3,12 +3,27 @@
 //
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
-use crate::generator::naming::property::QPropertyName;
-use crate::CppFragment;
+use syn::ForeignItemFn;
 
-pub fn generate(idents: &QPropertyName) -> CppFragment {
-    CppFragment::Header(format!(
-        "Q_SIGNAL void {ident_notify}();",
-        ident_notify = idents.notify.cpp
-    ))
+use crate::{
+    generator::naming::{property::QPropertyName, qobject::QObjectName},
+    parser::signals::ParsedSignal,
+};
+
+pub fn generate(idents: &QPropertyName, qobject_idents: &QObjectName) -> ParsedSignal {
+    // 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.cpp_class.rust;
+    let notify_cpp = &idents.notify.cpp;
+    let notify_rust_str = idents.notify.rust.to_string();
+    let method: ForeignItemFn = syn::parse_quote! {
+        #[doc = "Notify for the Q_PROPERTY"]
+        #[rust_name = #notify_rust_str]
+        fn #notify_cpp(self: Pin<&mut #cpp_class_rust>);
+    };
+    ParsedSignal::from_property_method(
+        method,
+        idents.notify.clone(),
+        qobject_idents.cpp_class.rust.clone(),
+    )
 }

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

@@ -106,13 +106,11 @@ impl GeneratedCppQObject {
             &qobject_idents,
             cxx_mappings,
         )?);
-        if let Some(signals_enum) = &qobject.signals {
-            generated.blocks.append(&mut generate_cpp_signals(
-                &signals_enum.signals,
-                &qobject_idents,
-                cxx_mappings,
-            )?);
-        }
+        generated.blocks.append(&mut generate_cpp_signals(
+            &qobject.signals,
+            &qobject_idents,
+            cxx_mappings,
+        )?);
         generated.blocks.append(&mut inherit::generate(
             &qobject.inherited_methods,
             &qobject.base_class,

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

@@ -118,7 +118,7 @@ pub fn generate_cpp_signals(
 mod tests {
     use super::*;
 
-    use crate::generator::naming::qobject::tests::create_qobjectname;
+    use crate::generator::naming::{qobject::tests::create_qobjectname, CombinedIdent};
     use crate::parser::parameter::ParsedFunctionParameter;
     use indoc::indoc;
     use pretty_assertions::assert_str_eq;
@@ -128,9 +128,11 @@ mod tests {
     #[test]
     fn test_generate_cpp_signals() {
         let signals = vec![ParsedSignal {
-            ident: format_ident!("data_changed"),
-            cxx_name: None,
-            inherit: false,
+            method: parse_quote! {
+                fn data_changed(self: Pin<&mut MyObject>, trivial: i32, opaque: UniquePtr<QColor>);
+            },
+            qobject_ident: format_ident!("MyObject"),
+            mutable: true,
             parameters: vec![
                 ParsedFunctionParameter {
                     ident: format_ident!("trivial"),
@@ -141,6 +143,12 @@ mod tests {
                     ty: parse_quote! { UniquePtr<QColor> },
                 },
             ],
+            ident: CombinedIdent {
+                cpp: format_ident!("dataChanged"),
+                rust: format_ident!("data_changed"),
+            },
+            safe: true,
+            inherit: false,
         }];
         let qobject_idents = create_qobjectname();
 
@@ -208,13 +216,21 @@ mod tests {
     #[test]
     fn test_generate_cpp_signals_mapped_cxx_name() {
         let signals = vec![ParsedSignal {
-            ident: format_ident!("data_changed"),
-            cxx_name: None,
-            inherit: false,
+            method: parse_quote! {
+                fn data_changed(self: Pin<&mut MyObject>, mapped: A1);
+            },
+            qobject_ident: format_ident!("MyObject"),
+            mutable: true,
             parameters: vec![ParsedFunctionParameter {
                 ident: format_ident!("mapped"),
                 ty: parse_quote! { A1 },
             }],
+            ident: CombinedIdent {
+                cpp: format_ident!("dataChanged"),
+                rust: format_ident!("data_changed"),
+            },
+            safe: true,
+            inherit: false,
         }];
         let qobject_idents = create_qobjectname();
 
@@ -280,10 +296,19 @@ mod tests {
     #[test]
     fn test_generate_cpp_signals_existing_cxx_name() {
         let signals = vec![ParsedSignal {
-            ident: format_ident!("ExistingSignal"),
-            cxx_name: Some("baseName".to_owned()),
-            inherit: true,
+            method: parse_quote! {
+                #[cxx_name = "baseName"]
+                fn existing_signal(self: Pin<&mut MyObject>);
+            },
+            qobject_ident: format_ident!("MyObject"),
+            mutable: true,
             parameters: vec![],
+            ident: CombinedIdent {
+                cpp: format_ident!("baseName"),
+                rust: format_ident!("existing_signal"),
+            },
+            safe: true,
+            inherit: true,
         }];
         let qobject_idents = create_qobjectname();
 

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

@@ -12,6 +12,7 @@ pub mod signals;
 use syn::Ident;
 
 /// Describes an ident which potentially has a different name in C++ and Rust
+#[derive(Clone, Debug, PartialEq)]
 pub struct CombinedIdent {
     /// The ident for C++
     pub cpp: Ident,

crates/cxx-qt-gen/src/generator/naming/signals.rs

@@ -10,6 +10,7 @@ use syn::Ident;
 
 /// Names for parts of a Q_SIGNAL
 pub struct QSignalName {
+    // TODO: this is removed
     pub enum_name: Ident,
     pub name: CombinedIdent,
     pub emit_name: CombinedIdent,
@@ -19,19 +20,12 @@ pub struct QSignalName {
 
 impl From<&ParsedSignal> for QSignalName {
     fn from(signal: &ParsedSignal) -> Self {
-        // Check if there is a cxx ident that should be used
-        let cxx_ident = if let Some(cxx_name) = &signal.cxx_name {
-            format_ident!("{}", cxx_name)
-        } else {
-            signal.ident.clone()
-        };
-
         Self {
-            enum_name: signal.ident.clone(),
-            name: CombinedIdent::from_signal(&signal.ident, &cxx_ident),
-            emit_name: CombinedIdent::emit_from_signal(&signal.ident, &cxx_ident),
-            connect_name: CombinedIdent::connect_from_signal(&signal.ident, &cxx_ident),
-            on_name: on_from_signal(&signal.ident),
+            enum_name: signal.ident.rust.clone(),
+            name: CombinedIdent::from_signal(&signal.ident),
+            emit_name: CombinedIdent::emit_from_signal(&signal.ident),
+            connect_name: CombinedIdent::connect_from_signal(&signal.ident),
+            on_name: on_from_signal(&signal.ident.rust),
         }
     }
 }
@@ -42,28 +36,30 @@ fn on_from_signal(ident: &Ident) -> Ident {
 
 impl CombinedIdent {
     /// For a given signal ident generate the Rust and C++ names
-    fn from_signal(ident: &Ident, cxx_ident: &Ident) -> Self {
+    fn from_signal(ident: &CombinedIdent) -> Self {
         Self {
-            cpp: format_ident!("{}", cxx_ident.to_string().to_case(Case::Camel)),
+            cpp: format_ident!("{}", ident.cpp.to_string().to_case(Case::Camel)),
             // Note that signal names are in camel case so we need to convert to snake and can't clone
-            rust: format_ident!("{}", ident.to_string().to_case(Case::Snake)),
+            rust: format_ident!("{}", ident.rust.to_string().to_case(Case::Snake)),
         }
     }
 
     /// For a given signal ident generate the Rust and C++ emit name
-    fn emit_from_signal(ident: &Ident, cxx_ident: &Ident) -> Self {
+    fn emit_from_signal(ident: &CombinedIdent) -> Self {
         Self {
-            cpp: format_ident!("emit{}", cxx_ident.to_string().to_case(Case::Pascal)),
-            rust: format_ident!("emit_{}", ident.to_string().to_case(Case::Snake)),
+            cpp: format_ident!("emit{}", ident.cpp.to_string().to_case(Case::Pascal)),
+            // Note that the Rust emit name is the same name as the signal for now
+            // in the future this emit wrapper in C++ will be removed.
+            rust: format_ident!("{}", ident.rust.to_string().to_case(Case::Snake)),
         }
     }
 
-    fn connect_from_signal(ident: &Ident, cxx_ident: &Ident) -> Self {
+    fn connect_from_signal(ident: &CombinedIdent) -> Self {
         Self {
             // Use signalConnect instead of onSignal here so that we don't
             // create a C++ name that is similar to the QML naming scheme for signals
-            cpp: format_ident!("{}Connect", cxx_ident.to_string().to_case(Case::Camel)),
-            rust: format_ident!("connect_{}", ident.to_string().to_case(Case::Snake)),
+            cpp: format_ident!("{}Connect", ident.cpp.to_string().to_case(Case::Camel)),
+            rust: format_ident!("connect_{}", ident.rust.to_string().to_case(Case::Snake)),
         }
     }
 }
@@ -72,12 +68,22 @@ impl CombinedIdent {
 mod tests {
     use super::*;
 
+    use syn::parse_quote;
+
     #[test]
     fn test_parsed_signal() {
         let qsignal = ParsedSignal {
-            ident: format_ident!("DataChanged"),
+            method: parse_quote! {
+                fn data_changed(self: Pin<&mut MyObject>);
+            },
+            qobject_ident: format_ident!("MyObject"),
+            mutable: true,
             parameters: vec![],
-            cxx_name: None,
+            ident: CombinedIdent {
+                cpp: format_ident!("dataChanged"),
+                rust: format_ident!("data_changed"),
+            },
+            safe: true,
             inherit: false,
         };
 
@@ -98,10 +104,19 @@ mod tests {
     #[test]
     fn test_parsed_signal_existing_cxx_name() {
         let qsignal = ParsedSignal {
-            ident: format_ident!("ExistingSignal"),
+            method: parse_quote! {
+                #[cxx_name = "baseName"]
+                fn existing_signal(self: Pin<&mut MyObject>);
+            },
+            qobject_ident: format_ident!("MyObject"),
+            mutable: true,
             parameters: vec![],
-            cxx_name: Some("baseName".to_owned()),
-            inherit: true,
+            ident: CombinedIdent {
+                cpp: format_ident!("baseName"),
+                rust: format_ident!("existing_signal"),
+            },
+            safe: true,
+            inherit: false,
         };
 
         let names = QSignalName::from(&qsignal);

crates/cxx-qt-gen/src/generator/rust/property/mod.rs

@@ -16,11 +16,14 @@ use crate::{
 };
 use syn::Result;
 
+use super::signals::generate_rust_signals;
+
 pub fn generate_rust_properties(
     properties: &Vec<ParsedQProperty>,
     qobject_idents: &QObjectName,
 ) -> Result<GeneratedRustQObjectBlocks> {
     let mut generated = GeneratedRustQObjectBlocks::default();
+    let mut signals = vec![];
 
     for property in properties {
         let idents = QPropertyName::from(property);
@@ -44,15 +47,11 @@ pub fn generate_rust_properties(
             .append(&mut setter.implementation_as_items()?);
 
         // Signals
-        let notify = signal::generate(&idents, qobject_idents);
-        generated
-            .cxx_mod_contents
-            .append(&mut notify.cxx_bridge_as_items()?);
-        generated
-            .cxx_qt_mod_contents
-            .append(&mut notify.implementation_as_items()?);
+        signals.push(signal::generate(&idents, qobject_idents));
     }
 
+    generated.append(&mut generate_rust_signals(&signals, qobject_idents)?);
+
     Ok(generated)
 }
 

crates/cxx-qt-gen/src/generator/rust/property/signal.rs

@@ -3,32 +3,27 @@
 //
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
-use crate::generator::{
-    naming::{property::QPropertyName, qobject::QObjectName},
-    rust::fragment::RustFragmentPair,
+use syn::ForeignItemFn;
+
+use crate::{
+    generator::naming::{property::QPropertyName, qobject::QObjectName},
+    parser::signals::ParsedSignal,
 };
-use quote::quote;
 
-pub fn generate(idents: &QPropertyName, qobject_idents: &QObjectName) -> RustFragmentPair {
-    let cpp_class_name_rust = &qobject_idents.cpp_class.rust;
+pub fn generate(idents: &QPropertyName, qobject_idents: &QObjectName) -> ParsedSignal {
+    // 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.cpp_class.rust;
     let notify_cpp = &idents.notify.cpp;
-    let notify_rust = idents.notify.rust.to_string();
-    let ident_str = idents.name.rust.to_string();
-    let getter_mutable_rust_str = idents.getter_mutable.rust.to_string();
-
-    RustFragmentPair {
-        cxx_bridge: vec![quote! {
-            unsafe extern "C++" {
-                #[doc = "Notify signal for the Q_PROPERTY"]
-                #[doc = #ident_str]
-                #[doc = "\n"]
-                #[doc = "This can be used to manually notify a change when the unsafe mutable getter,"]
-                #[doc = #getter_mutable_rust_str]
-                #[doc = ", is used."]
-                #[rust_name = #notify_rust]
-                fn #notify_cpp(self: Pin<&mut #cpp_class_name_rust>);
-            }
-        }],
-        implementation: vec![],
-    }
+    let notify_rust_str = idents.notify.rust.to_string();
+    let method: ForeignItemFn = syn::parse_quote! {
+        #[doc = "Notify for the Q_PROPERTY"]
+        #[rust_name = #notify_rust_str]
+        fn #notify_cpp(self: Pin<&mut #cpp_class_rust>);
+    };
+    ParsedSignal::from_property_method(
+        method,
+        idents.notify.clone(),
+        qobject_idents.cpp_class.rust.clone(),
+    )
 }