KDAB / cxx-qt

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

No errors/warnings are thrown when using wrong attributes. #965

Open mystchonky opened 1 month ago

mystchonky commented 1 month ago

When using a wrong attribute macro inside an extern block, there are no errors or warning. It would help a lot to have some sort of compile time sanitation to avoid user error.

Context: I spent a while to figure out why my properties were not exposed to QML when the error was using #[qml_property] instead of #[qproperty]

ahayzen-kdab commented 1 month ago

This is likely linked to our qobject tagged type item, we likely aren't passing through attributes like we do for other places like methods ? But we should also check that the same is working for methods too etc.

LeonMatthesKDAB commented 1 month ago

@ahayzen-kdab This is a problem I've not yet found a very clean way to handle in our parser/generator.

For most of the types we parse, there's a predefined list of attributes that we allow, like cxx_name, rust_name, namespace, base etc. So we'll probably want to add checks that only the expected attributes are added, and nothing more.

Maybe we should just list all allowed properties every time we parse and check that no other exist? It's somewhat of a cross-cutting concern though, as then that check would need to know every possible attribute that e.g. the naming later uses, which is not ideal... Something for a "checks" phase maybe?

ahayzen-kdab commented 1 month ago

@LeonMatthesKDAB right sounds like something for the checks phase. However for other things i thought was pass through unknown attributes?

eg #[my_attribute] here on a signal is passed through https://github.com/KDAB/cxx-qt/blob/309190a199f61a2406c541dc17cdd73c5e5b296e/crates/cxx-qt-gen/test_inputs/passthrough_and_naming.rs#L140-L143

As we can see in the test outputs https://github.com/KDAB/cxx-qt/blob/309190a199f61a2406c541dc17cdd73c5e5b296e/crates/cxx-qt-gen/test_outputs/passthrough_and_naming.rs#L264-L268

I assume we are doing this for all methods ? (invokables, signals, overrrides etc). But we should also do it for attributes on the type T which seems to be missing at the moment eg

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

    extern "RustQt" {
        #[my_attribute]
        #[qobject]
        type MyObject = super::MyObjectRust;
    }

should probably generate the following?

    unsafe extern "C++" {
        #[my_attribute]
        type QPushButton;
    }

    unsafe extern "C++" {
        #[my_attribute]
        type MyObject;
    }
    extern "Rust" {
        type MyObjectRust;
    }

If they did this then at least any unknown attributes would fail to compile in Rust rather than silently failing ?

As part of your refactoring we should have a map available of the original attributes for a token so then we should be able to tag the original attributes with some filters of known attributes?

LeonMatthesKDAB commented 1 month ago

Well that's really the question, whether we want to pass anything unknown through to CXX, or whether we throw an error...

Not so sure about that, but either way we need to figure out which attributes we've computed ourselves and which remain to be passed through/errored on.