-
Notifications
You must be signed in to change notification settings - Fork 231
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
[discuss] next steps for interface definition language? #385
Comments
Macros are personally very interesting to me, so I've been lightly messing around with what a macro-based syntax for uniffi might look like. Some early thoughts are visible in #386 but I hope to have more considered comments on this option sometime soon. |
An interesting (possibly minor) advantage of the keeping WebIDL for UDL is that it forces us to align reasonably well with real WebIDL, which is required for the current geckoJS backend implementation to work. |
I'mma hop on this since I saw the invitation on Element 😛 I haven't been following too closely how the tool has grown over the past few months, but.. There are a few quick naive thoughts hovering in my brain regarding taking the Marcos approach:
I don't have too much recent context to add much more (especially on how the gecko bindings went), but I'm glad we're revisiting this, I remember defining error handling was a little forced onto the idl. |
@tarikeshaq oh hi! Thanks for hopping in! 😁
Well, without wanting to give away any spoilers, I actually have this working in a branch as an experiment and am surprisingly happy with the consumer experience so far; ref the fxa-client-features branch if you're curious about the details. I hope to spend a few down cycles on polishing that up and presenting a more thorough proposal within the next week. |
Holy cow... |
As hinted above, I've been messing around with procedural macros and have the start of an idea that I quite like. I've pushed it here for early visibility: I'll write up some thoughts in the present issue for consideration, but I want to stress, I'm not wedded to any of the ideas here. I've been messing around with it because it's fun, and I think it seems promising, but there may be other better ideas to pursue here as well. So, the idea in the above draft PR is to lean very heavily into defining the interface through a procedural macro. Here's a strawfox of what you might write in a uniffi-ed crate with this new approach:
This is a procedural macro applied to an inline sub-module, and the contents of the inline sub-module directly implement the desired component interface. There's no separate declaration of the interface, you just implement it directly in Rust code inside the decorated sub-module. When you compile this as a Rust crate, it produces the same Thinking specifically about writing the Rust code here, this approach as several advantages:
The putting-it-in-a-submodule thing is actually quite important, because it means that UniFFI can see the entire interface at once, just like it can today when parsing from a separate file. This lets us perform important consistency checks, and also lets us insert runtime footgun protection such as the "checksum" that we put into all the FFI symbol names. When it comes time to generate the foreign-language bindings, there are a couple of options. The simplest would be to run A different approach, which I think feels better would be to operate on the crate as a whole rather than on a specific Rust file. I'm imagining a command like So, a summary of what a possible future could look like:
One downside of this approach, of course, is that it's a lot of work! We'd need to build and maintain a proc macro, we'd need to figure out how to map all the bits of UniFFI interface onto corresponding Rust syntax, we'd need to figure out how to give a good developer experience when the user tries to use a bit of Rust syntax that we don't support, and so on. My experiments in #416 have so far been pretty re-assuring in this regard. Rust's macro ecosystem and I plan to keep messing around with this approach in some background cycles. It's not urgent, but it's interesting. Any and all feedback is most welcome! |
I have been holding off on this thread. I'm not sure I'm convinced by the following argument as a deal breaker for a I, for one, will mourn the passing of the canonical IDL file. The IDL file is a concise description of the API that is implemented in Rust, but used by higher level language programmers: iOS, android or desktop developers. It is, or could be a lingua franca for each of the engineering stakeholders in the system. At its best, it is the common document with which these devs with disparate skill sets can gesture at, discuss and request changes: it is a communication medium for development teams. A mobile engineer implementing a new feature can succinctly request a new method, new attribute, new parameter, without having to understand Rust. A mobile engineer and a desktop engineer can design an API together then find a Rust engineer to implement it. Without it, the Rust engineer gets to mediate between the needs Desktop and iOS and Android and Rust, design the API that they won't use, and fielding feature requests written for a variety of languages. I'll add more thoughts and reasonings in another comment. |
I really like this, and will happily dance on the grave of the UDL file ;) I do see James's point, but the risk with the .udl that I see is that for the developers of the component it's not really canonical, but is for the consumers, which means the consumer tends to lose. Over time, ISTM that the rust developers are far more likely to add interesting notes, comments or warnings into the .rs file and not the udl file. We can already see this in nimbus - That, combined with good tooling, should mean the mobile consumer never needs to read the .rs file, just the generated documentation. Maybe that's wishful thinking though and assumes projects have great CI and tooling (ie, if mobile consumers still end up reading the .rs file instead of the generated code, we've screwed up - but that doesn't help those poor consumers) If Jame's felt really strong about this, I guess it would be an argument for keeping the UDL generator in that PR! I'm inclined to lean against that though.
That struck me as very odd too - I was going to comment in the PR but then realized it's really just the status-quo. Specifically, using gradle to generate stuff when everything is so tightly bound here seemed somehow like an opportunity for stuff to get out of sync, but I've no helpful suggestions. |
I like this framing at a high level, FWIW. As a concrete example, one of the things I don't totally love about the way wasm-bindgen works is that you end up with a bunch of Rust code with some annotations sprinkled in amongst it, and it's hard to see what the actual resulting API surface will be. I do agree that there's value in being able to reason about the API surface as a concrete artifact, somewhat independent of the implementation details. (Without expressing it in quite those concrete terms, I think this is partly what motivated my use of an I'll chew on this a bit, as I feel like there must be a way for us to have our cake and eat it too here somewhere... |
TBC: I'm not arguing for any particular IDL, just that the work flow include a hand-written/hand-writable IDL, and that a change in the IDL file should be reflected in the rust code (as all the bindings). This could involve, say, a proc_macro consuming the a IDL file to generate structs, traits (instead of impls) and enums. A change in the IDL of an object (in UDL, an interface) would cause a trait to change, and a compile error in Rust to help the Rust dev find what needs doing. How strongly do I feel about this? The upper bound is no stronger than: "I'll feel sad if we don't do this"; I'm also mindful of Rust devs who want to publish their crates on Carthage or Maven to not find this disgusting. |
While I see your point, I remain unconvinced. IIUC, we are discussing 2 main options:
To put it another way - I think artificially forcing a UDL is doing a dis-service to both the rust maintainer ("even though we could generate this, it's for your own good that you must hand-write this twice") and the non-rust consumer ("we've designed this simplistic, artificial language just for you, because you wouldn't understand the alternative") |
Cross-referencing for context, we had a team work-week last week and there was a flurry of activity on the experimental macro-based PR over in #416. @badboy raised an important concern here: for an API surface of any significant complexity, it can get pretty unwieldy to have the interface and its underlying Rust code all defined in a single place in a single file. If we do want to pursue the macro route, it's clear we have some more design work to do on the details of the syntax there. |
I've updated the original issue comment with a more thorough list of the current pain-points of our |
Mark kind of hinted at this on the call last week, but what if we combined the macro approach with a separate definition file? The file would just be 1 big macro call that contained a copy of the enums, function signatures, and trait definitions, whatever was needed. |
What would be the advantage of that over the current UDL?
Now don't get me wrong, I might also be more comfortable writing sort-of-Rust than I am writing UDL, so this shouldn't dismiss that idea right away. |
I have an idea for an alternative in the vein of We could copy parts of what SQLx does for its "offline mode" and have more fine-grained macros (applied to invidiual types, function and impl blocks) that analyze the item they are added to and serialize the necessary metadata to a file. Then a tool like I can dedicate some time to working on this if you think it is worth exploring. ¹ after getting rid of outdated ones by clearing the directory for them (e.g. |
That's a really interesting suggestion. When you say "serialize the necessary metadata to a file", does that mean the macros would write to a file in the One idea we've been floating around is using syn to parse the parts of the source tree it needs. You would point uniffi at the "root" FFI module, then if that module had a One issue is figuring out where the type definition is based on the use statement. The initial work might have some limitations, like it can't support inline modules or type aliases. |
Many files, since multiple macros can run in parallel. We'd have to come up with a naming scheme, ideally something like
I'll assume you mean Advantages (of parsing the whole crate):
Disadvantages:
|
See also this comment and this comment and related discussion about how |
After reading those comments as well as https://github.com/mozilla/uniffi-rs/blob/main/docs/diplomat-and-macros.md, I'm not convinced the same issues will arise from the approach I have suggested, so I will start working on a prototype soon 🙂 |
This seems like a very real issue to me. The other ones I mentioned have workarounds, but I can't really think of a way for this one. Do you currently use
How many hoops are required to embed this in the binary? It seems like we could use basically the same system that @jplatte suggests, but instead of writing to The upside the interface metadata is always attached to the library. There's only 1 file to keep track of. There's no need to clean and rebuild. Food for thought as you work on the prototype. Feel free go with your original plan of writing to the JSON file. It seems like a good place to start and we can iterate later. |
Update: I've been working on this for a while (using the JSON approach) and while I've only gotten to generate Rust scaffolding code for an argument-less unit-returning function from a One of those was how the scaffolding generation would be integrated into the Cargo build process, since you can't generate the scaffolding ahead of building the crate, because the proc-macros are called as part of that build process. The solution I came down on and discussed with @badboy yesterday is that there is going to be a separate ( We also discussed that it would be ideal to have FFI crate building be a regular To avoid spending lots of time on finding the right library files and extracting symbols from them, I will attempt to implement the metadata collection using |
It sounds like |
Quite the opposite! It's pretty clear that macros are the way forward, initial support for bridging of async functions landed very recently without support for UDL and overall I get the feeling that at least half the PRs are about improving the proc-macro frontend. The macros are also documented (albeit with a big warning about being experimental, maybe that can be toned down a little bit now). I'm not sure when I will submit the next PR to make progress on the checklist, but if you want to help, I'd be happy to write instructions for sub-tasks and review PRs. |
This is my feeling as well. I think the macros are usable now for many use-cases and we will keep improving their usability. My goal for our Mozilla code is to switch to a proc-macro/UDL combination in the next few months and only proc-macros in maybe a year. There was a time when I would have been all for writing a custom UDL parser, but now I feel like it's better to focus on the proc-macros. But if there's a particular reason that you want to explore custom UDL parsing, I'm open to discuss it. |
I see, this is very cool to hear! Not having to duplicate API definitions in both Rust and |
I added that. It should actually be pretty straight-forward: Before the compiler expands a derive macro, it takes the /// Client for the Foo Bar API.
///
/// Lorem ipsum dolor sit amet, [...]
#[derive(uniffi::Object)]
struct FooBarClient {
// ...
} the proc-macro input will be #[doc = "Client for the Foo Bar API."]
#[doc = ""]
#[doc = "Lorem ipsum dolor sit amet, [...]"]
struct FooBarClient {
// ...
} |
I've been using the macros and they've been great. The only problem for me is I can't use a type(s) defined in the macro in the .udl file. I still have to use the Is this something that might be supported in the future? Or is it just a growing pain until more things (ex: callback interfaces) are supported in proc macros? |
I think it will be much much easier to add support for the remaining things such as callback interfaces. |
Makes sense, the proc macros are so much nicer to use. |
I'm hoping to work on tying up the remaining loose ends starting in a couple weeks. I think that's:
Are there any other features that need implementing? Of course, if @jplatte or anyone else wants to implement those things, please feel free to. |
@bendk Personally the only things in my udl are the constructors, callback interfaces, and the types needed by the callback interfaces. So your list covers all my (current) use cases. |
One more thing we currently use UDL for is declaring a few simple types from external crates. Luckily these are all crates we can modify to include (optional) UniFFI derives, it's just that the single-crate restriction needs to be lifted (it's still in place, right?). Anyways I'd love to do constructors (just need to carve out some time for it), and would love if I didn't have to do callback interfaces 😄 |
After #1457 we can add traits to that list.
I've still got a longer term idea that support for traits can deprecate callback interfaces - the main difference will be that currently callback interfaces generate the rust trait code and a struct implementing it - so we could kill the former and conditionally do the latter. There's some devil in the detail, but I think it's achievable and would be a good outcome. |
@jplatte if you're taking suggestions on what to support next in the proc macro, I would love to support for callbacks🙏 |
I'm not but you're lucky, there is already a PR for that: #1573 (I'll review it in the coming week). |
Awesome thanks @bendk |
Should we close this in favor of #1257? |
When we started building this tool, the choice of WebIDL for interface definitions was deliberately short-term (ref ADR-0001 and we knew we'd need to revisit it someday. Now feels like a good time to start discussing what the next steps for interface definition might look like.
I'll try to do some scene-setting below, but let's consider this an open discussion thread for anyone interested in this topic to share whatever partially-baked thoughts they may have.
To begin, I want to note that I think WebIDL has worked out surprisingly well for our purposes, given that it was designed for a language ecosystem and set of constraints that are different to what we're doing here. It certainly helped with the goal of getting up and running quickly! But we have observed a few pain-points in practice (this list will be updated as we encounter more):
namespace example {}
to give a namespace for the component.namespace example { my_function() }
, while other interface members as defined as their own standalone items..udl
to help out with passing things by owned value vs by reference, which is a distinction that only matters internally to the Rust code..udl
files; theweedle
crate seems (quite justifiably!) designed more for working with known-good.webidl
files than for helping its users author new ones..udl
file and the Rust code; you basically get a gnarly error message from the generated Rust code, which is hard to act on if you don't have a good mental model of how it's generated..udl
file, but they're not available to the parser so we can't do interesting things with them (such as auto-generating docs for the generated code).There are probably more pain-points, so please feel free to list additional ones below and I'll try to add them to this list.
So what should the next steps be for the "defining the component interface" part of this tool? We've discussed a few main options in the past:
.udl
files in favour of something with Rust macros, in the style of wasm-bindgen.Love or hate any of those suggestions? Please share below! There may also be other approaches that we haven't noticed yet, and I'd love to hear suggestions for those as well.
Whatever solution we come up with, here's a list of the things we know we'll have to factor in to the design:
.udl
, in particular default arguments..udl
, and a good story for migrating them to the new syntax.┆Issue is synchronized with this Jira Task
┆Issue Number: UNIFFI-48
The text was updated successfully, but these errors were encountered: