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

Tracking issue for #[wasm_import_module] #52090

Closed
alexcrichton opened this Issue Jul 6, 2018 · 23 comments

Comments

Projects
None yet
7 participants
@alexcrichton
Member

alexcrichton commented Jul 6, 2018

This is a tracking issue for the #[wasm_import_module] attribute and the wasm_import_module feature. This attribute is applied to extern { ... } blocks like so:

#[wasm_import_module = "foo"]
extern {
    fn foo();
    fn bar();
    // ...
}

The WebAssembly specification requires that all imported values from the host environment have a two-level namespace:

All imports include two opaque names: a module name and an import name, which are required to be valid UTF-8. The interpretation of these names is up to the host environment but designed to allow a host environments, like the Web, to support a two-level namespace.

Additionally only globals (like static variables), functions, memories, and tables can be imported. The extern { ... } language block is used to import globals and functions, memories and tables cannot currently be manually imported.

Each field of the wasm import needs to be configurable by Rust as both fields have semantic meaning. Typically the first field of the import is the "module" interpreted as an ES module, and the second field is what to import from that module.

Currently LLVM and LLD will default imports to the "env" module. This means that if you compile this code:

extern {
    fn foo();
}

it will generate a wasm file that imports the function "foo" from the module "env". By using #[wasm_import_module] we can customize what happens here:

#[wasm_import_module = "somewhere else"]
extern {
    fn foo();
}

This attribute must be of the form #[wasm_import_module = "string"], no other forms are accepted. It can only be attached to an extern block. All items in the block are considered to come from the same module. Through the usage of #[link_name] we can then configure both fields of WebAssembly imports arbitrarily:

#[wasm_import_module = "somewhere else"]
extern {
    #[link_name = "some_other_name"]
    fn foo();
}

The #[wasm_import_module] is accepted on non-wasm platforms but has no effect, it is simply an ignored attribute.

Helpful links:

Open questions and TODO

  • Documentation in the reference
  • Should the attribute instead be #[link(wasm_import_module = "...")]?

cc rustwasm/team#82

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 6, 2018

The WebAssembly target doesn't actually have any concept of a single-level import, so in that sense this doesn't necessarily make sense in wasm:

extern {
    fn foo();
}

We're currently relying on LLVM's defacto behavior to set the import module as "env", but we could also specify it ourselves to do that. Additionally we could also forbid this at compile time and simply require that all extern blocks have a specified module. I would personally lean towards the latter of these interpretations.

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 6, 2018

I'd like to propose that this feature be put up for stabilization but I'm not currently a member of the compiler team to do so, would someone from @rust-lang/compiler like to help me champion this issue and FCP it for stabilization?

Also cc @rust-lang/wg-wasm

@Mark-Simulacrum

This comment has been minimized.

Member

Mark-Simulacrum commented Jul 6, 2018

Is it well-defined for the attribute to have a space as in the tracking issue #[wasm_import_section = "somewhere else"]? That seems counter intuitive to me; I haven't looked, but if not we should have the compiler itself deny invalid identifiers there.

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 6, 2018

The WebAssembly specification only has the requirement of "required to be valid UTF-8", and beyond that there is no restriction on what the import module can be. We wouldn't want to restrict it to only Rust identifiers because it's frequently an ES module path like ./foo or foo/bar or @foo/bar/baz.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 6, 2018

kennytm added a commit to kennytm/rust that referenced this issue Jul 6, 2018

Rollup merge of rust-lang#52093 - alexcrichton:update-issue, r=kennytm
rustc: Update tracking issue for wasm_import_module

It's now rust-lang#52090

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 6, 2018

kennytm added a commit to kennytm/rust that referenced this issue Jul 6, 2018

Rollup merge of rust-lang#52093 - alexcrichton:update-issue, r=kennytm
rustc: Update tracking issue for wasm_import_module

It's now rust-lang#52090
@nagisa

This comment has been minimized.

Contributor

nagisa commented Jul 6, 2018

@rfcbot merge

Seems fairly uncontroversial change to me. @alexcrichton summarized this better than I could above, so read that.

