Allow immutable signals, and add one to qml_features (#1317)

BenFordTytherington · web-flow · commit 8a8720357f08 · 2025-09-05T14:20:42.000Z
- Immutable signals are now allowed but only in extern "C++Qt" blocks
- Test added in qml_features with a const signal
diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs b/crates/cxx-qt-gen/src/generator/cpp/property/signal.rs
@@ -25,7 +25,7 @@ pub fn generate(idents: &QPropertyNames, qobject_name: &Name) -> Option<ParsedSi
             fn #notify_rust(self: Pin<&mut #cpp_class_rust>);
         };
 
-        Some(ParsedSignal::parse(method, CaseConversion::none()).unwrap())
+        Some(ParsedSignal::parse_rust_qt_signal(method, CaseConversion::none()).unwrap())
     } else {
         None
     }
diff --git a/crates/cxx-qt-gen/src/generator/cpp/signal.rs b/crates/cxx-qt-gen/src/generator/cpp/signal.rs
@@ -51,6 +51,7 @@ fn parameter_types_and_values(
     parameters: &[ParsedFunctionParameter],
     type_names: &TypeNames,
     self_ty: &Name,
+    mutable: bool,
 ) -> Result<Parameters> {
     let mut parameter_named_types_with_self = vec![];
     let mut parameter_types_with_self = vec![];
@@ -66,10 +67,11 @@ fn parameter_types_and_values(
 
     let parameter_named_types = parameter_named_types_with_self.join(", ");
 
+    let is_const = if mutable { "" } else { " const" };
     // Insert the extra argument into the closure
     let self_ty = self_ty.cxx_qualified();
-    parameter_named_types_with_self.insert(0, format!("{self_ty}& self"));
-    parameter_types_with_self.insert(0, format!("{self_ty}&"));
+    parameter_named_types_with_self.insert(0, format!("{self_ty}{is_const}& self"));
+    parameter_types_with_self.insert(0, format!("{self_ty}{is_const}&"));
     parameter_values_with_self.insert(0, "self".to_owned());
 
     Ok(Parameters {
@@ -109,7 +111,8 @@ pub fn generate_cpp_signal(
     let free_connect_ident_cpp = idents_helper.connect_name.cxx_unqualified();
 
     // Retrieve the parameters for the signal
-    let parameters = parameter_types_and_values(&signal.parameters, type_names, qobject_name)?;
+    let parameters =
+        parameter_types_and_values(&signal.parameters, type_names, qobject_name, signal.mutable)?;
     let parameters_named_types = parameters.named_types;
     let parameters_named_types_with_self = parameters.named_types_with_self;
     let parameter_types_with_self = parameters.types_with_self;
@@ -121,6 +124,8 @@ pub fn generate_cpp_signal(
     let signal_handler_call = idents_helper.function_call;
     let signal_handler_drop = idents_helper.function_drop;
     let namespace = idents_helper.namespace;
+    let reference_type = if signal.mutable { "&" } else { " const &" };
+    let is_const = if signal.mutable { "" } else { " const" };
 
     let signal_handler_type = format!("SignalHandler<::{namespace}::{param_struct} *>");
 
@@ -135,7 +140,7 @@ pub fn generate_cpp_signal(
     // Generate the Q_SIGNAL if this is not an existing signal
     if !signal.inherit {
         generated.methods.push(CppFragment::Header(format!(
-            "Q_SIGNAL void {signal_ident}({parameters_named_types});"
+            "Q_SIGNAL void {signal_ident}({parameters_named_types}){is_const};"
         )));
     }
 
@@ -144,7 +149,7 @@ pub fn generate_cpp_signal(
         r#"
             namespace {namespace} {{
             ::QMetaObject::Connection
-            {free_connect_ident_cpp}({qobject_ident_namespaced}& self, {signal_handler_alias_namespaced} closure, ::Qt::ConnectionType type);
+            {free_connect_ident_cpp}({qobject_ident_namespaced}{reference_type} self, {signal_handler_alias_namespaced} closure, ::Qt::ConnectionType type);
             }} // namespace {namespace}
             "#
         },
@@ -177,7 +182,7 @@ pub fn generate_cpp_signal(
 
             namespace {namespace} {{
             ::QMetaObject::Connection
-            {free_connect_ident_cpp}({qobject_ident_namespaced}& self, {signal_handler_alias_namespaced} closure, ::Qt::ConnectionType type)
+            {free_connect_ident_cpp}({qobject_ident_namespaced}{reference_type} self, {signal_handler_alias_namespaced} closure, ::Qt::ConnectionType type)
             {{
                 return ::QObject::connect(
                     &self,
diff --git a/crates/cxx-qt-gen/src/generator/rust/property/signal.rs b/crates/cxx-qt-gen/src/generator/rust/property/signal.rs
@@ -28,7 +28,7 @@ pub fn generate(idents: &QPropertyNames, qobject_names: &QObjectNames) -> Option
             fn #notify_rust(self: Pin<&mut #cpp_class_rust>);
         };
 
-        Some(ParsedSignal::parse(method, CaseConversion::none()).unwrap())
+        Some(ParsedSignal::parse_rust_qt_signal(method, CaseConversion::none()).unwrap())
     } else {
         None
     }
diff --git a/crates/cxx-qt-gen/src/generator/rust/signals.rs b/crates/cxx-qt-gen/src/generator/rust/signals.rs
@@ -85,9 +85,7 @@ pub fn generate_rust_signal(
     let self_type_cxx = if signal.mutable {
         parse_quote_spanned! {span => Pin<&mut #qobject_name_rust> }
     } else {
-        // CODECOV_EXCLUDE_START
-        unreachable!("Signals cannot be immutable right now so this cannot be reached")
-        // CODECOV_EXCLUDE_STOP
+        parse_quote_spanned! {span => &#qobject_name_rust }
     };
     let self_type_qualified = syn_type_cxx_bridge_to_qualified(&self_type_cxx, type_names)?;
     let qualified_impl = qobject_name.rust_qualified();
diff --git a/crates/cxx-qt-gen/src/parser/externcxxqt.rs b/crates/cxx-qt-gen/src/parser/externcxxqt.rs
@@ -3,6 +3,7 @@
 //
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
+use crate::parser::signals::ImmutabilityConstraint;
 use crate::{
     parser::{
         externqobject::ParsedExternQObject, require_attributes, signals::ParsedSignal,
@@ -108,7 +109,11 @@ impl ParsedExternCxxQt {
             if attribute_get_path(&foreign_fn.attrs, &["inherit"]).is_some() {
                 return Err(Error::new(foreign_fn.span(), "#[inherit] is not allowed or necessary in extern \"C++Qt\" blocks, as all signals are inherited by default"));
             }
-            let mut signal = ParsedSignal::parse(foreign_fn, auto_case)?;
+            let mut signal = ParsedSignal::parse_with_mutability(
+                foreign_fn,
+                auto_case,
+                ImmutabilityConstraint::Allowed,
+            )?;
             // extern "C++Qt" signals are always inherit = true
             // as they always exist on an existing QObject
             signal.inherit = true;
diff --git a/crates/cxx-qt-gen/src/parser/externrustqt.rs b/crates/cxx-qt-gen/src/parser/externrustqt.rs
@@ -91,7 +91,8 @@ impl ParsedExternRustQt {
     ) -> Result<()> {
         // Test if the function is a signal
         if attribute_get_path(&foreign_fn.attrs, &["qsignal"]).is_some() {
-            let parsed_signal_method = ParsedSignal::parse(foreign_fn.clone(), auto_case)?;
+            let parsed_signal_method =
+                ParsedSignal::parse_rust_qt_signal(foreign_fn.clone(), auto_case)?;
             if parsed_signal_method.inherit
                 && foreign_fn.sig.unsafety.is_none()
                 && self.unsafety.is_none()
diff --git a/crates/cxx-qt-gen/src/parser/signals.rs b/crates/cxx-qt-gen/src/parser/signals.rs
@@ -9,7 +9,8 @@ use crate::{
 };
 use core::ops::Deref;
 use std::ops::DerefMut;
-use syn::{spanned::Spanned, Attribute, Error, ForeignItemFn, Result, Visibility};
+use syn::spanned::Spanned;
+use syn::{Attribute, Error, ForeignItemFn, Result, Visibility};
 
 #[derive(Clone)]
 /// Describes an individual Signal
@@ -26,26 +27,35 @@ pub struct ParsedSignal {
     pub cfgs: Vec<Attribute>,
 }
 
+pub enum ImmutabilityConstraint {
+    Allowed,
+    Disallowed,
+}
+
 impl ParsedSignal {
     const ALLOWED_ATTRS: [&'static str; 6] =
         ["cfg", "cxx_name", "rust_name", "inherit", "doc", "qsignal"];
 
     #[cfg(test)]
     /// Test fn for creating a mocked signal from a method body
     pub fn mock(method: &ForeignItemFn) -> Self {
-        Self::parse(method.clone(), CaseConversion::none()).unwrap()
+        Self::parse_rust_qt_signal(method.clone(), CaseConversion::none()).unwrap()
     }
 
-    pub fn parse(method: ForeignItemFn, auto_case: CaseConversion) -> Result<Self> {
+    pub fn parse_with_mutability(
+        method: ForeignItemFn,
+        auto_case: CaseConversion,
+        immutability_allowed: ImmutabilityConstraint,
+    ) -> Result<Self> {
         let docs = extract_docs(&method.attrs);
         let cfgs = extract_cfgs(&method.attrs);
         let fields = MethodFields::parse(method, auto_case)?;
         let attrs = require_attributes(&fields.method.attrs, &Self::ALLOWED_ATTRS)?;
 
-        if !fields.mutable {
+        if matches!(immutability_allowed, ImmutabilityConstraint::Disallowed) && !fields.mutable {
             return Err(Error::new(
                 fields.method.span(),
-                "signals must be mutable, use Pin<&mut T> instead of T for the self type",
+                "immutable signals can only be used in `unsafe extern \"C++Qt\"` blocks, use Pin<&mut T> instead of T for the self type, or change the type of this extern block",
             ));
         }
 
@@ -65,6 +75,10 @@ impl ParsedSignal {
             cfgs,
         })
     }
+
+    pub fn parse_rust_qt_signal(method: ForeignItemFn, auto_case: CaseConversion) -> Result<Self> {
+        Self::parse_with_mutability(method, auto_case, ImmutabilityConstraint::Disallowed)
+    }
 }
 
 impl Deref for ParsedSignal {
@@ -94,28 +108,27 @@ mod tests {
     #[test]
     fn test_parse_signal_invalid() {
         assert_parse_errors! {
-            |input| ParsedSignal::parse(input, CaseConversion::none()) =>
+            |input| ParsedSignal::parse_rust_qt_signal(input, CaseConversion::none()) =>
 
-            // No immutable signals
-            { fn ready(self: &MyObject); }
+            // No namespaces
             {
-                // No namespaces
                 #[namespace = "disallowed_namespace"]
                 fn ready(self: Pin<&mut MyObject>);
             }
             // Missing self
             { fn ready(x: f64); }
-            // Self needs to be receiver like self: &T instead of &self
+            // Immutable signals must be in "C++Qt" blocks
             { fn ready(&self); }
-        }
+        };
     }
 
     #[test]
     fn test_parse_signal() {
         let method: ForeignItemFn = parse_quote! {
             fn ready(self: Pin<&mut MyObject>);
         };
-        let signal = ParsedSignal::parse(method.clone(), CaseConversion::none()).unwrap();
+        let signal =
+            ParsedSignal::parse_rust_qt_signal(method.clone(), CaseConversion::none()).unwrap();
         assert_eq!(signal.method, method);
         assert_eq!(signal.qobject_ident, format_ident!("MyObject"));
         assert!(signal.mutable);
@@ -132,7 +145,7 @@ mod tests {
             #[cxx_name = "cppReady"]
             fn ready(self: Pin<&mut MyObject>);
         };
-        let signal = ParsedSignal::parse(method, CaseConversion::none()).unwrap();
+        let signal = ParsedSignal::parse_rust_qt_signal(method, CaseConversion::none()).unwrap();
 
         let expected_method: ForeignItemFn = parse_quote! {
             #[cxx_name = "cppReady"]
@@ -154,7 +167,8 @@ mod tests {
             #[inherit]
             fn ready(self: Pin<&mut MyObject>);
         };
-        let signal = ParsedSignal::parse(method.clone(), CaseConversion::none()).unwrap();
+        let signal =
+            ParsedSignal::parse_rust_qt_signal(method.clone(), CaseConversion::none()).unwrap();
 
         assert_eq!(signal.method, method);
         assert_eq!(signal.qobject_ident, format_ident!("MyObject"));
@@ -171,7 +185,8 @@ mod tests {
         let method: ForeignItemFn = parse_quote! {
             fn ready(self: Pin<&mut MyObject>, x: f64, y: f64);
         };
-        let signal = ParsedSignal::parse(method.clone(), CaseConversion::none()).unwrap();
+        let signal =
+            ParsedSignal::parse_rust_qt_signal(method.clone(), CaseConversion::none()).unwrap();
         assert_eq!(signal.method, method);
         assert_eq!(signal.qobject_ident, format_ident!("MyObject"));
         assert!(signal.mutable);
@@ -191,7 +206,8 @@ mod tests {
         let method: ForeignItemFn = parse_quote! {
             pub(self) fn ready(self: Pin<&mut MyObject>);
         };
-        let signal = ParsedSignal::parse(method.clone(), CaseConversion::none()).unwrap();
+        let signal =
+            ParsedSignal::parse_rust_qt_signal(method.clone(), CaseConversion::none()).unwrap();
         assert_eq!(signal.method, method);
         assert_eq!(signal.qobject_ident, format_ident!("MyObject"));
         assert!(signal.mutable);
@@ -207,7 +223,8 @@ mod tests {
         let method: ForeignItemFn = parse_quote! {
             unsafe fn ready(self: Pin<&mut MyObject>);
         };
-        let signal = ParsedSignal::parse(method.clone(), CaseConversion::none()).unwrap();
+        let signal =
+            ParsedSignal::parse_rust_qt_signal(method.clone(), CaseConversion::none()).unwrap();
         assert_eq!(signal.method, method);
         assert_eq!(signal.qobject_ident, format_ident!("MyObject"));
         assert!(signal.mutable);
diff --git a/crates/cxx-qt-gen/test_inputs/signals.rs b/crates/cxx-qt-gen/test_inputs/signals.rs
@@ -17,6 +17,11 @@ mod ffi {
         /// When the QTimer timeout occurs
         #[qsignal]
         pub(self) fn timeout(self: Pin<&mut Self>);
+
+        /// A constant signal for when the timer is ready
+        #[qsignal]
+        fn const_ready(&self);
+
     }
 
     unsafe extern "RustQt" {
diff --git a/crates/cxx-qt-gen/test_outputs/signals.cpp b/crates/cxx-qt-gen/test_outputs/signals.cpp
@@ -56,6 +56,60 @@ QTimer_timeoutConnect(
 }
 } // 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::
+                QTimerCxxQtSignalParamsconst_ready*>::~SignalHandler() noexcept
+{
+  if (data[0] == nullptr && data[1] == nullptr) {
+    return;
+  }
+
+  drop_QTimer_signal_handler_const_ready(::std::move(*this));
+}
+
+template<>
+template<>
+void
+SignalHandler<
+  ::cxx_qt::my_object::rust::cxxqtgen1::QTimerCxxQtSignalParamsconst_ready*>::
+operator()<cxx_qt::my_object::QTimer const&>(
+  cxx_qt::my_object::QTimer const& self)
+{
+  call_QTimer_signal_handler_const_ready(*this, self);
+}
+
+static_assert(alignof(SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1::
+                                      QTimerCxxQtSignalParamsconst_ready*>) <=
+                alignof(::std::size_t),
+              "unexpected aligment");
+static_assert(sizeof(SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1::
+                                     QTimerCxxQtSignalParamsconst_ready*>) ==
+                sizeof(::std::size_t[2]),
+              "unexpected size");
+} // namespace rust::cxxqt1
+
+namespace cxx_qt::my_object::rust::cxxqtgen1 {
+::QMetaObject::Connection
+QTimer_const_readyConnect(
+  cxx_qt::my_object::QTimer const& self,
+  ::cxx_qt::my_object::rust::cxxqtgen1::QTimerCxxQtSignalHandlerconst_ready
+    closure,
+  ::Qt::ConnectionType type)
+{
+  return ::QObject::connect(
+    &self,
+    &cxx_qt::my_object::QTimer::const_ready,
+    &self,
+    [&, closure = ::std::move(closure)]() mutable {
+      closure.template operator()<cxx_qt::my_object::QTimer const&>(self);
+    },
+    type);
+}
+} // 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 {
diff --git a/crates/cxx-qt-gen/test_outputs/signals.h b/crates/cxx-qt-gen/test_outputs/signals.h
@@ -29,6 +29,11 @@ using QTimerCxxQtSignalHandlertimeout =
   ::rust::cxxqt1::SignalHandler<struct QTimerCxxQtSignalParamstimeout*>;
 } // namespace cxx_qt::my_object::rust::cxxqtgen1
 
+namespace cxx_qt::my_object::rust::cxxqtgen1 {
+using QTimerCxxQtSignalHandlerconst_ready =
+  ::rust::cxxqt1::SignalHandler<struct QTimerCxxQtSignalParamsconst_ready*>;
+} // namespace cxx_qt::my_object::rust::cxxqtgen1
+
 #include "directory/file_ident.cxx.h"
 
 namespace cxx_qt::my_object::rust::cxxqtgen1 {
@@ -39,6 +44,15 @@ QTimer_timeoutConnect(
   ::Qt::ConnectionType type);
 } // namespace cxx_qt::my_object::rust::cxxqtgen1
 
+namespace cxx_qt::my_object::rust::cxxqtgen1 {
+::QMetaObject::Connection
+QTimer_const_readyConnect(
+  cxx_qt::my_object::QTimer const& self,
+  ::cxx_qt::my_object::rust::cxxqtgen1::QTimerCxxQtSignalHandlerconst_ready
+    closure,
+  ::Qt::ConnectionType type);
+} // namespace cxx_qt::my_object::rust::cxxqtgen1
+
 namespace cxx_qt::my_object::rust::cxxqtgen1 {
 ::QMetaObject::Connection
 MyObject_readyConnect(
diff --git a/crates/cxx-qt-gen/test_outputs/signals.rs b/crates/cxx-qt-gen/test_outputs/signals.rs
diff --git a/examples/qml_features/cpp/external_qobject.cpp b/examples/qml_features/cpp/external_qobject.cpp
diff --git a/examples/qml_features/cpp/external_qobject.h b/examples/qml_features/cpp/external_qobject.h
diff --git a/examples/qml_features/rust/src/externcxxqt.rs b/examples/qml_features/rust/src/externcxxqt.rs
diff --git a/examples/qml_features/tests/tst_externcxxqt.qml b/examples/qml_features/tests/tst_externcxxqt.qml

Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ pub fn generate(idents: &QPropertyNames, qobject_name: &Name) -> Option<ParsedSi`
`25`	`25`	`fn #notify_rust(self: Pin<&mut #cpp_class_rust>);`
`26`	`26`	`};`
`27`	`27`
`28`		`- Some(ParsedSignal::parse(method, CaseConversion::none()).unwrap())`
	`28`	`+ Some(ParsedSignal::parse_rust_qt_signal(method, CaseConversion::none()).unwrap())`
`29`	`29`	`} else {`
`30`	`30`	`None`
`31`	`31`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ pub fn generate(idents: &QPropertyNames, qobject_names: &QObjectNames) -> Option`
`28`	`28`	`fn #notify_rust(self: Pin<&mut #cpp_class_rust>);`
`29`	`29`	`};`
`30`	`30`
`31`		`- Some(ParsedSignal::parse(method, CaseConversion::none()).unwrap())`
	`31`	`+ Some(ParsedSignal::parse_rust_qt_signal(method, CaseConversion::none()).unwrap())`
`32`	`32`	`} else {`
`33`	`33`	`None`
`34`	`34`	`}`