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

Add annotation framework for Wasm interfaces #519

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

electrum
Copy link
Collaborator

@electrum electrum commented Sep 9, 2024

This is an alternative approach to #506. See Demo and DemoGenerated for what the interface looks like.

It still needs actual runnable tests, maybe the OPA code plus some examples from wasm-corpus, but I wanted to put this up now to get some feedback. Also, needs tests for error handling.

Copy link
Collaborator

@andreaTP andreaTP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @electrum for putting this up to show where you are going, super useful!

Before jumping on a solution that is going this "high level" I would love to share some context regarding (my limited understanding of) CM, integration frameworks(extistm/wasm-bindgen/etc.) and the landscape we know about.

As a full disclosure, I'm comfortable moving forward and merging PRs up to:

(probably the final result will look pretty similar)

Would you be able to join tomorrow's call?
Alternatively, we can setup something ad-hoc, possibly, I'd like to have @evacchi and Ben's experience on the table.

@electrum
Copy link
Collaborator Author

electrum commented Sep 9, 2024

Let's talk about it tomorrow on the call.

import com.dylibso.chicory.runtime.Instance;
import com.dylibso.chicory.wasm.types.Value;

@WasmModule
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more inline with what I was thinking. I like the annotation approach and providing explicit c types. What I think we're trying to accomplish is more akin to what JNA does today. Not necessarily write the high-level bindings for you yet, but give you some types and code to keep you from having to explicitly encode the ABI level details when we can just borrow from some known ABI and types (usually C). This reduces repetition and mistakes as @andreaTP pointed out.

I can't make the call today but I'd vote for this approach. Feel free to make a decision without me though I trust y'all to figure out the right approach.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the annotation approach and providing explicit c types.

Agreed, thanks for your input here @bhelx 👍

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.

3 participants