AFAIK env is also the most common "section" in use, so this attribute would not see too much of an use, but when some library uses a section other than env, this becomes critical for interop with that library.

@rfcbot

This comment has been minimized.

rfcbot commented Jul 6, 2018

Team member @nagisa has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@nagisa

This comment has been minimized.

Contributor

nagisa commented Jul 6, 2018

This feature will documentation in the book/reference/etc. before being finally stabilised. Please add a TODO to that effect to the text of the issue.

@nagisa

This comment has been minimized.

Contributor

nagisa commented Jul 6, 2018

Also a naming bikeshed: couldn’t we reuse #[link] and add a #[link(wasm_section = "foobarbaz")] or something like that instead of coining a new attribute?

@tschneidereit

This comment has been minimized.

tschneidereit commented Jul 6, 2018

@alexcrichton, I assume the mentions of wasm_import_section should be wasm_import_module?

@Mark-Simulacrum, @nagisa, as @alexcrichton mentioned above, this will be used to import from ES module paths. Once the ES module integration is standardized, this will in fact be used a lot: whenever a Wasm module is loaded as an ES module instead of via the JS API (WebAssembly.instantiateStreaming etc).

@alexcrichton my only concern is that this is somewhat verbose. E.g. compare this JS import

import {
    some_other_name as foo,
    bar
} from "somewhere else";

to this Rust one:

#[wasm_import_section = "somewhere else"]
extern {
    #[link_name = "some_other_name"]
    fn foo();
    fn bar();
}

I suppose this is something where sugar could be added later if it turns out to really be too verbose in practice, though.

@Pauan

This comment has been minimized.

Member

Pauan commented Jul 6, 2018

I am not a member of the Rust Compiler team (I am a member of the Rust WebAssembly team), however one concern I have is with using wasm_import_module to import a relative ES6 module (which will be a common use-case).

Consider this Rust code:

// path/to/some/crate/foo.rs
#[wasm_import_module = "./bar.js"]
extern {
    ...
}

In this case we have a file foo.rs which is importing a file bar.js (which is in the same folder).

My expectation is that this will work. However, it doesn't work, because after the crate is compiled into .wasm, the path to foo.rs is lost, so it ends up looking for ./bar.js in a completely different (and wrong) folder.

I'm not sure how to fix this issue.

Perhaps it can be fixed entirely by third-party tools like wasm-bindgen and wasm-pack (can it?).

But I suspect it might require some changes to the wasm_import_module attribute, such as saving the path to the .rs file somewhere, so that way any downstream tools (like wasm-pack) can recreate the correct relative path.

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 6, 2018

@nagisa it's true yeah, going off #[link] would also be fine by me! Although @tschneidereit is right in that wasm_import_section was a mistake above (corrected now), so I think it'd look like:

#[link(wasm_import_module = "...")]
extern {
    // ...
}

@tschneidereit ah thanks for pointing out my mistake! I definitely meant wasm_import_module the whole time! Also it's true yeah that this is a bit wordy, but what I'm hoping for here is the low-level support in rustc as opposed to a high-level attribute. Basically providing the building blocks for tools like wasm-bindgen. With wasm-bindgen these attributes aren't actually ever declared by a user, but rather they're purely internal implementation details.

