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

examples/wasm: update export example to show the use of the go:wasmimport directive #3589

Closed
wants to merge 1 commit into from

Conversation

deadprogram
Copy link
Member

This PR updates the wasm "export" example to show the use of the go:wasmimport directive. See #3165 for details.

…port directive

Signed-off-by: deadprogram <ron@hybridgroup.com>
Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

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

//go:wasmimport is meant for only importing functions, but it also unintentionally exports functions.

Do we want to officially support that, or add a new //go:wasmexport instead?

@deadprogram
Copy link
Member Author

deadprogram commented Mar 27, 2023

The proposal at golang/go#38248 was accepted and already implemented here golang/go@02411bc so in the interest of keeping compatibility we should probably do the same thing for now.

@@ -8,12 +8,12 @@ import (
func main() {
}

//export add
//go:wasmimport add
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this update is premature, go:wasmimport is only supported on functions with no body that get linked at runtime to exports of other modules.

Here it seems that the //export directive is intended to ask the compiler to add the function to the list of exports of the module, or am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah OK. I totally misunderstood what was happening here. Thanks for the clarification.

@aykevl
Copy link
Member

aykevl commented Mar 27, 2023

I have made a PR to fix this: #3610
It should make this PR (intentionally) fail.

That said, I do agree it would be a good idea to add an example with //go:wasmimport, but make it an actual import instead of an export.

@deadprogram
Copy link
Member Author

It should make this PR (intentionally) fail.

I think the best thing to come out of this PR is to prevent me from doing what is in this PR. 😸

Thanks @achille-roussel and @aykevl

I will close this one since it does not actually do anything we need.

@deadprogram deadprogram deleted the wasm-correct-example-docs branch March 27, 2023 17:34
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