Skip to content

Commit

Permalink
cxx-qt-gen: do not generate getter/setter for fields anymore
Browse files Browse the repository at this point in the history
As we move qproperty to an attribute we need to disable generating
helpers for fields as we won't be able to find them when the struct
is outside the bridge.

There are three options going forwards
- let the developer create their own getters/setters (this commit)
- add a #[field(T, NAME)] as an attribute to the qobject later
- remove the (un)safe for qproperty with notify so that rust_mut is safe

Related to KDAB#559
  • Loading branch information
ahayzen-kdab committed Jun 12, 2023
1 parent 10e324c commit 4e3b5d7
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 107 deletions.
8 changes: 6 additions & 2 deletions crates/cxx-qt-gen/src/parser/qobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl ParsedQObject {

// Parse any properties in the struct
// and remove the #[qproperty] attribute
let (properties, fields) = Self::parse_struct_fields(&mut qobject_struct.fields)?;
let (properties, _) = Self::parse_struct_fields(&mut qobject_struct.fields)?;

// Ensure that the QObject is marked as pub otherwise the error is non obvious
// https://github.com/KDAB/cxx-qt/issues/457
Expand All @@ -119,7 +119,11 @@ impl ParsedQObject {
inherited_methods: vec![],
passthrough_impl_items: vec![],
properties,
fields,
// Do not generate helpers for fields as they are going to move out of the bridge
//
// TODO: we may bring #[field(T, NAME)] as a way of doing this
// if we want to keep the unsafety vs safety of property changes with or without notify
fields: vec![],
qml_metadata,
locking: true,
threading: false,
Expand Down
17 changes: 0 additions & 17 deletions crates/cxx-qt-gen/test_outputs/inheritance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,23 +86,6 @@ mod cxx_qt_inheritance {
pub struct MyObject {
data: Vec<i32>,
}
impl MyObjectQt {
fn data(&self) -> &Vec<i32> {
&self.rust().data
}
}
impl MyObjectQt {
fn data_mut<'a>(self: Pin<&'a mut Self>) -> &'a mut Vec<i32> {
unsafe { &mut self.rust_mut().get_unchecked_mut().data }
}
}
impl MyObjectQt {
fn set_data(self: Pin<&mut Self>, value: Vec<i32>) {
unsafe {
self.rust_mut().data = value;
}
}
}
impl MyObject {
#[doc(hidden)]
pub fn data_wrapper(
Expand Down
51 changes: 0 additions & 51 deletions crates/cxx-qt-gen/test_outputs/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,57 +232,6 @@ mod cxx_qt_ffi {
self.connect_trivial_changed(func, CxxQtConnectionType::AutoConnection)
}
}
impl MyObjectQt {
fn opaque(&self) -> &UniquePtr<Opaque> {
&self.rust().opaque
}
}
impl MyObjectQt {
fn opaque_mut<'a>(self: Pin<&'a mut Self>) -> &'a mut UniquePtr<Opaque> {
unsafe { &mut self.rust_mut().get_unchecked_mut().opaque }
}
}
impl MyObjectQt {
fn set_opaque(self: Pin<&mut Self>, value: UniquePtr<Opaque>) {
unsafe {
self.rust_mut().opaque = value;
}
}
}
impl MyObjectQt {
fn private_rust_field(&self) -> &i32 {
&self.rust().private_rust_field
}
}
impl MyObjectQt {
fn private_rust_field_mut<'a>(self: Pin<&'a mut Self>) -> &'a mut i32 {
unsafe { &mut self.rust_mut().get_unchecked_mut().private_rust_field }
}
}
impl MyObjectQt {
fn set_private_rust_field(self: Pin<&mut Self>, value: i32) {
unsafe {
self.rust_mut().private_rust_field = value;
}
}
}
impl MyObjectQt {
pub fn public_rust_field(&self) -> &f64 {
&self.rust().public_rust_field
}
}
impl MyObjectQt {
pub fn public_rust_field_mut<'a>(self: Pin<&'a mut Self>) -> &'a mut f64 {
unsafe { &mut self.rust_mut().get_unchecked_mut().public_rust_field }
}
}
impl MyObjectQt {
pub fn set_public_rust_field(self: Pin<&mut Self>, value: f64) {
unsafe {
self.rust_mut().public_rust_field = value;
}
}
}
impl cxx_qt::Locking for MyObjectQt {}
impl cxx_qt::CxxQtType for MyObjectQt {
type Rust = MyObject;
Expand Down
10 changes: 5 additions & 5 deletions examples/demo_threading/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ mod ffi {
/// A Q_INVOKABLE that returns the current power usage for a given uuid
#[qinvokable]
pub fn sensor_power(self: Pin<&mut Self>, uuid: &QString) -> f64 {
let sensors = SensorsWorker::read_sensors(self.sensors_map_mut());
let sensors = SensorsWorker::read_sensors(&unsafe { self.rust_mut() }.sensors_map);

if let Ok(uuid) = Uuid::parse_str(&uuid.to_string()) {
sensors.get(&uuid).map(|v| v.power).unwrap_or_default()
Expand All @@ -112,16 +112,16 @@ mod ffi {
let sensors_changed = Arc::new(AtomicBool::new(false));

// Make relevent clones so that we can pass them to the threads
let accumulator_sensors = Arc::clone(self.as_mut().sensors_map_mut());
let accumulator_sensors = Arc::clone(&unsafe { self.as_mut().rust_mut() }.sensors_map);
let accumulator_sensors_changed = Arc::clone(&sensors_changed);
let accumulator_qt_thread = self.qt_thread();
let sensors = Arc::clone(self.as_mut().sensors_map_mut());
let sensors = Arc::clone(&unsafe { self.as_mut().rust_mut() }.sensors_map);
let sensors_qt_thread = self.qt_thread();
let timeout_sensors = Arc::clone(self.as_mut().sensors_map_mut());
let timeout_sensors = Arc::clone(&unsafe { self.as_mut().rust_mut() }.sensors_map);
let timeout_network_tx = network_tx.clone();

// Start our threads
*self.join_handles_mut() = Some([
unsafe { self.rust_mut() }.join_handles = Some([
// Create a TimeoutWorker
// If a sensor is not seen for N seconds then a disconnect is requested
std::thread::spawn(move || {
Expand Down
36 changes: 23 additions & 13 deletions examples/qml_features/rust/src/containers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,43 +62,48 @@ pub mod ffi {
/// Reset all the containers
#[qinvokable]
pub fn reset(mut self: Pin<&mut Self>) {
self.as_mut().set_hash(QHash_QString_QVariant::default());
self.as_mut().set_list(QList_i32::default());
// Update the private rust fields via the rust_mut
{
let mut rust_mut = unsafe { self.as_mut().rust_mut() };
rust_mut.hash = QHash_QString_QVariant::default();
rust_mut.list = QList_i32::default();
rust_mut.set = QSet_i32::default();
rust_mut.vector = QVector_i32::default();
}

self.as_mut().set_map(QMap_QString_QVariant::default());
self.as_mut().set_set(QSet_i32::default());
self.as_mut().set_vector(QVector_i32::default());

self.update_strings();
}

/// Append the given number to the vector container
#[qinvokable]
pub fn append_vector(mut self: Pin<&mut Self>, value: i32) {
self.as_mut().vector_mut().append(value);
unsafe { self.as_mut().rust_mut() }.vector.append(value);

self.update_strings();
}

/// Append the given number to the list container
#[qinvokable]
pub fn append_list(mut self: Pin<&mut Self>, value: i32) {
self.as_mut().list_mut().append(value);
unsafe { self.as_mut().rust_mut() }.list.append(value);

self.update_strings();
}

/// Insert the given number into the set container
#[qinvokable]
pub fn insert_set(mut self: Pin<&mut Self>, value: i32) {
self.as_mut().set_mut().insert(value);
unsafe { self.as_mut().rust_mut() }.set.insert(value);

self.update_strings();
}

/// Insert the given string and variant to the hash container
#[qinvokable]
pub fn insert_hash(mut self: Pin<&mut Self>, key: QString, value: QVariant) {
self.as_mut().hash_mut().insert(key, value);
unsafe { self.as_mut().rust_mut() }.hash.insert(key, value);

self.update_strings();
}
Expand All @@ -118,7 +123,8 @@ pub mod ffi {
fn update_strings(mut self: Pin<&mut Self>) {
let hash_items = self
.as_ref()
.hash()
.rust()
.hash
.iter()
.map(|(key, value)| {
let value = value.value::<i32>().unwrap_or(0);
Expand All @@ -130,7 +136,8 @@ pub mod ffi {

let list_items = self
.as_ref()
.list()
.rust()
.list
.iter()
.map(|value| value.to_string())
.collect::<Vec<String>>()
Expand All @@ -139,7 +146,8 @@ pub mod ffi {

let map_items = self
.as_ref()
.map()
.rust()
.map
.iter()
.map(|(key, value)| {
let value = value.value::<i32>().unwrap_or(0);
Expand All @@ -151,7 +159,8 @@ pub mod ffi {

let set_items = self
.as_ref()
.set()
.rust()
.set
.iter()
.map(|value| value.to_string())
.collect::<Vec<String>>()
Expand All @@ -160,7 +169,8 @@ pub mod ffi {

let vector_items = self
.as_ref()
.vector()
.rust()
.vector
.iter()
.map(|value| value.to_string())
.collect::<Vec<String>>()
Expand Down
15 changes: 10 additions & 5 deletions examples/qml_features/rust/src/custom_base_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,15 @@ pub mod ffi {
}

fn add_cpp_context(mut self: Pin<&mut Self>) {
let count = self.vector().len();
let count = self.as_ref().rust().vector.len();
unsafe {
self.as_mut().begin_insert_rows(
&QModelIndex::default(),
count as i32,
count as i32,
);
let id = *self.id();
self.as_mut().set_id(id + 1);
let id = self.as_ref().rust().id;
self.as_mut().rust_mut().id = id + 1;
self.as_mut().vector_mut().push((id, (id as f64) / 3.0));
self.as_mut().end_insert_rows();
}
Expand All @@ -114,7 +114,7 @@ pub mod ffi {
pub fn clear(mut self: Pin<&mut Self>) {
unsafe {
self.as_mut().begin_reset_model();
self.as_mut().set_id(0);
self.as_mut().rust_mut().id = 0;
self.as_mut().vector_mut().clear();
self.as_mut().end_reset_model();
}
Expand All @@ -139,7 +139,7 @@ pub mod ffi {
/// Remove the row with the given index
#[qinvokable]
pub fn remove(mut self: Pin<&mut Self>, index: i32) {
if index < 0 || (index as usize) >= self.vector().len() {
if index < 0 || (index as usize) >= self.as_ref().rust().vector.len() {
return;
}

Expand All @@ -150,6 +150,11 @@ pub mod ffi {
self.as_mut().end_remove_rows();
}
}

/// Safe helper to access mutate the vector field
pub fn vector_mut<'a>(self: Pin<&'a mut Self>) -> &'a mut Vec<(u32, f64)> {
&mut unsafe { self.rust_mut().get_unchecked_mut() }.vector
}
}

// ANCHOR: book_inherit_qalm_impl_unsafe
Expand Down
7 changes: 4 additions & 3 deletions examples/qml_features/rust/src/invokables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ pub mod ffi {

/// Mutable C++ context method that helps to store the color
pub fn store_helper(mut self: Pin<&mut Self>, red: f32, green: f32, blue: f32) {
self.as_mut().set_red(red);
self.as_mut().set_green(green);
self.as_mut().set_blue(blue);
let mut rust_mut = unsafe { self.as_mut().rust_mut() };
rust_mut.red = red;
rust_mut.green = green;
rust_mut.blue = blue;
}
}
// ANCHOR_END: book_impl_qobject
Expand Down
6 changes: 3 additions & 3 deletions examples/qml_features/rust/src/signals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub mod ffi {
// Determine if logging is enabled
if *qobject.as_ref().logging_enabled() {
// If no connections have been made, then create them
if qobject.as_ref().connections().is_none() {
if qobject.as_ref().rust().connections.is_none() {
// ANCHOR: book_signals_connect
let connections = [
qobject.as_mut().on_connected(|_, url| {
Expand All @@ -93,15 +93,15 @@ pub mod ffi {
ConnectionType::QueuedConnection,
),
];
qobject.as_mut().set_connections(Some(connections));
unsafe { qobject.as_mut().rust_mut() }.connections = Some(connections);
// ANCHOR_END: book_signals_connect
}
} else {
// Disabling logging so disconnect
// ANCHOR: book_signals_disconnect
// By making connections None, we trigger a drop on the connections
// this then causes disconnections
qobject.as_mut().set_connections(None);
unsafe { qobject.as_mut().rust_mut() }.connections = None;
// ANCHOR_END: book_signals_disconnect
}
})
Expand Down
8 changes: 4 additions & 4 deletions tests/basic_cxx_qt/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ mod ffi {
pub fn queue_test(self: Pin<&mut Self>) {
let qt_thread = self.qt_thread();
qt_thread
.queue(|ctx| {
*ctx.update_call_count_mut() += 1;
.queue(|qobject| {
unsafe { qobject.rust_mut() }.update_call_count += 1;
})
.unwrap();
}
Expand All @@ -80,8 +80,8 @@ mod ffi {
let qt_thread_cloned = qt_thread.clone();
handles.push(std::thread::spawn(move || {
qt_thread_cloned
.queue(|ctx| {
*ctx.update_call_count_mut() += 1;
.queue(|qobject| {
unsafe { qobject.rust_mut() }.update_call_count += 1;
})
.unwrap();
N_REQUESTS.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
Expand Down
8 changes: 4 additions & 4 deletions tests/basic_cxx_qt/rust/src/locking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ pub mod ffi {
impl qobject::RustLockingEnabled {
#[qinvokable]
pub fn get_counter(&self) -> u32 {
self.counter().load(Ordering::Acquire)
self.rust().counter.load(Ordering::Acquire)
}

#[qinvokable]
pub fn increment(self: Pin<&mut Self>) {
let counter = self.as_ref().get_counter();
thread::sleep(Duration::from_millis(100));
self.counter().store(counter + 1, Ordering::Release);
self.rust().counter.store(counter + 1, Ordering::Release);
}
}

Expand All @@ -45,14 +45,14 @@ pub mod ffi {
impl qobject::RustLockingDisabled {
#[qinvokable]
pub fn get_counter(&self) -> u32 {
self.counter().load(Ordering::Acquire)
self.rust().counter.load(Ordering::Acquire)
}

#[qinvokable]
pub fn increment(self: Pin<&mut Self>) {
let counter = self.as_ref().get_counter();
thread::sleep(Duration::from_millis(100));
self.counter().store(counter + 1, Ordering::Release);
self.rust().counter.store(counter + 1, Ordering::Release);
}
}
}

0 comments on commit 4e3b5d7

Please sign in to comment.