@Pauan my hope with this attribute (and #[wasm_custom_section]) is to provide the low-level support necessary to take advantage of these features of the wasm spec. I would expect higher level tools to actually process JS files and paths and such, but they need the ability to simply at the lowest level control what rustc does. This is similar to SIMD stabilization where we didn't stabilized an ergonomic interface to SIMD (as that takes quite some time) but rather just the low level tools needed to work with SIMD on stable.

@Pauan

This comment has been minimized.

Member

Pauan commented Jul 6, 2018

my hope with this attribute (and #[wasm_custom_section]) is to provide the low-level support necessary to take advantage of these features of the wasm spec. I would expect higher level tools to actually process JS files and paths and such, but they need the ability to simply at the lowest level control what rustc does.

Sure, I agree. My concern is this:

  1. You are writing a Rust crate.

  2. Your Rust crate has a dependency on another Rust crate.

  3. That other Rust crate uses wasm_import_module to import a .js file which is local to that crate.

I think that situation will be difficult for wasm-bindgen / wasm-pack to handle, because the wasm_import_module (and the .js file) are in a completely different crate.

Given that, it may be necessary for wasm_import_module to be extended with additional functionality (such as storing the path to the .rs file), but that might be a breaking change, which is why I'm concerned about stabilizing it.

So as long as we have some way of fixing the above problem without requiring breaking changes to wasm_import_module, then I'm okay with stabilizing wasm_import_module.

But I don't see any obvious ways to do that, do you have any ideas?

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 6, 2018

@Pauan yes I think it'll be difficult to handle but I don't think it'll be a problem in practice. It sounds similar to "what if a crate uses unsafe incorrectly?" or "what if a crate depends on a libc symbol I haven't bound?" Crate authors themselves have to be proactive about what they're using and authors would basically need to understand that using #[wasm_import_module] to import from a local JS file basically doesn't work, they'd need a different mechanism (like what wasm-bindgen/wasm-pack would provide).

I see #[wasm_import_module], like SIMD, as a building block for tool authors and framework authors, not all developers. Not all developers are writing SIMD intrinsics by hand, nor will they by writing custom wasm imports by hand. Rather centralized tools (like the faster crate or wasm-bindgen) will emit these primitives.

@Pauan

This comment has been minimized.

Member

Pauan commented Jul 7, 2018

@alexcrichton So, what you're saying is that users generally won't use wasm_import_module, instead tools such as wasm-bindgen / wasm-pack / etc. will automatically insert wasm_import_module (after doing the appropriate relative path resolution)?

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 7, 2018

Indeed yes, that may likely be the case

@Pauan

This comment has been minimized.

Member

Pauan commented Jul 7, 2018

@alexcrichton Alright, in that case I don't have any concerns.

@eddyb

This comment has been minimized.

Member

eddyb commented Jul 12, 2018

Don't we have #[link(name = "...")] or something for pretty much this purpose exactly?

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 12, 2018

@eddyb unfortunately no, this is configuring a two-level namespace which we don't have a configuration for right now for other platforms. Something like #[link(name = "...")] is selecting the name of the library to link

@eddyb

This comment has been minimized.

Member

eddyb commented Jul 12, 2018

@alexcrichton But isn't the first of the two level-namespace equivalent to the library half of the pair of "library and symbol" from, e.g. Windows?

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 12, 2018

@eddyb maybe? In that sense wasm has three namespaces because it'll still have archives of compiled C objects which need to be identified with #[link(name = "...")]

@eddyb

This comment has been minimized.

Member

eddyb commented Jul 12, 2018

@alexcrichton Ah, this makes more sense, sorry about the confusion!
Any overlap with maybe sections or using commas somewhere? I just expect that the more specific to wasm a mechanism, the more we'll regret it in the future.

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 12, 2018

Currently no in the sense that wasm doesn't dictate any meaning with the first level of the namespaced import, it's up to bundlers and module instantiators to interpret that

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 16, 2018

rustc: Stabilize #[wasm_import_module]
This commit stabilizes the `#[wasm_import_module]` attribute. Tracked by rust-lang#52090
this issue can be attached to foreign modules (`extern { ... }` blocks) and is
used to configured the module name that the imports are listed with. The
WebAssembly specification indicates two utf-8 names are associated with all
imported items, one for the module the item comes from and one for the item
itself. The item itself is configurable in Rust via its identifier or
`#[link_name = "..."]`, but the module name was previously not configurable and
defaulted to `"env"`. This commit ensures that this is also configurable.

Closes rust-lang#52090

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 16, 2018

rustc: Stabilize #[wasm_import_module]
This commit stabilizes the `#[wasm_import_module]` attribute. Tracked by rust-lang#52090
this issue can be attached to foreign modules (`extern { ... }` blocks) and is
used to configured the module name that the imports are listed with. The
WebAssembly specification indicates two utf-8 names are associated with all
imported items, one for the module the item comes from and one for the item
itself. The item itself is configurable in Rust via its identifier or
`#[link_name = "..."]`, but the module name was previously not configurable and
defaulted to `"env"`. This commit ensures that this is also configurable.

Closes rust-lang#52090

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 16, 2018

rustc: Stabilize #[wasm_import_module] as #[link(...)]
This commit stabilizes the `#[wasm_import_module]` attribute as
`#[link(wasm_import_module = "...")]`. Tracked by rust-lang#52090 this new directive in
the `#[link]` attribute is used to configured the module name that the imports
are listed with. The WebAssembly specification indicates two utf-8 names are
associated with all imported items, one for the module the item comes from and
one for the item itself. The item itself is configurable in Rust via its
identifier or `#[link_name = "..."]`, but the module name was previously not
configurable and defaulted to `"env"`. This commit ensures that this is also
configurable.

Closes rust-lang#52090
@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Jul 16, 2018

I've submitted #52445 which folds this into the #[link] attribute, which if merged should close this issue.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 18, 2018

rustc: Stabilize #[wasm_import_module] as #[link(...)]
This commit stabilizes the `#[wasm_import_module]` attribute as
`#[link(wasm_import_module = "...")]`. Tracked by rust-lang#52090 this new directive in
the `#[link]` attribute is used to configured the module name that the imports
are listed with. The WebAssembly specification indicates two utf-8 names are
associated with all imported items, one for the module the item comes from and
one for the item itself. The item itself is configurable in Rust via its
identifier or `#[link_name = "..."]`, but the module name was previously not
configurable and defaulted to `"env"`. This commit ensures that this is also
configurable.

Closes rust-lang#52090

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 18, 2018

rustc: Stabilize #[wasm_import_module] as #[link(...)]
This commit stabilizes the `#[wasm_import_module]` attribute as
`#[link(wasm_import_module = "...")]`. Tracked by rust-lang#52090 this new directive in
the `#[link]` attribute is used to configured the module name that the imports
are listed with. The WebAssembly specification indicates two utf-8 names are
associated with all imported items, one for the module the item comes from and
one for the item itself. The item itself is configurable in Rust via its
identifier or `#[link_name = "..."]`, but the module name was previously not
configurable and defaulted to `"env"`. This commit ensures that this is also
configurable.

Closes rust-lang#52090

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 18, 2018

rustc: Stabilize #[wasm_import_module] as #[link(...)]
This commit stabilizes the `#[wasm_import_module]` attribute as
`#[link(wasm_import_module = "...")]`. Tracked by rust-lang#52090 this new directive in
the `#[link]` attribute is used to configured the module name that the imports
are listed with. The WebAssembly specification indicates two utf-8 names are
associated with all imported items, one for the module the item comes from and
one for the item itself. The item itself is configurable in Rust via its
identifier or `#[link_name = "..."]`, but the module name was previously not
configurable and defaulted to `"env"`. This commit ensures that this is also
configurable.

Closes rust-lang#52090

bors added a commit that referenced this issue Jul 20, 2018

Auto merge of #52445 - alexcrichton:wasm-import-module, r=eddyb
rustc: Stabilize #[wasm_import_module] as #[link(...)]

This commit stabilizes the `#[wasm_import_module]` attribute as
`#[link(wasm_import_module = "...")]`. Tracked by #52090 this new directive in
the `#[link]` attribute is used to configured the module name that the imports
are listed with. The WebAssembly specification indicates two utf-8 names are
associated with all imported items, one for the module the item comes from and
one for the item itself. The item itself is configurable in Rust via its
identifier or `#[link_name = "..."]`, but the module name was previously not
configurable and defaulted to `"env"`. This commit ensures that this is also
configurable.

Closes #52090

@bors bors closed this in #52445 Jul 20, 2018

mati865 added a commit to mati865/rust that referenced this issue Jul 24, 2018

@Jiboo Jiboo referenced this issue Oct 22, 2018

Closed

Import attribute #68

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment