Skip to content
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

Remove the need for [External] and typedef UDL definitions #2302

Open
bendk opened this issue Nov 9, 2024 · 3 comments
Open

Remove the need for [External] and typedef UDL definitions #2302

bendk opened this issue Nov 9, 2024 · 3 comments

Comments

@bendk
Copy link
Contributor

bendk commented Nov 9, 2024

We currently need these for a few reasons and I'm hoping to make them all obsolete.

  • To know which UniffiTag system to use. This won't be necessary after Make UniffiTag consistent for UDL and procmacros #1865.
  • To specify the bridge type for custom types. This is no longer needed now that New custom type system #2150 is merged.
  • To know the FFI type when lowering. However, this can be determined using the proc-macro metadata and we're already translating all UDL definitions to proc-macro invocations.
  • To fill in required fields for the uniffi_meta types. We can avoid this by making uniffi_udl generate its own types, rather than the uniffi_meta types. We don't need to all of that data, we just need to enough to generate the proc-macro calls. One side benefit of this is that we could remove fields in uniffi_meta that only exist to support uniffi_udl (example)

The only downside I can see is that maybe the error messages will get worse if you misspell a type. I'm not too worried about this since I'm more and more convinced we should be pushing users away from UDL anyways.

bendk added a commit to bendk/uniffi-rs that referenced this issue Nov 9, 2024
This wasn't released in 0.28.2.
@bendk
Copy link
Contributor Author

bendk commented Dec 31, 2024

To fill in required fields for the uniffi_meta types. We can avoid this by making uniffi_udl generate its own types, rather than the uniffi_meta types. We don't need to all of that data, we just need to enough to generate the proc-macro calls. One side benefit of this is that we could remove fields in uniffi_meta that only exist to support uniffi_udl

This is not currently true, since not all users are using library mode. In that case, we do need to be able to generate the bindings directly from the UDL and we don't get to read the metadata. I think this change is blocked unless we decide to force users to use library mode.

Should we do that? I'm not so sure, since one of the main remaining benefits of UDL is that it allows users to avoid library mode. I guess I'm thinking if we're going to break things for users, maybe the next step is to remove UDL support altogether rather than force-library mode. In any case, I think don't think we're ready for either step yet.

@mhammond
Copy link
Member

mhammond commented Jan 1, 2025

TBH I don't see a reason to deprecate/remove UDL. However, I think "library mode" is a different and possibly unrelated issue. I think that in practice, library mode is the only thing that works with multiple crates, and I don't see a good reason which would justify keeping non-library-mode around for this "edge-case" of only ever wanting to use a single crate. That said though, I also see no reason to deprecate that until there's an actual cost to us of keeping it.

It's the strange intersection between "library mode" and scaffolding that I don't actually understand though. IIUC, all library mode users who use UDL need both "udl only" mode to generate the .rs and library mode to generate the bindings.

IOW, I think I'd support a story that was something like "only library mode is supported for bindings", but "udl parsing is supported to help generate scaffolding", and I guess I don't see why they need to be mutually exclusive - and I'd still say it's OK to describe that scenario as a user of library mode.

tl;dr - I don't see a contradiction between "we only support library mode" and "we can generate scaffolding from UDL"

@bendk
Copy link
Contributor Author

bendk commented Jan 2, 2025

IOW, I think I'd support a story that was something like "only library mode is supported for bindings", but "udl parsing is supported to help generate scaffolding", and I guess I don't see why they need to be mutually exclusive - and I'd still say it's OK to describe that scenario as a user of library mode.

I wouldn't hate this, but why not tell users to go all the way and delete the UDL file and add proc-macro attributes? It's not that hard to do. When I was doing it for the app-services crates the switch to library mode was much harder -- mostly because messing with build scripts is harder that updating Rust code.

I guess it comes down to if UDL is providing benefit as a language-neutral description of the API or something like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants