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 13, 2023
1 parent 1b5bf8d commit 1e1fd8d
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 113 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Removed support for `cxx_type` and `cxx_return_type` and related conversion methods.
- Removed `newCppObject` function that allowed creation of default-constructed QObject from Rust.
- Generation of getter and setter for private Rust fields

## [0.5.3](https://github.com/KDAB/cxx-qt/compare/v0.5.2...v0.5.3) - 2023-05-19

Expand Down
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 @@ -84,23 +84,6 @@ pub 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 cxx_qt::Locking for MyObjectQt {}
impl cxx_qt::CxxQtType for MyObjectQt {
type Rust = MyObject;
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 @@ -233,57 +233,6 @@ pub 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 @@ -105,7 +105,7 @@ use uuid::Uuid;
impl ffi::EnergyUsageQt {
/// A Q_INVOKABLE that returns the current power usage for a given uuid
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 @@ -129,16 +129,16 @@ impl ffi::EnergyUsageQt {
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
45 changes: 27 additions & 18 deletions examples/qml_features/rust/src/containers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub mod ffi {
pub(crate) list: QList_i32,
// Expose as a Q_PROPERTY so that QML tests can ensure that QVariantMap works
#[qproperty]
map: QMap_QString_QVariant,
pub(crate) map: QMap_QString_QVariant,
pub(crate) set: QSet_i32,
pub(crate) vector: QVector_i32,
}
Expand Down Expand Up @@ -86,6 +86,7 @@ pub mod ffi {
}

use core::pin::Pin;
use cxx_qt::CxxQtType;
use cxx_qt_lib::{
QHash, QHashPair_QString_QVariant, QList, QMap, QMapPair_QString_QVariant, QSet, QString,
QVariant, QVector,
Expand All @@ -96,41 +97,44 @@ use cxx_qt_lib::{
impl ffi::RustContainersQt {
/// Reset all the containers
fn reset(mut self: Pin<&mut Self>) {
self.as_mut()
.set_hash(QHash::<QHashPair_QString_QVariant>::default());
self.as_mut().set_list(QList::<i32>::default());
self.as_mut()
.set_map(QMap::<QMapPair_QString_QVariant>::default());
self.as_mut().set_set(QSet::<i32>::default());
self.as_mut().set_vector(QVector::<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::<QHashPair_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::<QMapPair_QString_QVariant>::default());

self.update_strings();
}

/// Append the given number to the vector container
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
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
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
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 @@ -139,7 +143,7 @@ impl ffi::RustContainersQt {
fn insert_map(mut self: Pin<&mut Self>, key: QString, value: QVariant) {
// SAFETY: map is a Q_PROPERTY so ensure we manually trigger changed
unsafe {
self.as_mut().map_mut().insert(key, value);
self.as_mut().rust_mut().map.insert(key, value);
self.as_mut().map_changed();
}

Expand All @@ -149,7 +153,8 @@ impl ffi::RustContainersQt {
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 @@ -161,7 +166,8 @@ impl ffi::RustContainersQt {

let list_items = self
.as_ref()
.list()
.rust()
.list
.iter()
.map(|value| value.to_string())
.collect::<Vec<String>>()
Expand All @@ -170,7 +176,8 @@ impl ffi::RustContainersQt {

let map_items = self
.as_ref()
.map()
.rust()
.map
.iter()
.map(|(key, value)| {
let value = value.value::<i32>().unwrap_or(0);
Expand All @@ -182,7 +189,8 @@ impl ffi::RustContainersQt {

let set_items = self
.as_ref()
.set()
.rust()
.set
.iter()
.map(|value| value.to_string())
.collect::<Vec<String>>()
Expand All @@ -191,7 +199,8 @@ impl ffi::RustContainersQt {

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 @@ -207,16 +207,21 @@ impl ffi::CustomBaseClassQt {
}

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();
}
}

/// 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_clear
Expand All @@ -225,7 +230,7 @@ impl ffi::CustomBaseClassQt {
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 @@ -250,7 +255,7 @@ impl ffi::CustomBaseClassQt {

/// Remove the row with the given index
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 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 @@ -76,9 +76,10 @@ impl ffi::RustInvokablesQt {

/// Mutable C++ context method that helps to store the color
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_invokable_impl
Expand Down
7 changes: 4 additions & 3 deletions examples/qml_features/rust/src/signals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ pub mod ffi {
}

use core::pin::Pin;
use cxx_qt::CxxQtType;
use cxx_qt_lib::{ConnectionType, QString, QUrl};

// TODO: this will change to qobject::RustSignals once
Expand Down Expand Up @@ -92,7 +93,7 @@ impl ffi::RustSignalsQt {
// 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 @@ -109,15 +110,15 @@ impl ffi::RustSignalsQt {
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
11 changes: 6 additions & 5 deletions tests/basic_cxx_qt/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,24 +84,25 @@ impl ffi::MyObjectQt {
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();
}

fn queue_test_multi_thread(self: Pin<&mut Self>) {
static N_THREADS: usize = 100;
static N_REQUESTS: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0);
static N_REQUESTS: std::sync::atomic::AtomicUsize =
std::sync::atomic::AtomicUsize::new(0);

let mut handles = Vec::new();
let qt_thread = self.qt_thread();
for _ in 0..N_THREADS {
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
Loading

0 comments on commit 1e1fd8d

Please sign in to comment.