KDAB / cxx-qt

Safe interop between Rust and Qt
https://kdab.github.io/cxx-qt/book/
1.02k stars 67 forks source link

virtual void function overrides in rust #819

Closed knoxfighter closed 7 months ago

knoxfighter commented 7 months ago

Given the following C++ code, where extern.h is not changeable:

extern.h

class CustomWidget : public QWidget
{
  Q_OBJECT

  ...
}

class CustomWidgetFactory {
public:
  virtual CustomWidget* createWidget() = 0;
}

cpp_example.h

class MyCustomWidget : public CustomWidget {
  Q_OBJECT

  ...
}

class MyCustomWidgetFactory : public CustomWidgetFactory {
  CustomWidget* createWidget() override;
}

cpp_example.cpp

CustomWidget* MyCustomWidgetFactory::createWidget() {
  return new MyCustomWidget();
}

How do i transcribe and translate the cpp_example.h/cpp to rust? Creating "MyCustomWidget" should be easy by following the inheritance example. The Factory on the other hand is a mystery to me.

ahayzen-kdab commented 7 months ago

Pointers and constructors are slightly tricky with CXX at the moment ...

This would look like

mod ffi {
    extern "C++ {
        include!("extern.h");
        type CustomWidget;
    }

    extern "RustQt" {
        #[qobject]
        #[base = "CustomWidgetFactory"]
        type MyCustomWidgetFactory = super::MyCustomWidgetFactoryRust;
    }

    unsafe extern "RustQt" {
        #[cxx_override]
        #[qinvokable]
        unsafe fn createWidget(self: &MyCustomWidgetFactory -> *mut CustomWidget;
    }
...

Then you will need a C++ function to construct the CustomWidget as CXX doesn't support static methods / constructors etc (we might add a template into cxx-qt-lib so that it can be reused in the future or even generate a new_unique_ptr / new_ptr methods to create C++ instances from Rust for types @LeonMatthesKDAB another reason for this ).

CustomWidget* createCustomWidgetPtr()
{
   return new CustomWidget;
}

and then

extern "C++" {
   include!("helper.h");
   unsafe fn createCustomWidgetPtr() -> *mut CustomWidget;
}

Then you can implement the method

impl ffi::MyCustomWidgetFactory {
    fn createWidget(self: &MyCustomWidgetFactory -> *mut CustomWidget {
        ffi::createCustomWidgetPtr()
    }
}

However you will need to either set the parent of the widget or use things like UniquePtr and SharedPtr to ensure you don't leak memory etc.

knoxfighter commented 7 months ago

Thank you, unfortunately this does not work. You used #[qobject] on the MyCustomWidgetFactory class. This assumes that the MyCustomWidgetFactory inherits from QObject but the Factory is its own class and has no parents.

Therefore building the code results in the following error:

[...]/out/cxx-qt-gen/ffi.cxxqt.h(49): error C2338: static_assert failed: 'MyCustomWidgetFactory must inherit from QObject'
[...]/out/cxx-qt-gen/ffi.cxxqt.h(49): error C2338: static_assert failed: 'MyCustomWidgetFactory must inherit from QObject'
[...]/out/moc_ffi.cxxqt.h.cpp(160): error C2039: 'staticMetaObject': is not a member of 'CustomWidgetFactory'
[...]/extern.h(129): note: see declaration of 'CustomWidgetFactory'
[...]/out/moc_ffi.cxxqt.h.cpp(159): error C2737: 'staticMetaObject': const object must be initialized
[...]/out/moc_ffi.cxxqt.h.cpp(194): error C2039: 'qt_metacast': is not a member of 'CustomWidgetFactory'
[...]/extern.h(129): note: see declaration of 'CustomWidgetFactory'
[...]/out/moc_ffi.cxxqt.h.cpp(199): error C2039: 'qt_metacall': is not a member of 'CustomWidgetFactory'
[...]/extern.h(129): note: see declaration of 'CustomWidgetFactory'
[...]/out/cxx-qt-gen/ffi.cxxqt.h(49): error C2338: static_assert failed: 'MyCustomWidgetFactory must inherit from QObject'
[...]/out/cxx-qt-gen/src/ffi.cxxqt.cpp(20): error C2665: 'CustomWidgetFactory::CustomWidgetFactory': no overloaded function could convert all the argument types
[...]/extern.h(147): note: could be 'CustomWidgetFactory::CustomWidgetFactory(const CustomWidgetFactory &)'
[...]/out/cxx-qt-gen/src/ffi.cxxqt.cpp(20): note: 'CustomWidgetFactory::CustomWidgetFactory(const CustomWidgetFactory &)': cannot convert argument 1 from 'QObject *' to 'const CustomWidgetFactory &'
[...]/out/cxx-qt-gen/src/ffi.cxxqt.cpp(20): note: Reason: cannot convert from 'QObject *' to 'const CustomWidgetFactory'
[...]/out/cxx-qt-gen/src/ffi.cxxqt.cpp(20): note: while trying to match the argument list '(QObject *)'
ahayzen-kdab commented 7 months ago

Ah i missed that, then you'll need to use the patch here https://github.com/KDAB/cxx-qt/pull/767 that allows for still declaring a extern "RustQt" type with the override attributes on functions but allows you to skip having #[qobject] and have a bare class instead :-)

LeonMatthesKDAB commented 7 months ago

Hi @knoxfighter , thanks for reporting this issue. As @ahayzen-kdab mentioned, we've hit similar problems already and the draft he linked is a good workaround for now.

You should be aware that we're still uncertain whether we want to support defining normal classes in an extern "RustQt" block. The goal of CXX-Qt is mainly to extend CXX to support QObjects. For this we need inheritance support, as Qt heavily relies on that. However, it's debatable whether support for inheritance without QObjects shouldn't rather be part of CXX, instead of CXX-Qt. So you probably shouldn't rely on this draft PR making it into a stable release.

@knoxfighter Would it be acceptable for you to simply create the MyCustomWidgetFactory in C++, rather than Rust? I know it's not ideal, but we expect to not be able to support every C++ feature in CXX-Qt. Or would that defeat the purpose of using CXX-Qt for your use case? Keep in mind that it should be possible to still define MyCustomWidget entirely in Rust :)

ahayzen-kdab commented 7 months ago

@LeonMatthesKDAB note the place where I hit this is when wanting to implement a QQuickFramebufferObject::Renderer in Rust which is a Qt class but without a QObject :-) https://doc.qt.io/qt-6/qquickframebufferobject-renderer.html It is still valuable to have our cxx_override mechanism that would be tricky (impossible?) to implement in CXX without major changes due to our duality of classes :-)

So i think this is very useful and also allows us to add support for things like QGadget in the future (#352) which isn't a QObject.

tl;dr; my vote goes with this being super useful :-)

knoxfighter commented 7 months ago

Thank you very much for all your help. I managed to make it work how i want to.

@LeonMatthesKDAB In theory, yes, i could write the classes in C++, but my goal is to have as much as possible in rust. I also think that you are right that the inheritance should be part of CXX directly, but as long as that is not the case i will patch in the PR. I also vote for new_unique_ptr / new_ptr in CXX-Qt, so i can get rid of the rest C++.

This issue can be closed from my side.

Have a great time knox

greaka commented 7 months ago

I think it's worth converting this idea to an issue. It would cut down on some cpp that I still have to manually write and would like to get rid of.

LeonMatthesKDAB commented 7 months ago

Thanks for the feedback :)

I've extracted this into two separate issues.

@knoxfighter , make_unique is actually already in cxx-qt-lib/common.h, but not yet released (coming with 0.7 at the latest). You could use our main branch directly if you want to use it right away :)