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 'wasm_import_module' option to the @[Link] annotation #11935

Merged

Conversation

lbguilherme
Copy link
Contributor

@lbguilherme lbguilherme commented Mar 26, 2022

Lib imports in WebAssembly have a "namespace" to them, such that identically-named symbols can be imported from different modules. By default, all C-like libraries expose their symbols on the "env" module, but there are some cases where it is needed to import from a module with a different name.

For example, this is how to import WASI functions directly:

@[Link(wasm_import_module: "wasi_snapshot_preview1")]
lib LibWasi2
  fun random_get(buf : UInt8*, len : UInt32) : UInt16
end

Likewise, Asyncify (needed for Fibers and GC) come from the "asyncify" module. This flag is only meaningful when targeting wasm and will be ignored on other targets.

This is very similar to how Rust implements it: https://github.com/bytecodealliance/wasi/blob/984fa4fc17a074e8ea720cf319ad5e0357a991c7/src/lib_generated.rs#L2166


Additionally, all top-level fun functions are exported. For example:

fun add(a : Int64, b : Int64)
  a + b
end

# Exports the function "add" to be called externally.

@lbguilherme lbguilherme force-pushed the feat/wasm_import_module branch from 2af59c7 to 711a9ce Compare March 26, 2022 13:30
@straight-shoota
Copy link
Member

This is entering new territory in multiple ways. So we need to carefully discuss the kind of change we want to make.
Could you please start with an issue detailing the problem (and proposed solution)?
AFAIK experience with WebAssembly is pretty scarce among compiler contributors. So we should have a discussion to properly understand the specific challenges and consider our options.

@lbguilherme
Copy link
Contributor Author

I understand the need to discuss the solution, but in this case, I really don't see another way to implement this since this is the way Rust or Clang do it. This is a small change and we can discuss with a more concrete view of the impacts in mind, the discussion will probably be easier this way since, as you said, there isn't much experience with wasm from other contributors. I don't mind if the decision here is to implement it in an entirely different way, this doesn't have to be accepted just because it's done.

I'll prefer opening issues on future occurrences.

@lbguilherme
Copy link
Contributor Author

I added exporting all top-level fun as discussed in #11941. This behavior is in line with the other targets.

@beta-ziliani beta-ziliani added this to the 1.6.0 milestone Sep 22, 2022
@straight-shoota straight-shoota merged commit abe4d34 into crystal-lang:master Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants