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

Adding ADR-0009 #1861

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adding ADR-0009 #1861

wants to merge 1 commit into from

Conversation

bendk
Copy link
Contributor

@bendk bendk commented Nov 22, 2023

Use handle map handles to pass objects across the FFI,

@bendk bendk requested a review from a team as a code owner November 22, 2023 18:57
@bendk bendk requested review from badboy and removed request for a team November 22, 2023 18:58
@bendk
Copy link
Contributor Author

bendk commented Nov 22, 2023

I'll state my preference up-front: I think we should be using handles.

I don't like 1 because it's too hard to debug errors. Whenever I want to add new functionality that involves passing an object, I almost always mess it up at least once and currently this results in an impossible to understand stack trace or a crash with no message at all. When I was trying to switch things over to using a handle map in #1808, I definitely had bugs like this, but getting the nice error message was much nicer.

I don't like 4 because I don't want to add the dependency for all users.

I slightly prefer 3 over 2, but I'd be fine with either of those.

@bendk
Copy link
Contributor Author

bendk commented Nov 22, 2023

I think the changes in #1823 and #1826 are good regardless of this decision, so the ADR doesn't mention them. Please speak up if you think some of those should be part of the ADR, or another one.

This ADR might guide how some of that code looks, for example naming the variant FfiType::Handle makes more sense if we choose option 2 or 3.

@badboy
Copy link
Member

badboy commented Nov 23, 2023

Rendered

@bendk bendk mentioned this pull request Dec 1, 2023
This revisits the decision in `ADR-0005` and explores using going back to handle maps to pass objects across the FFI.
@bendk
Copy link
Contributor Author

bendk commented Dec 6, 2023

Just pushed some small changes to the text and added a document that tries to describe how the handlemaps would work in detail: https://github.com/mozilla/uniffi-rs/blob/6307aa0561312472557c8770da1042a7d74a24ad/docs/handles.md

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

Successfully merging this pull request may close these issues.

2 participants