-
Notifications
You must be signed in to change notification settings - Fork 235
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.
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
UniFFiing types and metadata: splitting out UDL #1604
Comments
What's the problem with the current structure? Why move everything around now when we'll have to move everything around or do other big refactorings again after UDL is removed? Do you expect UDL to stay around for long once the proc-macros have feature parity? |
I personally doubt UDL will be removed for a number of years - I'm certainly not going to force our internal projects which are quite stable to either migrate away from UDL or stay on old versions of UniFFI. I even suspect some people will choose to use UDL even after proc-macros have feature parity, certainly until we have nice tooling for generating docs. IMO effective tooling for the consumers will be critical - the consumers of our crates are not Rust engineers, and currently the UDL doubles as user-documentation. I don't think asking them to grep Rust code for macros to try and reverse engineer the foreign interface they use is going to be warmly received. Casual conversations with these engineers imply it might never be well received - the use of UDL is a feature to some of our users, not a bug. |
I like this overall. Even if we decide to kill UDL in the next year or so, this could be seen as a good first step.
I wonder if we could move the scaffolding generation to another crate or maybe even to a module inside Ideally, this code would parse the UDL file, generate a series of macros calls, and then go through more-or-less the same codepath. This makes the testing a lot simpler. I had to duplicate the Maybe this is part of the N++ plan, I'm not sure how to best sequence all of the work here. |
FWIW, A preview of "step 2" is at https://github.com/mhammond/uniffi-rs/pull/5/files. It's quite a large patch, but I quite like the outcome. |
If agreed, the outcome of this issue will be the following:
uniffi_meta
structsuniffi_bindgen
depend only onuniffi_meta
- ie, it knows nothing about weedle, udl, etcuniffi_bindgen
however does remain responsible for generating "scaffolding" (ie, the rust code to support what's defined in the UDL)The end result is that:
UDL is "demoted" (ie, no longer a first-class citizen of uniffi_bindgen)
uniffi_macros
is "promoted" to the canonical source foruniffi_bindgen
.uniffi_bindgen
continues to use its "interface" objects - eg, all bindings continue to use auniffi_bindgen::interface::Object
, it just that theseObject
s are always instantiated fromuniffi_meta
(whereas now they are constructed via both meta and weedle types)A downside of this is that
uniffi_meta
will need to grow to accomodate metadata specific only to bindings - eg, the boolean to indicate whether an argument isByRef
will be added touniffi_meta
even though bindings and the procmacros never need it.uniffi_meta
will similarly need to grow support for everything binding specific that can be expressed in UDL, even ifuniffi_macros
can't yet do that.I propose:
Step 1
Type
,Literal
andFfiType
, intouniffi_meta
, removing the existing equivalent types.** Even though the uniffi_meta versions of these structs are arguably cleaner, we stick with the OG type struct to avoid breaking bindings.
Step 2
uniffi_udl
: All UDL specific code inuniffi_bindgen
is moved to this crate.uniffi_udl
depends onuniffi_meta
, but notuniffi_bindgen
- thus it deals exclusively withuniffi_meta
.uniffi_meta
grows support for fields that are not yet supported byuniffi_macros
but whichuniffi_bindgen
needs.uniffi_bindgen
then springs aComponentInterface
andTypeUniverse
into existence only viauniffi_meta
types.Steps N++
@jplatte @bendk any thoughts or objections?
┆Issue is synchronized with this Jira Task
┆Issue Number: UNIFFI-285
The text was updated successfully, but these errors were encountered: