Skip to content

Commit 1e988fd

Browse files
BenFordTytheringtonBenFordTytherington
committed
Allow immutable signals, and add one to qml_features
- Immutable signals are now allowed but only in extern "C++Qt" blocks - Test added in qml_features with a const signal
1 parent 48acbdb commit 1e988fd

File tree

15 files changed

+231
-30
lines changed

15 files changed

+231
-30
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub fn generate(idents: &QPropertyNames, qobject_name: &Name) -> Option<ParsedSi
2525
fn #notify_rust(self: Pin<&mut #cpp_class_rust>);
2626
};
2727

28-
Some(ParsedSignal::parse(method, CaseConversion::none()).unwrap())
28+
Some(ParsedSignal::parse_rust_qt_signal(method, CaseConversion::none()).unwrap())
2929
} else {
3030
None
3131
}

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ fn parameter_types_and_values(
5151
parameters: &[ParsedFunctionParameter],
5252
type_names: &TypeNames,
5353
self_ty: &Name,
54+
mutable: bool,
5455
) -> Result<Parameters> {
5556
let mut parameter_named_types_with_self = vec![];
5657
let mut parameter_types_with_self = vec![];
@@ -66,10 +67,11 @@ fn parameter_types_and_values(
6667

6768
let parameter_named_types = parameter_named_types_with_self.join(", ");
6869

70+
let is_const = if mutable { "" } else { " const" };
6971
// Insert the extra argument into the closure
7072
let self_ty = self_ty.cxx_qualified();
71-
parameter_named_types_with_self.insert(0, format!("{self_ty}& self"));
72-
parameter_types_with_self.insert(0, format!("{self_ty}&"));
73+
parameter_named_types_with_self.insert(0, format!("{self_ty}{is_const}& self"));
74+
parameter_types_with_self.insert(0, format!("{self_ty}{is_const}&"));
7375
parameter_values_with_self.insert(0, "self".to_owned());
7476

7577
Ok(Parameters {
@@ -109,7 +111,8 @@ pub fn generate_cpp_signal(
109111
let free_connect_ident_cpp = idents_helper.connect_name.cxx_unqualified();
110112

111113
// Retrieve the parameters for the signal
112-
let parameters = parameter_types_and_values(&signal.parameters, type_names, qobject_name)?;
114+
let parameters =
115+
parameter_types_and_values(&signal.parameters, type_names, qobject_name, signal.mutable)?;
113116
let parameters_named_types = parameters.named_types;
114117
let parameters_named_types_with_self = parameters.named_types_with_self;
115118
let parameter_types_with_self = parameters.types_with_self;
@@ -121,6 +124,8 @@ pub fn generate_cpp_signal(
121124
let signal_handler_call = idents_helper.function_call;
122125
let signal_handler_drop = idents_helper.function_drop;
123126
let namespace = idents_helper.namespace;
127+
let reference_type = if signal.mutable { "&" } else { " const &" };
128+
let is_const = if signal.mutable { "" } else { " const" };
124129

125130
let signal_handler_type = format!("SignalHandler<::{namespace}::{param_struct} *>");
126131

@@ -135,7 +140,7 @@ pub fn generate_cpp_signal(
135140
// Generate the Q_SIGNAL if this is not an existing signal
136141
if !signal.inherit {
137142
generated.methods.push(CppFragment::Header(format!(
138-
"Q_SIGNAL void {signal_ident}({parameters_named_types});"
143+
"Q_SIGNAL void {signal_ident}({parameters_named_types}){is_const};"
139144
)));
140145
}
141146

@@ -144,7 +149,7 @@ pub fn generate_cpp_signal(
144149
r#"
145150
namespace {namespace} {{
146151
::QMetaObject::Connection
147-
{free_connect_ident_cpp}({qobject_ident_namespaced}& self, {signal_handler_alias_namespaced} closure, ::Qt::ConnectionType type);
152+
{free_connect_ident_cpp}({qobject_ident_namespaced}{reference_type} self, {signal_handler_alias_namespaced} closure, ::Qt::ConnectionType type);
148153
}} // namespace {namespace}
149154
"#
150155
},
@@ -177,7 +182,7 @@ pub fn generate_cpp_signal(
177182
178183
namespace {namespace} {{
179184
::QMetaObject::Connection
180-
{free_connect_ident_cpp}({qobject_ident_namespaced}& self, {signal_handler_alias_namespaced} closure, ::Qt::ConnectionType type)
185+
{free_connect_ident_cpp}({qobject_ident_namespaced}{reference_type} self, {signal_handler_alias_namespaced} closure, ::Qt::ConnectionType type)
181186
{{
182187
return ::QObject::connect(
183188
&self,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ pub fn generate(idents: &QPropertyNames, qobject_names: &QObjectNames) -> Option
2828
fn #notify_rust(self: Pin<&mut #cpp_class_rust>);
2929
};
3030

31-
Some(ParsedSignal::parse(method, CaseConversion::none()).unwrap())
31+
Some(ParsedSignal::parse_rust_qt_signal(method, CaseConversion::none()).unwrap())
3232
} else {
3333
None
3434
}

‎crates/cxx-qt-gen/src/generator/rust/signals.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,7 @@ pub fn generate_rust_signal(
8585
let self_type_cxx = if signal.mutable {
8686
parse_quote_spanned! {span => Pin<&mut #qobject_name_rust> }
8787
} else {
88-
// CODECOV_EXCLUDE_START
89-
unreachable!("Signals cannot be immutable right now so this cannot be reached")
90-
// CODECOV_EXCLUDE_STOP
88+
parse_quote_spanned! {span => &#qobject_name_rust }
9189
};
9290
let self_type_qualified = syn_type_cxx_bridge_to_qualified(&self_type_cxx, type_names)?;
9391
let qualified_impl = qobject_name.rust_qualified();

‎crates/cxx-qt-gen/src/parser/externcxxqt.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ impl ParsedExternCxxQt {
108108
if attribute_get_path(&foreign_fn.attrs, &["inherit"]).is_some() {
109109
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"));
110110
}
111-
let mut signal = ParsedSignal::parse(foreign_fn, auto_case)?;
111+
let mut signal = ParsedSignal::parse_with_mutability(foreign_fn, auto_case, true)?;
112112
// extern "C++Qt" signals are always inherit = true
113113
// as they always exist on an existing QObject
114114
signal.inherit = true;

‎crates/cxx-qt-gen/src/parser/externrustqt.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ impl ParsedExternRustQt {
9191
) -> Result<()> {
9292
// Test if the function is a signal
9393
if attribute_get_path(&foreign_fn.attrs, &["qsignal"]).is_some() {
94-
let parsed_signal_method = ParsedSignal::parse(foreign_fn.clone(), auto_case)?;
94+
let parsed_signal_method = ParsedSignal::parse_rust_qt_signal(foreign_fn.clone(), auto_case)?;
9595
if parsed_signal_method.inherit
9696
&& foreign_fn.sig.unsafety.is_none()
9797
&& self.unsafety.is_none()

‎crates/cxx-qt-gen/src/parser/signals.rs

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ use crate::{
99
};
1010
use core::ops::Deref;
1111
use std::ops::DerefMut;
12-
use syn::{spanned::Spanned, Attribute, Error, ForeignItemFn, Result, Visibility};
12+
use syn::spanned::Spanned;
13+
use syn::{Attribute, Error, ForeignItemFn, Result, Visibility};
1314

1415
#[derive(Clone)]
1516
/// Describes an individual Signal
@@ -33,19 +34,24 @@ impl ParsedSignal {
3334
#[cfg(test)]
3435
/// Test fn for creating a mocked signal from a method body
3536
pub fn mock(method: &ForeignItemFn) -> Self {
36-
Self::parse(method.clone(), CaseConversion::none()).unwrap()
37+
Self::parse_rust_qt_signal(method.clone(), CaseConversion::none()).unwrap()
3738
}
3839

39-
pub fn parse(method: ForeignItemFn, auto_case: CaseConversion) -> Result<Self> {
40+
pub fn parse_with_mutability(
41+
method: ForeignItemFn,
42+
auto_case: CaseConversion,
43+
immutable_allowed: bool,
44+
) -> Result<Self> {
4045
let docs = extract_docs(&method.attrs);
4146
let cfgs = extract_cfgs(&method.attrs);
4247
let fields = MethodFields::parse(method, auto_case)?;
4348
let attrs = require_attributes(&fields.method.attrs, &Self::ALLOWED_ATTRS)?;
4449

45-
if !fields.mutable {
50+
// TODO: add proper checks
51+
if !fields.mutable && !immutable_allowed {
4652
return Err(Error::new(
4753
fields.method.span(),
48-
"signals must be mutable, use Pin<&mut T> instead of T for the self type",
54+
"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",
4955
));
5056
}
5157

@@ -65,6 +71,10 @@ impl ParsedSignal {
6571
cfgs,
6672
})
6773
}
74+
75+
pub fn parse_rust_qt_signal(method: ForeignItemFn, auto_case: CaseConversion) -> Result<Self> {
76+
Self::parse_with_mutability(method, auto_case, false)
77+
}
6878
}
6979

7080
impl Deref for ParsedSignal {
@@ -94,28 +104,26 @@ mod tests {
94104
#[test]
95105
fn test_parse_signal_invalid() {
96106
assert_parse_errors! {
97-
|input| ParsedSignal::parse(input, CaseConversion::none()) =>
107+
|input| ParsedSignal::parse_rust_qt_signal(input, CaseConversion::none()) =>
98108

99-
// No immutable signals
100-
{ fn ready(self: &MyObject); }
109+
// No namespaces
101110
{
102-
// No namespaces
103111
#[namespace = "disallowed_namespace"]
104112
fn ready(self: Pin<&mut MyObject>);
105113
}
106114
// Missing self
107115
{ fn ready(x: f64); }
108-
// Self needs to be receiver like self: &T instead of &self
116+
// Immutable signals must be in "C++Qt" blocks
109117
{ fn ready(&self); }
110-
}
118+
};
111119
}
112120

113121
#[test]
114122
fn test_parse_signal() {
115123
let method: ForeignItemFn = parse_quote! {
116124
fn ready(self: Pin<&mut MyObject>);
117125
};
118-
let signal = ParsedSignal::parse(method.clone(), CaseConversion::none()).unwrap();
126+
let signal = ParsedSignal::parse_rust_qt_signal(method.clone(), CaseConversion::none()).unwrap();
119127
assert_eq!(signal.method, method);
120128
assert_eq!(signal.qobject_ident, format_ident!("MyObject"));
121129
assert!(signal.mutable);
@@ -132,7 +140,7 @@ mod tests {
132140
#[cxx_name = "cppReady"]
133141
fn ready(self: Pin<&mut MyObject>);
134142
};
135-
let signal = ParsedSignal::parse(method, CaseConversion::none()).unwrap();
143+
let signal = ParsedSignal::parse_rust_qt_signal(method, CaseConversion::none()).unwrap();
136144

137145
let expected_method: ForeignItemFn = parse_quote! {
138146
#[cxx_name = "cppReady"]
@@ -154,7 +162,7 @@ mod tests {
154162
#[inherit]
155163
fn ready(self: Pin<&mut MyObject>);
156164
};
157-
let signal = ParsedSignal::parse(method.clone(), CaseConversion::none()).unwrap();
165+
let signal = ParsedSignal::parse_rust_qt_signal(method.clone(), CaseConversion::none()).unwrap();
158166

159167
assert_eq!(signal.method, method);
160168
assert_eq!(signal.qobject_ident, format_ident!("MyObject"));
@@ -171,7 +179,7 @@ mod tests {
171179
let method: ForeignItemFn = parse_quote! {
172180
fn ready(self: Pin<&mut MyObject>, x: f64, y: f64);
173181
};
174-
let signal = ParsedSignal::parse(method.clone(), CaseConversion::none()).unwrap();
182+
let signal = ParsedSignal::parse_rust_qt_signal(method.clone(), CaseConversion::none()).unwrap();
175183
assert_eq!(signal.method, method);
176184
assert_eq!(signal.qobject_ident, format_ident!("MyObject"));
177185
assert!(signal.mutable);
@@ -191,7 +199,7 @@ mod tests {
191199
let method: ForeignItemFn = parse_quote! {
192200
pub(self) fn ready(self: Pin<&mut MyObject>);
193201
};
194-
let signal = ParsedSignal::parse(method.clone(), CaseConversion::none()).unwrap();
202+
let signal = ParsedSignal::parse_rust_qt_signal(method.clone(), CaseConversion::none()).unwrap();
195203
assert_eq!(signal.method, method);
196204
assert_eq!(signal.qobject_ident, format_ident!("MyObject"));
197205
assert!(signal.mutable);
@@ -207,7 +215,7 @@ mod tests {
207215
let method: ForeignItemFn = parse_quote! {
208216
unsafe fn ready(self: Pin<&mut MyObject>);
209217
};
210-
let signal = ParsedSignal::parse(method.clone(), CaseConversion::none()).unwrap();
218+
let signal = ParsedSignal::parse_rust_qt_signal(method.clone(), CaseConversion::none()).unwrap();
211219
assert_eq!(signal.method, method);
212220
assert_eq!(signal.qobject_ident, format_ident!("MyObject"));
213221
assert!(signal.mutable);

‎crates/cxx-qt-gen/test_inputs/signals.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ mod ffi {
1717
/// When the QTimer timeout occurs
1818
#[qsignal]
1919
pub(self) fn timeout(self: Pin<&mut Self>);
20+
21+
/// A constant signal for when the timer is ready
22+
#[qsignal]
23+
fn const_ready(&self);
24+
2025
}
2126

2227
unsafe extern "RustQt" {

‎crates/cxx-qt-gen/test_outputs/signals.cpp

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,60 @@ QTimer_timeoutConnect(
5656
}
5757
} // namespace cxx_qt::my_object::rust::cxxqtgen1
5858

59+
// Define namespace otherwise we hit a GCC bug
60+
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480
61+
namespace rust::cxxqt1 {
62+
template<>
63+
SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1::
64+
QTimerCxxQtSignalParamsconst_ready*>::~SignalHandler() noexcept
65+
{
66+
if (data[0] == nullptr && data[1] == nullptr) {
67+
return;
68+
}
69+
70+
drop_QTimer_signal_handler_const_ready(::std::move(*this));
71+
}
72+
73+
template<>
74+
template<>
75+
void
76+
SignalHandler<
77+
::cxx_qt::my_object::rust::cxxqtgen1::QTimerCxxQtSignalParamsconst_ready*>::
78+
operator()<cxx_qt::my_object::QTimer const&>(
79+
cxx_qt::my_object::QTimer const& self)
80+
{
81+
call_QTimer_signal_handler_const_ready(*this, self);
82+
}
83+
84+
static_assert(alignof(SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1::
85+
QTimerCxxQtSignalParamsconst_ready*>) <=
86+
alignof(::std::size_t),
87+
"unexpected aligment");
88+
static_assert(sizeof(SignalHandler<::cxx_qt::my_object::rust::cxxqtgen1::
89+
QTimerCxxQtSignalParamsconst_ready*>) ==
90+
sizeof(::std::size_t[2]),
91+
"unexpected size");
92+
} // namespace rust::cxxqt1
93+
94+
namespace cxx_qt::my_object::rust::cxxqtgen1 {
95+
::QMetaObject::Connection
96+
QTimer_const_readyConnect(
97+
cxx_qt::my_object::QTimer const& self,
98+
::cxx_qt::my_object::rust::cxxqtgen1::QTimerCxxQtSignalHandlerconst_ready
99+
closure,
100+
::Qt::ConnectionType type)
101+
{
102+
return ::QObject::connect(
103+
&self,
104+
&cxx_qt::my_object::QTimer::const_ready,
105+
&self,
106+
[&, closure = ::std::move(closure)]() mutable {
107+
closure.template operator()<cxx_qt::my_object::QTimer const&>(self);
108+
},
109+
type);
110+
}
111+
} // namespace cxx_qt::my_object::rust::cxxqtgen1
112+
59113
// Define namespace otherwise we hit a GCC bug
60114
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480
61115
namespace rust::cxxqt1 {

‎crates/cxx-qt-gen/test_outputs/signals.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ using QTimerCxxQtSignalHandlertimeout =
2929
::rust::cxxqt1::SignalHandler<struct QTimerCxxQtSignalParamstimeout*>;
3030
} // namespace cxx_qt::my_object::rust::cxxqtgen1
3131

32+
namespace cxx_qt::my_object::rust::cxxqtgen1 {
33+
using QTimerCxxQtSignalHandlerconst_ready =
34+
::rust::cxxqt1::SignalHandler<struct QTimerCxxQtSignalParamsconst_ready*>;
35+
} // namespace cxx_qt::my_object::rust::cxxqtgen1
36+
3237
#include "directory/file_ident.cxx.h"
3338

3439
namespace cxx_qt::my_object::rust::cxxqtgen1 {
@@ -39,6 +44,15 @@ QTimer_timeoutConnect(
3944
::Qt::ConnectionType type);
4045
} // namespace cxx_qt::my_object::rust::cxxqtgen1
4146

47+
namespace cxx_qt::my_object::rust::cxxqtgen1 {
48+
::QMetaObject::Connection
49+
QTimer_const_readyConnect(
50+
cxx_qt::my_object::QTimer const& self,
51+
::cxx_qt::my_object::rust::cxxqtgen1::QTimerCxxQtSignalHandlerconst_ready
52+
closure,
53+
::Qt::ConnectionType type);
54+
} // namespace cxx_qt::my_object::rust::cxxqtgen1
55+
4256
namespace cxx_qt::my_object::rust::cxxqtgen1 {
4357
::QMetaObject::Connection
4458
MyObject_readyConnect(

0 commit comments

Comments
 (0)