cxx_name on methods

[knoxfighter]

The API i work with doesn't follow Qt codestyle. Therefore it contains a function named OnViewChange (with a capital first letter, aka. PascalCase). cxx-qt does not support cxx_name attributes to rename such functions, they are always formatted to CamelCase.

I wrote this as another example for #667

[LeonMatthesKDAB]

Hi, thank you for taking the time to report this issue.

Could you provide a small code snippet that shows how you're declaring the OnViewChange method with CXX-Qt?
Is it in extern "C++Qt" or extern "RustQt", a #[qinvokable], #[cxx_override]?
Maybe I can provide you with a workaround for now 😅

We know there are issues with (re-)naming items in CXX-Qt. That's definitely an area of improvement for the next release.
It's loosely tracked as part of #665

[knoxfighter]

It is part of a rust override.

The C++ API:

class UIContextNotification {
    ...
    virtual void OnViewChange(UIContext* context, ViewFrame* frame, const QString& type){}
    ...
}

My rust override:

unsafe extern "RustQt" {
    #[base = "UIContextNotification"]
    type MyUIContextNotification = super::MyUIContextNotificationRust;
}

unsafe extern "RustQt" {
    #[cxx_name = "OnViewChange"] // this is ignored and `onViewChange` is used instead
    #[cxx_override]
    unsafe fn OnViewChange(
        self: Pin<&mut MyUIContextNotification>,
        context: *mut UIContext,
        frame: *mut ViewFrame,
        type_: &QString,
    );
}

which results in the compiler error:

ffi_ui_notifications.cxxqt.h(20): error C3668: 'MyUIContextNotification::onViewChange': method with override specifier 'override' did not override any base class methods

[LeonMatthesKDAB]

I'm just looking through the code.
This seems to indeed be no longer possible to do at all 🙈
The camel case conversion is always used.

I think this entirely broke during the conversion to our new API for 0.6...
We'll definitely have to fix this.

Sorry there's no workaround at the moment.

[ahayzen-kdab]

Right this is an area we need to refactor, hopefully for 0.7.

For a workaround maybe you can use an alias in C++ ? Eg create a derived class with a camel case name and then use that in Rust.

The changes in #667 were an early attempt at trying to improve the problem, we were also trying to figure out if by default we still want the automatic camel <-> snake conversion (which is useful for things like QML signals as otherwise you potentially end up with my_property becoming onMy_propertyChanged).

I think before when we discussed the options we decided this was a possible route

extern	no attributes	cxx_name / rust_name
RustQt	C++: camelCase, Rust: original	C++: cxx_name / original, Rust: rust_name / original
C++Qt	C++: original, Rust: snake_case ?	C++: cxx_name / original, Rust: rust_name / original

I am unsure about C++Qt -> no attributes -> Rust case though, maybe only the automatic conversion happens in RustQt. @LeonMatthesKDAB what do you think ?

[LeonMatthes]

Hm, I actually quite like your proposal, including snake_case Conversion in C++Qt...
As then you'd always just write exactly what is used in the "language of origin", and it would still look native in the other language.

And with the table you provided, it should be easy enough to explain.

[ahayzen-kdab]

Hm, I actually quite like your proposal, including snake_case Conversion in C++Qt... As then you'd always just write exactly what is used in the "language of origin", and it would still look native in the other language.

And with the table you provided, it should be easy enough to explain.

Right that was my though, it's either "language of origin" or if you specify a cxx/rust_name then it follows those or the original. This should allow you to get to all combinations i think 🤔

[LeonMatthesKDAB]

As of #1005, you should now be able to use cxx_name and rust_name on methods again 🥳 .

I think we may still be missing the correct case conversions in the extern "C++" blocks, as described here #828 (comment) , but otherwise we're making good progress.

[LeonMatthesKDAB]

We've decided to drop the automatic renaming for 0.7.
There is the option to add an explicit auto_case_convert attribute to the bridge in the future to re-add this.
But for now, this issue is closed.