Allow external crates to write their own binding generators #1201
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
Use &Path rather than AsRef<Path>. We can call `as_ref()` before calling into `BindingGenerator` and keep the implementation code simple. Added code to extract the binding-specific config from the `uniffi.toml` config that contains it. Instead of leveraging `From<&ComponentInterface>` and `MergeWith`, have some `BindingGeneratorConfig` methods that explicitly do what we need them to do: update the values read from TOML with defaults from `ComponentInterface`. When implementing custom types, I wasn't sure how to properly implement the `MergeWith` trait. For example, what if we have 2 configs, each with a custom type map should we try to take entries from both? Having methods specific to the actual process can avoid issues like that. In the custom types case, we never get custom types from the component interface, so we don't need to worry about merging them.
- Updated `BindingsGeneratorConfig` and added an empty implementation for bindings that don't have config values. - Instead of having `generate_external_bindings` construct the `BindingsGenerator` it now inputs one instead. The advantage here is that bindings generators can have whatever fields they want. For the desktop JS project I want to make a generator store an enum that controls if it's generating the .webidl, .cpp, .h, or .jsm file. - Have `write_bindings()` input the config since we're no longer passing that to the `BindingsGenerator` constructor. - Removed the `compile_bindings()` and `run_script()` methods. These are going to be replaced with some other system to run tests. - Separated the trait bounds for the `AsRef<Path>` arguments to `write_bindings()`. I think the previous code required all of them to be the same type. Now one can be a `PathBuf` and another can be a `Path`. - Made ComponentInterface public, since it's used in `BindingsGenerator` - Added `ComponentInterface` method to get all FFI functions, except the `RustBuffer` ones - Pass in an owned ComponentInterface rather than a reference - Don't return an error when `uniffi.toml` is missing - Added some anyhow context calls for better error tracking
Seperate bindings mvp
tarikeshaq
changed the title
bendk
requested changes
Apr 1, 2022
bendk
approved these changes
Apr 1, 2022
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.
These are the first bits needed to implement an external binding generator that lives in an outside crate.
Most of the changes are in the
uniffi_bindgen/src/lib.rsto define a traitBindingGeneratorthat an external crate can use to take advantage of the logic inuniffito parse theudlinto aComponentInterface.This is not a breaking change, and only exposes the functionality (it doesn't change the existing crates)
cc @bendk @mhammond