Skip to content

Implement Accessing Export Memory #33

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

BjoernAkAManf
Copy link
Contributor

Besides implementing the above capabilities (similar to the imports PR #31 ), this also adds some documentation from the Rust Binding.

Still unsure about the name match_extern_type
Copy link
Owner

@kawamuray kawamuray left a comment

Choose a reason for hiding this comment

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

thanks for an another PR! Some comments left.

Follows the rust bindings choice for just one type.
Clean up the duplicated code as a result.

Dependent code needs to change accordingly.
@BjoernAkAManf
Copy link
Contributor Author

I'll take a look at the documentation later. I agree that the rust specific part might not be appropriate.
I think it does have some value though.

@BjoernAkAManf
Copy link
Contributor Author

Seems like a dependency issue occured in the meantime.

I'll have a loom at it later :)

Copy link
Owner

@kawamuray kawamuray left a comment

Choose a reason for hiding this comment

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

Thanks, followed up some comments.

Seems like a dependency issue occured in the meantime.

yes indeed.. looks like one of transitive dependency of wasmtime now requires newer version of Cargo to build it. Weirdly I could not reproduce it until I clear local crates cache.

Anyway, I managed to pass the build by upgrading rust nightly version, so you can also PR to update the rust version we're using in CI to pass the build: https://github.com/kawamuray/wasmtime-java/blob/master/.github/workflows/ci.yml#L24

@@ -8,18 +8,8 @@
@Accessors(fluent = true)
@AllArgsConstructor(access = AccessLevel.PACKAGE)
public class ImportType {
Copy link
Owner

Choose a reason for hiding this comment

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

I think we don't even need ImportType and ExportType separately. They share exactly the same definition, so we just need to rename this class to ExternType and use it for exports too?

* All WebAssembly instances and items will be attached to and refer to a Store. For example instances, functions,
* globals, and tables are all attached to a Store. Instances are created by instantiating a Module within a Store.
* <p>
* A Store is intended to be a short-lived object in a program. No form of GC is implemented at this time so once an
Copy link
Owner

Choose a reason for hiding this comment

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

This document has a rust-java gap problems too. The notion of "GC", "drop", and so on.

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