Foreign trait interfaces #1791
Merged
Foreign trait interfaces #1791
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bendk
force-pushed
the
foreign-trait-interfaces
branch
from
2353d5e to
7291c25
Compare
This is prep work for allowing the foreign bindings to implement traits. Moved trait interface code in `uniffi_macros` to its own module. Updated the test trait in the coverall fixture. I think this one will work better to test foreign trait impls. The `ancestor_names()` method is a good way to test trait implementations that bounce between the Rust and foreign side of the FFI. Added Getters test like we have with callback interfaces. We want to replace callback interfaces with trait interfaces, so we should have good test coverage. This actually revealed that the trait code didn't support exceptions yet.
The goal is to re-use this for trait interfaces
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
force-pushed
the
foreign-trait-interfaces
branch
from
7291c25 to
cd194bd
Compare
|
Looks like this one is maybe ready to merge. The code isn't ideal, but it's working. The only way it's worse than the current code is when when a Rust trait object is passed back and forth across the FFI, in which case it will be less efficient. We just released |
bendk
added a commit
to bendk/uniffi-rs
that referenced
this pull request
Oct 27, 2023
This was merged as part of mozilla#1791 just after the `0.25.0` release was made because we wanted to give that code a bit of time in main before releasing it. However, this part is useful for traits implemented in Rust.
bendk
added a commit
to bendk/uniffi-rs
that referenced
this pull request
Oct 27, 2023
This is part of mozilla#1791, which was merged just after the `0.25.0` release was made. We wanted to give that code a bit of time in main before releasing it. However, this part is useful for traits implemented in Rust.
bendk
added a commit
that referenced
this pull request
Oct 27, 2023
This is part of #1791, which was merged just after the `0.25.0` release was made. We wanted to give that code a bit of time in main before releasing it. However, this part is useful for traits implemented in Rust.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enables foreign code to implement trait interfaces. The code seems to be working for me, but there are some issues that should be resolved before this is merged:
FfiTypeis inefficient and also feels too risky to me. I think we should implement Better system for handles #1730 to fix this, but we could also just try to clean it up with the current system.I also think the example could be improved some.