Record metadata for a struct implementing a trait. #2204
Conversation
mhammond
force-pushed
the
push-yolpttzuvznp
branch
3 times, most recently
from
5411e96 to
fa42fd1
Compare
|
Bindings all turned out to be fairly easy - which in some ways is "bad" because it highlights an inefficiency here, but good because it seems fine and useful. |
mhammond
force-pushed
the
push-yolpttzuvznp
branch
2 times, most recently
from
d177e59 to
7163779
Compare
mhammond
force-pushed
the
push-yolpttzuvznp
branch
3 times, most recently
from
317c1dd to
8fbccb5
Compare
|
The other languages actually aren't working :) So just Python for now. |
mhammond
force-pushed
the
push-yolpttzuvznp
branch
from
8fbccb5 to
ba7bd54
Compare
There was a problem hiding this comment.
Looks great and I'm pretty amazed how easy it was. I just noticed a couple minor things.
I think this is a breaking change since the metadata format is going to change right? If so, please bump UNIFFI_CONTRACT_VERSION and let's wait to merge until I can get a 0.28.2 release out.
uniffi_meta/src/metadata.rs
Outdated
| @@ -23,7 +23,7 @@ pub mod codes { | |||
| pub const UNIFFI_TRAIT: u8 = 11; | |||
| pub const TRAIT_INTERFACE: u8 = 12; | |||
| pub const CALLBACK_TRAIT_INTERFACE: u8 = 13; | |||
| //pub const UNKNOWN: u8 = 255; | |||
| pub const AS_TRAIT: u8 = 14; | |||
There was a problem hiding this comment.
AS_TRAIT doesn't communicate what this is doing to me, I keep thinking of AsRef<T> and expecting some sort of casting to be happening. That's essentially what happens when you use the object as a trait, but here we're just tracking the list. What about something like TRAIT_IMPLS?
| @@ -8,7 +8,7 @@ | |||
| {% if ci.is_name_used_as_error(name) %} | |||
| class {{ impl_name }}(Exception): | |||
| {%- else %} | |||
| class {{ impl_name }}: | |||
| class {{ impl_name }}({% for t in obj.as_trait_impls() %}{{ t.trait_name|class_name }},{% endfor %}): | |||
There was a problem hiding this comment.
🤯 it's amazing that this is all we need to support this feature.
I'm a little unclear on this - I don't think it changes any existing metadata item formats, but does add a new one - does that qualify as needing this? I'd have though we expect consumers to ignore what they don't understand? |
I'm not clear either, but I think this line will fail and that will bubble up to make the entire metadata parsing step fail. I could definitely be wrong about that, but I think that's how it works. I agree that it makes more sense to ignore things you don't understand. |
|
I see, thanks for the explanation! |
mhammond
force-pushed
the
push-yolpttzuvznp
branch
2 times, most recently
from
0fdf117 to
fbe3026
Compare
```
#[uniffi::export]
impl MyTrait for MyObject { ... }
```
Previously worked as it ignored `MyTrait`. This adds new metadata to record it,
allowing foreign bindings to implement things like inheritance.
Includes Python generating an inheritance chain to reflect this relationship.
This will not generate correct code if a struct declares more than 1 trait,
and there's some undesirable re-wrapping when traits from these objects gets
passed back and forward, but seems to work surprisingly well.
Fixes mozilla#2196.
mhammond
force-pushed
the
push-yolpttzuvznp
branch
from
fbe3026 to
58df528
Compare
Builds on mozilla#2204 which landed the metadata and Python support.
Builds on mozilla#2204 which landed the metadata and Python support.
Builds on mozilla#2204 which landed the metadata and Python support. Also fixes minor issues with `CallbackInterface` types.
Builds on #2204 which landed the metadata and Python support. Also fixes minor issues with `CallbackInterface` types.
Builds on mozilla#2196, mozilla#2204 and mozilla#2297. Closes mozilla#2169.
Builds on mozilla#2196, mozilla#2204 and mozilla#2297. Closes mozilla#2169.
Record metadata for a struct implementing a trait.
Previously worked as it ignored
MyTrait. This adds new metadata to record it,allowing foreign bindings to implement things like inheritance.
Includes Python generating an inheritance chain to reflect this relationship.
This will not generate correct code if a struct declares more than 1 trait,
and there's some undesirable re-wrapping when traits from these objects gets
passed back and forward, but seems to work surprisingly well.
On the path to #2196.