From 8f616b33edf043efbe8f167a98391740e5eda6e1 Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Fri, 5 Jan 2024 17:06:15 -0500 Subject: [PATCH] Adding ADR on UniffiTag handling --- docs/adr/0009-uniffi-tagging.md | 95 +++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 docs/adr/0009-uniffi-tagging.md diff --git a/docs/adr/0009-uniffi-tagging.md b/docs/adr/0009-uniffi-tagging.md new file mode 100644 index 0000000000..1c9fa34bd3 --- /dev/null +++ b/docs/adr/0009-uniffi-tagging.md @@ -0,0 +1,95 @@ +# Handling `UniffiTag` and using types from other crates + +* Status: proposed +* Deciders: Uniffi developers +* Date: 2024-01-05 + +## Context and Problem Statement + +UniFFI's various FFI-related traits `FfiConverter`, `Lift`, `Lower`, `LiftReturn`, `LowerReturn`, etc. all have a generic "tag" parameter. +The code in `uniffi_macros::setup_scaffolding` generates a zero-sized `crate::UniffiTag` type for use in that parameter in other generated code. +For example, when lowering a type we use `<#type as Lower>::lower`. + +The reason for this is the so-called "Orphan rule", which prevents generated code from implemented a remote trait on a remote type. +If the FFI traits did not have a generic parameter, then it would be impossible to generate an code that implments those traits for a type defined in another crate. +See [ADR-0006 Wrapping types](./0006-wrapping-types.md) for more discussion. + +For local types we have a choice: implement only for the local tag (`impl Lift for MyType`) or implement it for all tags (`impl Lift for MyType)`). +This choice mainly affects other crates that also want to use that type. +If `crate_a` implements the traits for their local tag and `crate_b` uses that type, then users must take some explicit action to implement the FFI traits for `crate_b::UniffiTag` +If `crate_a` implements the traits for all tags, then users will normally not need to take any explicit action. +However, in the case where `crate_a` is implementing the trait for a remote type, then they will. +This is quite confusing in the abstract, hopefully the example below clears things up. + +Currently, UDL-based generation uses the former approach and proc-macros use the latter. We should pick one approach and stick with it. + +### Example + +- `crate_a` depends on the `bitcoin` crate and uses the [Network](https://docs.rs/bitcoin/latest/bitcoin/network/enum.Network.html) enum in their exported API. + (Let's assume that we've implemented a way to do this with proc-macros, maybe some system similar to [the way serde handles it](https://serde.rs/remote-derive.html)) +- `crate_a` defines/exports a `NFTGenerator` type, which is just a normal UniFFI interface. +- `crate_b` depends on `crate_a` and `bitcoin` and wants to define the function `fn build_nft_generator(network: Network) -> NFTGenerator`. + This is a convenience function that simplifies constructing an `NFTGenerator`. + +When `crate_a` implement the FFI traits for `Network` it must implement them for its local `UniffiTag` only. +However, it can implement the FFI traits for `NFTGenerator` for either its local `UniffiTag` or all tags. +This choice will affect how `crate_b` "imports" the UniFFI types. + +### Always implement traits for the local tag only + +- `crate_a` implements the FFI traits for its local `UniffiTag` for both `Network` and `NFTGenerator`. +- `crate_b` needs to take explicit action to use the UniFFI types from `crate_a`. + +```rust + +use crate_a::{NFTGenerator, Network}; + +uniffi::use_object!(crate_a, NFTGenerator) +uniffi::use_enum!(crate_a, Network) +``` + +Note: right now these macros are named `use_udl_object`, etc. +Once proc-macros can implement traits for remote types we probably should rename them to something not specific to UDL, but the exact name is out of scope for this ADR. + +### Implement traits for all tags when possible + + +- `crate_a` implements the FFI traits for its local `UniffiTag` for `Network` +- `crate_a` implements the FFI traits for all tags for `NFTGenerator`. + This is allowed since the type is local to `crate_a`. +- `crate_b` only needs to take explicit action to use `Network` since it's a remote type for `crate_a`. + +```rust + +use crate_a::{NFTGenerator, Network}; + +uniffi::use_enum!(crate_a, Network) +``` + +## Decision Drivers + +- Using one system for UDL and a different one for proc-macros is overly complicated and therefore not discussed as an option. +- TODO: add more as we discuss this. + +## Considered Options + +### [Option 1] Always implement FFI traits for the local tag only + +### [Option 2] Implement FFI traits for all tags for non-remote tags + +## Pros and Cons of the Options + +### [Option 1] Always implement FFI traits for the local tag only + +- Good, because it's more explicit. +- Good, because it's easier to explain. + Telling users "you always have to add `uniffi::use_*!`" to use a type from another crate is easier than saying you sometimes you need it and sometimes you don't. + +### [Option 2] Implement FFI traits for all tags for non-remote tags + +- Good, because it's less typing +- Good, because it's the simplest system when remote types are not involved. + This happens when all types come from the user's crates or the 3rd-party crates implement the UniFFI traits themselves. + +## Decision Outcome +