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.

Sign up for GitHub

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

Jump to bottom

Support foreign implements of trait interfaces #1578

Closed
bendk opened this issue Jun 2, 2023 · 1 comment
Closed

Support foreign implements of trait interfaces #1578

bendk opened this issue Jun 2, 2023 · 1 comment
Labels
uniffi

Comments

@bendk
Copy link
Contributor

bendk commented Jun 2, 2023 •
edited
Loading

We recently added support for trait interfaces (AKA sending Box from Rust to the foreign code). We should extend that code to also allow the foreign code to pass classes that implement the interface to APIs that expect a trait object. In the manual example, that means if they define a ForeignButton class that implements Button, they can pass that to press()..

This would create some nice symmetry and combine well with #1576.

It also addresses a real use-case we have in application-services. We have Rust sync engines for our components, but for historical reasons we also have sync engines defined on the foreign side and don't want to transition away right now. It would be really great if we could make it so SyncManager could seamlessly work with both types of engines at once.

┆Issue is synchronized with this Jira Task
┆Issue Number: UNIFFI-277
@bendk
Copy link
Contributor Author

bendk commented Jun 2, 2023

The main question I have about this is if we should give some support for using concrete types rather than always using type-erased versions. The main issue I see is when Rust passes a foreign object back to the foreign code. In this case, it might be nice for UniFFI to automagically convert it back to the concrete object, rather than passing back some sort of type-erased trait object. This eliminates a lot of overhead, since you can directly call methods on the object. With a trait object, you'd have to lower/lift the arguments across the FFI to pass them to the scaffolding function, then the scaffolding function would lower/lift them again to call into the callback interface.

I think that case could be handled if UniFFI inserted a uniffi_lower method into the trait. The default implementation would do what we currently do and create a Box<Arc<dyn trait>> and pass that over the FFI. But for the type that we create for callback interfaces, we would pass something different back across the FFI. Then the foreign code could distinguish between the 2 cases and sometimes recover the concrete object.

This sounds like more complexity than it's worth to me, but I wanted to mention for completeness.

Another way to address this would be to create an actual vtable for the trait interface. Foreign objects would use a vtable would have functions on the foreign side. This avoids the round-tripping issue that I mentioned above, although I think you would still need to lower/lift the arguments once.

bendk added a commit to bendk/uniffi-rs that referenced this issue Jun 13, 2023
@bendk
Use TraitMethodMetadata for callback interface methods
f44e9d0 
This brings the concepts closer together and should make it easier to
allow trait interfaces to be created from foreign objects (mozilla#1578).

- Refactored `get_or_insert_object()` to just be `get_object()`.
  Ordered the metadata enum so that object definitions always come
  before methods.
- Removed the `Result` return from `add_known_type()`, since it couldn't
  fail anyways.
bendk added a commit to bendk/uniffi-rs that referenced this issue Jun 14, 2023
@bendk
Use TraitMethodMetadata for callback interface methods
ba28e9d 
This brings the concepts closer together and should make it easier to
allow trait interfaces to be created from foreign objects (mozilla#1578).

- Refactored `get_or_insert_object()` to just be `get_object()`.
  Ordered the metadata enum so that object definitions always come
  before methods.
- Removed the `Result` return from `add_known_type()`, since it couldn't
  fail anyways.
@bendk bendk mentioned this issue Sep 5, 2023
Closed
@bendk bendk mentioned this issue Oct 10, 2023
Merged
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 11, 2023
@bendk
Make From<UnexpectedUniFFICallbackError> optional
7f18b82 
This is in preparation of mozilla#1578 -- allowing foreign languages to
implement traits.

One issue there is that if we want to allow foreign languages to
implement traits, then any Result return types need to implement
`LiftReturn`, and that impl currently has a bound on `E:
From<UnexpectedUniFFICallbackError`.  IOW, it's only implemented for
errors that can be converted from UnexpectedUniFFICallbackError.

This makes sense when the trait is actually used by the foreign
languages, however not all traits are intended for that use.  For that
matter, some libraries may be fine panicking in the the face of
unexpected callback errors.

By using the auto-ref specialization hack, we're able to convert the
error if there`s a `From<UnexpectedUniFFICallbackError>` impl, but use a
panicking version if not.  The trait bound is no longer required to
implement `LiftReturn`.
@bendk bendk mentioned this issue Oct 11, 2023
Merged
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 11, 2023
@bendk
Make From<UnexpectedUniFFICallbackError> optional
4ec2ec4 
This is in preparation of mozilla#1578 -- allowing foreign languages to
implement traits.

One issue there is that if we want to allow foreign languages to
implement traits, then any Result return types need to implement
`LiftReturn`, and that impl currently has a bound on `E:
From<UnexpectedUniFFICallbackError`.  IOW, it's only implemented for
errors that can be converted from UnexpectedUniFFICallbackError.

This makes sense when the trait is actually used by the foreign
languages, however not all traits are intended for that use.  For that
matter, some libraries may be fine panicking in the the face of
unexpected callback errors.

By using the auto-ref specialization hack, we're able to convert the
error if there's a `From<UnexpectedUniFFICallbackError>` impl, but use a
panicking version if not.  The trait bound is no longer required to
implement `LiftReturn`.
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 11, 2023
@bendk
Make From<UnexpectedUniFFICallbackError> optional
4e8aa39 
This is in preparation of mozilla#1578 -- allowing foreign languages to
implement traits.

One issue there is that if we want to allow foreign languages to
implement traits, then any Result return types need to implement
`LiftReturn`, and that impl currently has a bound on `E:
From<UnexpectedUniFFICallbackError`.  IOW, it's only implemented for
errors that can be converted from UnexpectedUniFFICallbackError.

This makes sense when the trait is actually used by the foreign
languages, however not all traits are intended for that use.  For that
matter, some libraries may be fine panicking in the the face of
unexpected callback errors.

By using the auto-ref specialization hack, we're able to convert the
error if there's a `From<UnexpectedUniFFICallbackError>` impl, but use a
panicking version if not.  The trait bound is no longer required to
implement `LiftReturn`.
@bendk bendk changed the title Allow foreign objects to converted to trait interfaces Support foreign implements of trait interfaces Oct 12, 2023
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 12, 2023
@bendk
Make From<UnexpectedUniFFICallbackError> optional
87629ca 
This is in preparation of mozilla#1578 -- allowing foreign languages to
implement traits.

One issue there is that if we want to allow foreign languages to
implement traits, then any Result return types need to implement
`LiftReturn`, and that impl currently has a bound on `E:
From<UnexpectedUniFFICallbackError`.  IOW, it's only implemented for
errors that can be converted from UnexpectedUniFFICallbackError.

This makes sense when the trait is actually used by the foreign
languages, however not all traits are intended for that use.  For that
matter, some libraries may be fine panicking in the the face of
unexpected callback errors.

By using the auto-ref specialization hack, we're able to convert the
error if there's a `From<UnexpectedUniFFICallbackError>` impl, but use a
panicking version if not.  The trait bound is no longer required to
implement `LiftReturn`.
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 12, 2023
@bendk
Make From<UnexpectedUniFFICallbackError> optional
c71c4f1 
This is in preparation of mozilla#1578 -- allowing foreign languages to
implement traits.

One issue there is that if we want to allow foreign languages to
implement traits, then any Result return types need to implement
`LiftReturn`, and that impl currently has a bound on `E:
From<UnexpectedUniFFICallbackError`.  IOW, it's only implemented for
errors that can be converted from UnexpectedUniFFICallbackError.

This makes sense when the trait is actually used by the foreign
languages, however not all traits are intended for that use.  For that
matter, some libraries may be fine panicking in the the face of
unexpected callback errors.

By using the auto-ref specialization hack, we're able to convert the
error if there's a `From<UnexpectedUniFFICallbackError>` impl, but use a
panicking version if not.  The trait bound is no longer required to
implement `LiftReturn`.
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 12, 2023
@bendk
Support foreign implementations of trait interfaces (mozilla#1578)
ae89238 
Scaffolding:
* Generate a struct that implements the trait using a callback interface callback
* Make `try_lift` input a callback interface handle and create one of those structs.
* Don't use `try_lift` in the trait interface method scaffolding.
  `try_lift` expects to lift a callback handle, but scaffolding
  methods are called with a leaked object pointer.
* Removed the unused RustCallStatus param from the callback initialization function

Kotlin/Python/Swift:
* Refactored the callback interface code so it can also be used for trait interfaces.
* Changed the callback interface handle map code so that it doesn't
  try to re-use the handles. If an object is lowered twice, we now
  generate two different handles. This is required for trait
  interfaces, and I think it's also would be the right thing for
  callback interfaces if they could be passed back into the foreign
  language from Rust.
* Make `lower()` return a callback interface handle.
* Added some code to clarify how we generate the protocol and the
  implementation of that protocol for an object

Other:
* Trait interfaces are still not supported on Ruby.
* Updated the coverall bindings tests to test this.
* Updated the traits example, although there's definitely more room for improvement.

TODO:

I think a better handle solution (mozilla#1730) could help with a few things:

* We're currently wrapping the object every time it's passed across the
  FFI.  If the foreign code receives a trait object, then passes it back
  to Rust.  Rust now has a handle to the foreign impl and that foreign
  impl just calls back into Rust.  This can lead to some extremely
  inefficent FFI calls if an object is passed around enough.
* The way we're coercing between pointers, usize, and uint64 is
  probably wrong and at the very least extremely brittle.

There should be better tests for reference counts, but I'm waiting until
we address the handle issue to implement them.
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 12, 2023
@bendk
Support foreign implementations of trait interfaces (mozilla#1578)
66bc1f1 
Scaffolding:
* Generate a struct that implements the trait using a callback interface callback
* Make `try_lift` input a callback interface handle and create one of those structs.
* Don't use `try_lift` in the trait interface method scaffolding.
  `try_lift` expects to lift a callback handle, but scaffolding
  methods are called with a leaked object pointer.
* Removed the unused RustCallStatus param from the callback initialization function

Kotlin/Python/Swift:
* Refactored the callback interface code so it can also be used for trait interfaces.
* Changed the callback interface handle map code so that it doesn't
  try to re-use the handles. If an object is lowered twice, we now
  generate two different handles. This is required for trait
  interfaces, and I think it's also would be the right thing for
  callback interfaces if they could be passed back into the foreign
  language from Rust.
* Make `lower()` return a callback interface handle.
* Added some code to clarify how we generate the protocol and the
  implementation of that protocol for an object

Other:
* Trait interfaces are still not supported on Ruby.
* Updated the coverall bindings tests to test this.
* Updated the traits example, although there's definitely more room for improvement.

TODO:

I think a better handle solution (mozilla#1730) could help with a few things:

* We're currently wrapping the object every time it's passed across the
  FFI.  If the foreign code receives a trait object, then passes it back
  to Rust.  Rust now has a handle to the foreign impl and that foreign
  impl just calls back into Rust.  This can lead to some extremely
  inefficent FFI calls if an object is passed around enough.
* The way we're coercing between pointers, usize, and uint64 is
  probably wrong and at the very least extremely brittle.

There should be better tests for reference counts, but I'm waiting until
we address the handle issue to implement them.
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 12, 2023
@bendk
Support foreign implementations of trait interfaces (mozilla#1578)
1bfae98 
Scaffolding:
* Generate a struct that implements the trait using a callback interface callback
* Make `try_lift` input a callback interface handle and create one of those structs.
* Don't use `try_lift` in the trait interface method scaffolding.
  `try_lift` expects to lift a callback handle, but scaffolding
  methods are called with a leaked object pointer.
* Removed the unused RustCallStatus param from the callback initialization function

Kotlin/Python/Swift:
* Factored out the callback interface impl and interface/protocol
  templates so it can also be used for trait interfaces.
* Changed the callback interface handle map code so that it doesn't
  try to re-use the handles. If an object is lowered twice, we now
  generate two different handles. This is required for trait
  interfaces, and I think it's also would be the right thing for
  callback interfaces if they could be passed back into the foreign
  language from Rust.
* Make `lower()` return a callback interface handle.
* Added some code to clarify how we generate the protocol and the
  implementation of that protocol for an object

Other:
* Trait interfaces are still not supported on Ruby.
* Updated the coverall bindings tests to test this.
* Updated the traits example, although there's definitely more room for improvement.

TODO:

I think a better handle solution (mozilla#1730) could help with a few things:

* We're currently wrapping the object every time it's passed across the
  FFI.  If the foreign code receives a trait object, then passes it back
  to Rust.  Rust now has a handle to the foreign impl and that foreign
  impl just calls back into Rust.  This can lead to some extremely
  inefficent FFI calls if an object is passed around enough.
* The way we're coercing between pointers, usize, and uint64 is
  probably wrong and at the very least extremely brittle.

There should be better tests for reference counts, but I'm waiting until
we address the handle issue to implement them.
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 12, 2023
@bendk
Support foreign implementations of trait interfaces (mozilla#1578)
2353d5e 
Scaffolding:
* Generate a struct that implements the trait using a callback interface callback
* Make `try_lift` input a callback interface handle and create one of those structs.
* Don't use `try_lift` in the trait interface method scaffolding.
  `try_lift` expects to lift a callback handle, but scaffolding
  methods are called with a leaked object pointer.
* Removed the unused RustCallStatus param from the callback initialization function

Kotlin/Python/Swift:
* Factored out the callback interface impl and interface/protocol
  templates so it can also be used for trait interfaces.
* Changed the callback interface handle map code so that it doesn't
  try to re-use the handles. If an object is lowered twice, we now
  generate two different handles. This is required for trait
  interfaces, and I think it's also would be the right thing for
  callback interfaces if they could be passed back into the foreign
  language from Rust.
* Make `lower()` return a callback interface handle.
* Added some code to clarify how we generate the protocol and the
  implementation of that protocol for an object

Other:
* Trait interfaces are still not supported on Ruby.
* Updated the coverall bindings tests to test this.
* Updated the traits example, although there's definitely more room for improvement.

TODO:

I think a better handle solution (mozilla#1730) could help with a few things:

* We're currently wrapping the object every time it's passed across the
  FFI.  If the foreign code receives a trait object, then passes it back
  to Rust.  Rust now has a handle to the foreign impl and that foreign
  impl just calls back into Rust.  This can lead to some extremely
  inefficent FFI calls if an object is passed around enough.
* The way we're coercing between pointers, usize, and uint64 is
  probably wrong and at the very least extremely brittle.

There should be better tests for reference counts, but I'm waiting until
we address the handle issue to implement them.
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 12, 2023
@bendk
Support foreign implementations of trait interfaces (mozilla#1578)
7291c25 
Scaffolding:
* Generate a struct that implements the trait using a callback interface callback
* Make `try_lift` input a callback interface handle and create one of those structs.
* Don't use `try_lift` in the trait interface method scaffolding.
  `try_lift` expects to lift a callback handle, but scaffolding
  methods are called with a leaked object pointer.
* Removed the unused RustCallStatus param from the callback initialization function

Kotlin/Python/Swift:
* Factored out the callback interface impl and interface/protocol
  templates so it can also be used for trait interfaces.
* Changed the callback interface handle map code so that it doesn't
  try to re-use the handles. If an object is lowered twice, we now
  generate two different handles. This is required for trait
  interfaces, and I think it's also would be the right thing for
  callback interfaces if they could be passed back into the foreign
  language from Rust.
* Make `lower()` return a callback interface handle.
* Added some code to clarify how we generate the protocol and the
  implementation of that protocol for an object

Other:
* Trait interfaces are still not supported on Ruby.
* Updated the coverall bindings tests to test this.
* Updated the traits example, although there's definitely more room for improvement.

TODO:

I think a better handle solution (mozilla#1730) could help with a few things:

* We're currently wrapping the object every time it's passed across the
  FFI.  If the foreign code receives a trait object, then passes it back
  to Rust.  Rust now has a handle to the foreign impl and that foreign
  impl just calls back into Rust.  This can lead to some extremely
  inefficent FFI calls if an object is passed around enough.
* The way we're coercing between pointers, usize, and uint64 is
  probably wrong and at the very least extremely brittle.

There should be better tests for reference counts, but I'm waiting until
we address the handle issue to implement them.
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 13, 2023
@bendk
Make From<UnexpectedUniFFICallbackError> optional
19b44a7 
This is in preparation of mozilla#1578 -- allowing foreign languages to
implement traits.

One issue there is that if we want to allow foreign languages to
implement traits, then any Result return types need to implement
`LiftReturn`, and that impl currently has a bound on `E:
From<UnexpectedUniFFICallbackError`.  IOW, it's only implemented for
errors that can be converted from UnexpectedUniFFICallbackError.

This makes sense when the trait is actually used by the foreign
languages, however not all traits are intended for that use.  For that
matter, some libraries may be fine panicking in the the face of
unexpected callback errors.

By using the auto-ref specialization hack, we're able to convert the
error if there's a `From<UnexpectedUniFFICallbackError>` impl, but use a
panicking version if not.  The trait bound is no longer required to
implement `LiftReturn`.
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 13, 2023
@bendk
Make From<UnexpectedUniFFICallbackError> optional
3d58ae3 
This is in preparation of mozilla#1578 -- allowing foreign languages to
implement traits.

One issue there is that if we want to allow foreign languages to
implement traits, then any Result return types need to implement
`LiftReturn`, and that impl currently has a bound on `E:
From<UnexpectedUniFFICallbackError`.  IOW, it's only implemented for
errors that can be converted from UnexpectedUniFFICallbackError.

This makes sense when the trait is actually used by the foreign
languages, however not all traits are intended for that use.  For that
matter, some libraries may be fine panicking in the the face of
unexpected callback errors.

By using the auto-ref specialization hack, we're able to convert the
error if there's a `From<UnexpectedUniFFICallbackError>` impl, but use a
panicking version if not.  The trait bound is no longer required to
implement `LiftReturn`.
bendk added a commit that referenced this issue Oct 16, 2023
@bendk
Make From<UnexpectedUniFFICallbackError> optional
a3403ac 
This is in preparation of #1578 -- allowing foreign languages to
implement traits.

One issue there is that if we want to allow foreign languages to
implement traits, then any Result return types need to implement
`LiftReturn`, and that impl currently has a bound on `E:
From<UnexpectedUniFFICallbackError`.  IOW, it's only implemented for
errors that can be converted from UnexpectedUniFFICallbackError.

This makes sense when the trait is actually used by the foreign
languages, however not all traits are intended for that use.  For that
matter, some libraries may be fine panicking in the the face of
unexpected callback errors.

By using the auto-ref specialization hack, we're able to convert the
error if there's a `From<UnexpectedUniFFICallbackError>` impl, but use a
panicking version if not.  The trait bound is no longer required to
implement `LiftReturn`.
bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 18, 2023
@bendk
Support foreign implementations of trait interfaces (mozilla#1578)
cd194bd 
Scaffolding:
* Generate a struct that implements the trait using a callback interface callback
* Make `try_lift` input a callback interface handle and create one of those structs.
* Don't use `try_lift` in the trait interface method scaffolding.
  `try_lift` expects to lift a callback handle, but scaffolding
  methods are called with a leaked object pointer.
* Removed the unused RustCallStatus param from the callback initialization function

Kotlin/Python/Swift:
* Factored out the callback interface impl and interface/protocol
  templates so it can also be used for trait interfaces.
* Changed the callback interface handle map code so that it doesn't
  try to re-use the handles. If an object is lowered twice, we now
  generate two different handles. This is required for trait
  interfaces, and I think it's also would be the right thing for
  callback interfaces if they could be passed back into the foreign
  language from Rust.
* Make `lower()` return a callback interface handle.
* Added some code to clarify how we generate the protocol and the
  implementation of that protocol for an object

Other:
* Trait interfaces are still not supported on Ruby.
* Updated the coverall bindings tests to test this.
* Updated the traits example, although there's definitely more room for improvement.

TODO:

I think a better handle solution (mozilla#1730) could help with a few things:

* We're currently wrapping the object every time it's passed across the
  FFI.  If the foreign code receives a trait object, then passes it back
  to Rust.  Rust now has a handle to the foreign impl and that foreign
  impl just calls back into Rust.  This can lead to some extremely
  inefficent FFI calls if an object is passed around enough.
* The way we're coercing between pointers, usize, and uint64 is
  probably wrong and at the very least extremely brittle.

There should be better tests for reference counts, but I'm waiting until
we address the handle issue to implement them.
bendk added a commit that referenced this issue Oct 18, 2023
@bendk
Support foreign implementations of trait interfaces (#1578)
ff72041 
Scaffolding:
* Generate a struct that implements the trait using a callback interface callback
* Make `try_lift` input a callback interface handle and create one of those structs.
* Don't use `try_lift` in the trait interface method scaffolding.
  `try_lift` expects to lift a callback handle, but scaffolding
  methods are called with a leaked object pointer.
* Removed the unused RustCallStatus param from the callback initialization function

Kotlin/Python/Swift:
* Factored out the callback interface impl and interface/protocol
  templates so it can also be used for trait interfaces.
* Changed the callback interface handle map code so that it doesn't
  try to re-use the handles. If an object is lowered twice, we now
  generate two different handles. This is required for trait
  interfaces, and I think it's also would be the right thing for
  callback interfaces if they could be passed back into the foreign
  language from Rust.
* Make `lower()` return a callback interface handle.
* Added some code to clarify how we generate the protocol and the
  implementation of that protocol for an object

Other:
* Trait interfaces are still not supported on Ruby.
* Updated the coverall bindings tests to test this.
* Updated the traits example, although there's definitely more room for improvement.

TODO:

I think a better handle solution (#1730) could help with a few things:

* We're currently wrapping the object every time it's passed across the
  FFI.  If the foreign code receives a trait object, then passes it back
  to Rust.  Rust now has a handle to the foreign impl and that foreign
  impl just calls back into Rust.  This can lead to some extremely
  inefficent FFI calls if an object is passed around enough.
* The way we're coercing between pointers, usize, and uint64 is
  probably wrong and at the very least extremely brittle.

There should be better tests for reference counts, but I'm waiting until
we address the handle issue to implement them.
@bendk bendk closed this as completed Oct 19, 2023
@bendk bendk mentioned this issue Oct 19, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
uniffi
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bendk @data-sync-user