Skip to content

Fix: Constructors no longer fail silently #1295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Fix: Constructors no longer fail silently #1295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion book/src/bridge/extern_rustqt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ is equivalent to writing
impl cxx_qt::Constructor<()> for x {}
```

inside the bridge.
inside the bridge. You can then implement your constructors outside the bridge, using either of these traits.

For further documentation see the [traits page](./traits.md).

Expand Down
2 changes: 1 addition & 1 deletion book/src/bridge/traits.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ For further documentation, refer to the documentation of the individual traits:

- [CxxQtType](https://docs.rs/cxx-qt/latest/cxx_qt/trait.CxxQtType.html) - trait to reach the Rust implementation of a `QObject`
- This trait is automatically implemented for any `#[qobject]` type inside `extern "RustQt"` blocks.
- [Constructor](https://docs.rs/cxx-qt/latest/cxx_qt/trait.Constructor.html) - custom constructor
- [Constructor](https://docs.rs/cxx-qt/latest/cxx_qt/trait.Constructor.html) - custom constructor. This must be declared in the bridge in order for you to implement it outside the bridge
- [Initialize](https://docs.rs/cxx-qt/latest/cxx_qt/trait.Initialize.html) - execute Rust code when the object is constructed, or as shorthand for an empty constructor
- [Threading](https://docs.rs/cxx-qt/latest/cxx_qt/trait.Threading.html) - marker trait whether CXX-Qt threading should be enabled

Expand Down
27 changes: 21 additions & 6 deletions crates/cxx-qt-gen/src/generator/rust/constructor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,13 @@ pub fn generate(

let rust_struct_name_rust = qobject_names.rust_struct.rust_unqualified();

result
.cxx_qt_mod_contents
.append(&mut vec![parse_quote_spanned! {
qobject_name_rust.span() => // TODO! Improve this span
impl ::cxx_qt::ConstructorDeclared for #qobject_name_rust_qualified {}
}]);

for (index, constructor) in constructors.iter().enumerate() {
let lifetime = constructor.lifetime.as_ref().map(|lifetime| {
quote! {
Expand Down Expand Up @@ -599,8 +606,16 @@ mod tests {
},
);

// Shim impl only appears once
assert_tokens_eq(
&blocks.cxx_qt_mod_contents[0],
quote! {
impl ::cxx_qt::ConstructorDeclared for qobject::MyObject {}
},
);

assert_tokens_eq(
&blocks.cxx_qt_mod_contents[1],
quote! {
#[doc(hidden)]
pub fn route_arguments_MyObject_0() -> qobject::CxxQtConstructorArgumentsMyObject0
Expand All @@ -619,7 +634,7 @@ mod tests {
},
);
assert_tokens_eq(
&blocks.cxx_qt_mod_contents[1],
&blocks.cxx_qt_mod_contents[2],
quote! {
#[doc(hidden)]
#[allow(unused_variables)]
Expand All @@ -633,7 +648,7 @@ mod tests {
},
);
assert_tokens_eq(
&blocks.cxx_qt_mod_contents[2],
&blocks.cxx_qt_mod_contents[3],
quote! {
#[doc(hidden)]
#[allow(unused_variables)]
Expand Down Expand Up @@ -725,7 +740,7 @@ mod tests {
);

assert_tokens_eq(
&blocks.cxx_qt_mod_contents[3],
&blocks.cxx_qt_mod_contents[4],
quote! {
#[doc(hidden)]
pub fn route_arguments_MyObject_1<'lifetime>(arg0: *const QObject) -> qobject::CxxQtConstructorArgumentsMyObject1<'lifetime>
Expand Down Expand Up @@ -753,7 +768,7 @@ mod tests {
},
);
assert_tokens_eq(
&blocks.cxx_qt_mod_contents[4],
&blocks.cxx_qt_mod_contents[5],
quote! {
#[doc(hidden)]
#[allow(unused_variables)]
Expand All @@ -767,7 +782,7 @@ mod tests {
},
);
assert_tokens_eq(
&blocks.cxx_qt_mod_contents[5],
&blocks.cxx_qt_mod_contents[6],
quote! {
#[doc(hidden)]
#[allow(unused_variables)]
Expand Down Expand Up @@ -806,7 +821,7 @@ mod tests {
]);

assert_eq!(blocks.cxx_mod_contents.len(), 10);
assert_eq!(blocks.cxx_qt_mod_contents.len(), 6);
assert_eq!(blocks.cxx_qt_mod_contents.len(), 7);

let namespace_attr = quote! {
#[namespace = "qobject::cxx_qt_MyObject"]
Expand Down
1 change: 1 addition & 0 deletions crates/cxx-qt-gen/test_outputs/invokables.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ unsafe impl ::cxx_qt::casting::Upcast<::cxx_qt::QObject> for ffi::MyObject {
ffi::cxx_qt_ffi_MyObject_downcastPtr(base)
}
}
impl ::cxx_qt::ConstructorDeclared for ffi::MyObject {}
#[doc(hidden)]
pub fn route_arguments_MyObject_0<'a>(
arg0: i32,
Expand Down
7 changes: 5 additions & 2 deletions crates/cxx-qt/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@ pub trait Threading: Sized {
fn threading_drop(cxx_qt_thread: &mut CxxQtThread<Self>);
}

#[doc(hidden)]
pub trait ConstructorDeclared {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This trait will need the same generics and type arguments as the constructor trait, so that we can make sure you actually declare the correct constructors inside the bridge.

Otherwise you could do stuff like:

#[cxx_qt::bridge]
mod qobject {
    // implements ConstructorDeclared
    impl cxx_qt::Constructor<(i32,)> for MyObject {}
}

impl cxx_qt::Constructor<(i32,)> for MyObject {
   // ...
}

// Additional constructor declared
// No compiler error!
impl cxx_qt::Constructor<()> for MyObject {
  // ...
}

and the trait bound would not catch it.


/// This trait can be implemented on any [CxxQtType] to define a
/// custom constructor in C++ for the QObject.
///
Expand Down Expand Up @@ -307,7 +310,7 @@ pub trait Threading: Sized {
///
/// If a QObject implements the `Initialize` trait, and the inner Rust struct is [Default]-constructible it will automatically implement `cxx_qt::Constructor<()>`.
/// Additionally, implementing `impl cxx_qt::Initialize` will act as shorthand for `cxx_qt::Constructor<()>`.
pub trait Constructor<Arguments>: CxxQtType {
pub trait Constructor<Arguments>: CxxQtType + ConstructorDeclared {
/// The arguments that are passed to the [`new()`](Self::new) function to construct the inner Rust struct.
/// This must be a tuple of CXX compatible types.
///
Expand Down Expand Up @@ -394,7 +397,7 @@ pub trait Constructor<Arguments>: CxxQtType {
/// ```
// TODO: Once the QObject type is available in the cxx-qt crate, also auto-generate a default
// constructor that takes QObject and passes it to the parent.
pub trait Initialize: CxxQtType {
pub trait Initialize: CxxQtType + ConstructorDeclared {
/// This function is called to initialize the QObject after construction.
fn initialize(self: core::pin::Pin<&mut Self>);
}
Expand Down
Loading