Skip to content
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.

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

Traits for CxxQtType/QtObject #562

Open
LeonMatthesKDAB opened this issue May 29, 2023 · 8 comments
Open

Traits for CxxQtType/QtObject #562

LeonMatthesKDAB opened this issue May 29, 2023 · 8 comments
Labels
⬆️ feature New feature or request 👷 refactor Something needs to change 🥳🎉 1.0 This issue is part of stabilization for 1.0 release

Comments

@LeonMatthesKDAB
Copy link
Collaborator

LeonMatthesKDAB commented May 29, 2023

We could move some of our "magic" methods into traits, which would make it a lot more obvious where they are coming from, and would allow us to e.g. "inherit" methods from QObject, reason about types, etc.

// cxx-qt crate - may also need to go in cxx-qt-lib
mod ffi {
    extern "C++" {
        type QObject;

        fn children(self: &QObject) -> QList<*mut QObject>;
        fn objectName(self: &QObject) -> QString;
    }
}

// Note: QtObject vs QObject
trait QtObject {
    fn as_qobject(&self) -> &cxx_qt_lib::QObject;
    fn as_qobject_mut(&mut self) -> Pin<&mut cxx_qt_lib::QObject>;

    fn children(&self) -> QList<*mut QObject> {
        as_qobject().children()
    }

    fn objectName(&self) -> Pin<&mut cxx_qt_lib::QObject> {
        as_qobject().objectName()
    }

    // ... other methods wrapped as needed.
}

trait CxxQtType {
    type Inner;

    fn rust(&self) -> &Self::Inner;
    fn rust_mut(self: Pin<&mut self>) -> &mut Self::Inner;
}

Then in C++, we could generate a generic cast to QObject, similar to how we do constructors.

template<typename T>
QObject *as_qobject(T* t) {
    return static_cast<QObject*>(t);
}
@LeonMatthesKDAB LeonMatthesKDAB added 🥳🎉 1.0 This issue is part of stabilization for 1.0 release ⬆️ feature New feature or request 👷 refactor Something needs to change labels May 29, 2023
@LeonMatthesKDAB LeonMatthesKDAB changed the title Traits for CxxQtObject/QtObject Traits for CxxQtType/QtObject May 29, 2023
@ahayzen-kdab
Copy link
Collaborator

I also wonder with this if we have an existing C++ QObject if we should allow for tagging it to get the trait implemented on it ?

Eg if we have

unsafe extern "C++Qt" {
   type MyQObject;
}

This won't have the QtObject trait implemented on it, but instead we could use the same syntax as RustQt to to do this

Eg

unsafe extern "C++Qt" {
   #[qobject] 
   type MyQObject;
}

@LeonMatthesKDAB
Copy link
Collaborator Author

Hm, shouldn't all classes in "C++Qt" be QObjects?
In that case we should just always implement the trait.

Though that brings up the question of whether all types within C++Qt are actually QObjects 🤔
Could they also be QGadgets? And would there be any use for it?

But maybe we should start requiring #[qobject] on every type in "C++Qt" to make sure the behavior is the same in C++Qt as in RustQt.

@LeonMatthesKDAB
Copy link
Collaborator Author

Also another suggestion regarding naming, we could use AsX here.
E.g. the trait could be named AsQObject.

@ahayzen-kdab
Copy link
Collaborator

Q_GADGET IIRC doesn't support Q_SIGNAL but does support things like Q_PROPERTY. As the only feature we effectively add with extern "C++Qt" is signals then it could make sense to assume all are deriving from QObject.

But I fear it's a slight inconsistency with extern "RustQt".

Maybe the #[qobject] attribute ends up meaning

  • Implement the AsQObject trait on this type
  • If we are generating code then this needs the Q_OBJECT macro
  • If we are generating code this then derives from QObject unless there is a base_class

@LeonMatthesKDAB
Copy link
Collaborator Author

LeonMatthesKDAB commented Feb 21, 2024

Note: We should potentially make use of Deref as well to allow for upcasting to the base class.

Once https://doc.rust-lang.org/std/pin/struct.Pin.html#method.as_deref_mut is no longer nightly only, we could even use DerefMut and upcast pointers easily that way.

Something along the lines of:

unsafe trait Inherits {
    // note using an associated type only allows for single-inheritance!
    type Base;

    fn upcast_ptr(self: *mut Self) -> *mut Base;

    fn upcast_mut(self: Pin<&mut Self>) -> Pin<&mut Base> {
       // provided by the trait
    }

    fn upcast<'a>(&'a self) -> &'a Base { }
}

// unsure whether this is possible, but would be awesome!
impl<T> Deref for T where T: Inherits {
    type Target = <T as Inherits>::Base;
    ...
}

@dphaldes
Copy link
Contributor

It would be nice if we could use From and Into traits. It will be consistent with how rust generally works.

@LeonMatthesKDAB
Copy link
Collaborator Author

LeonMatthesKDAB commented Apr 24, 2024

Right, I was hoping we could even get away without explicit Into calls by using Deref, but we'll have to see how this pans out.

@dphaldes
Copy link
Contributor

Rust is verbose and is one of its strong points. Automatic casting has caused problems for me in the past, which is why I would be against it. In my opinion, Rust's type inference powered object.into() syntax gives the best of automatic casting and is clear when the cast actually happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⬆️ feature New feature or request 👷 refactor Something needs to change 🥳🎉 1.0 This issue is part of stabilization for 1.0 release
Projects
None yet
Development

No branches or pull requests

3 participants