Skip to content

Commit

Permalink
Propagate optional import names to the wasmtime/C API
Browse files Browse the repository at this point in the history
With the module linking proposal the field name on imports is now
optional, and only the module is required to be specified. This commit
propagates this API change to the boundary of wasmtime's API, ensuring
consumers are aware of what's optional with module linking and what
isn't. Note that it's expected that all existing users will either
update accordingly or unwrap the result since module linking is
presumably disabled.
  • Loading branch information
alexcrichton committed Nov 23, 2020
1 parent 1dd20b4 commit 62be684
Show file tree
Hide file tree
Showing 18 changed files with 132 additions and 93 deletions.
16 changes: 8 additions & 8 deletions cranelift/wasm/src/environ/dummy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ impl<'data> ModuleEnvironment<'data> for DummyEnvironment {
&mut self,
index: TypeIndex,
module: &'data str,
field: &'data str,
field: Option<&'data str>,
) -> WasmResult<()> {
assert_eq!(
self.info.functions.len(),
Expand All @@ -591,7 +591,7 @@ impl<'data> ModuleEnvironment<'data> for DummyEnvironment {
self.info.functions.push(Exportable::new(index));
self.info
.imported_funcs
.push((String::from(module), String::from(field)));
.push((String::from(module), String::from(field.unwrap())));
Ok(())
}

Expand All @@ -609,12 +609,12 @@ impl<'data> ModuleEnvironment<'data> for DummyEnvironment {
&mut self,
global: Global,
module: &'data str,
field: &'data str,
field: Option<&'data str>,
) -> WasmResult<()> {
self.info.globals.push(Exportable::new(global));
self.info
.imported_globals
.push((String::from(module), String::from(field)));
.push((String::from(module), String::from(field.unwrap())));
Ok(())
}

Expand All @@ -627,12 +627,12 @@ impl<'data> ModuleEnvironment<'data> for DummyEnvironment {
&mut self,
table: Table,
module: &'data str,
field: &'data str,
field: Option<&'data str>,
) -> WasmResult<()> {
self.info.tables.push(Exportable::new(table));
self.info
.imported_tables
.push((String::from(module), String::from(field)));
.push((String::from(module), String::from(field.unwrap())));
Ok(())
}

Expand Down Expand Up @@ -672,12 +672,12 @@ impl<'data> ModuleEnvironment<'data> for DummyEnvironment {
&mut self,
memory: Memory,
module: &'data str,
field: &'data str,
field: Option<&'data str>,
) -> WasmResult<()> {
self.info.memories.push(Exportable::new(memory));
self.info
.imported_memories
.push((String::from(module), String::from(field)));
.push((String::from(module), String::from(field.unwrap())));
Ok(())
}

Expand Down
14 changes: 7 additions & 7 deletions cranelift/wasm/src/environ/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -674,31 +674,31 @@ pub trait ModuleEnvironment<'data>: TargetEnvironment {
&mut self,
index: TypeIndex,
module: &'data str,
field: &'data str,
field: Option<&'data str>,
) -> WasmResult<()>;

/// Declares a table import to the environment.
fn declare_table_import(
&mut self,
table: Table,
module: &'data str,
field: &'data str,
field: Option<&'data str>,
) -> WasmResult<()>;

/// Declares a memory import to the environment.
fn declare_memory_import(
&mut self,
memory: Memory,
module: &'data str,
field: &'data str,
field: Option<&'data str>,
) -> WasmResult<()>;

/// Declares an event import to the environment.
fn declare_event_import(
&mut self,
event: Event,
module: &'data str,
field: &'data str,
field: Option<&'data str>,
) -> WasmResult<()> {
drop((event, module, field));
Err(WasmError::Unsupported("wasm events".to_string()))
Expand All @@ -709,15 +709,15 @@ pub trait ModuleEnvironment<'data>: TargetEnvironment {
&mut self,
global: Global,
module: &'data str,
field: &'data str,
field: Option<&'data str>,
) -> WasmResult<()>;

/// Declares a module import to the environment.
fn declare_module_import(
&mut self,
ty_index: TypeIndex,
module: &'data str,
field: &'data str,
field: Option<&'data str>,
) -> WasmResult<()> {
drop((ty_index, module, field));
Err(WasmError::Unsupported("module linking".to_string()))
Expand All @@ -728,7 +728,7 @@ pub trait ModuleEnvironment<'data>: TargetEnvironment {
&mut self,
ty_index: TypeIndex,
module: &'data str,
field: &'data str,
field: Option<&'data str>,
) -> WasmResult<()> {
drop((ty_index, module, field));
Err(WasmError::Unsupported("module linking".to_string()))
Expand Down
16 changes: 7 additions & 9 deletions cranelift/wasm/src/sections_translator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,27 +156,25 @@ pub fn parse_import_section<'data>(

for entry in imports {
let import = entry?;
let module_name = import.module;
let field_name = import.field.unwrap(); // TODO Handle error when module linking is implemented.
match entity_type(import.ty, environ)? {
EntityType::Function(idx) => {
environ.declare_func_import(idx, module_name, field_name)?;
environ.declare_func_import(idx, import.module, import.field)?;
}
EntityType::Module(idx) => {
environ.declare_module_import(idx, module_name, field_name)?;
environ.declare_module_import(idx, import.module, import.field)?;
}
EntityType::Instance(idx) => {
environ.declare_instance_import(idx, module_name, field_name)?;
environ.declare_instance_import(idx, import.module, import.field)?;
}
EntityType::Memory(ty) => {
environ.declare_memory_import(ty, module_name, field_name)?;
environ.declare_memory_import(ty, import.module, import.field)?;
}
EntityType::Event(e) => environ.declare_event_import(e, module_name, field_name)?,
EntityType::Event(e) => environ.declare_event_import(e, import.module, import.field)?,
EntityType::Global(ty) => {
environ.declare_global_import(ty, module_name, field_name)?;
environ.declare_global_import(ty, import.module, import.field)?;
}
EntityType::Table(ty) => {
environ.declare_table_import(ty, module_name, field_name)?;
environ.declare_table_import(ty, import.module, import.field)?;
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions crates/c-api/include/doc-wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,8 @@
*
* This function takes ownership of the `module`, `name`, and
* #wasm_externtype_t arguments. The caller is responsible for deleting the
* returned value.
* returned value. Note that `name` can be `NULL` where in the module linking
* proposal the import name can be omitted.
*
* \fn const wasm_name_t* wasm_importtype_module(const wasm_importtype_t *);
* \brief Returns the module this import is importing from.
Expand All @@ -972,7 +973,9 @@
* \brief Returns the name this import is importing from.
*
* The returned memory is owned by the #wasm_importtype_t argument, the caller
* should not deallocate it.
* should not deallocate it. Note that `NULL` can be returned which means
* that the import name is not provided. This is for imports with the module
* linking proposal that only have the module specified.
*
* \fn const wasm_externtype_t* wasm_importtype_type(const wasm_importtype_t *);
* \brief Returns the type of item this import is importing.
Expand Down
11 changes: 7 additions & 4 deletions crates/c-api/src/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,17 +116,20 @@ pub extern "C" fn wasmtime_linker_get_default(
pub extern "C" fn wasmtime_linker_get_one_by_name(
linker: &wasmtime_linker_t,
module: &wasm_name_t,
name: &wasm_name_t,
name: Option<&wasm_name_t>,
item_ptr: &mut *mut wasm_extern_t,
) -> Option<Box<wasmtime_error_t>> {
let linker = &linker.linker;
let module = match str::from_utf8(module.as_slice()) {
Ok(s) => s,
Err(_) => return bad_utf8(),
};
let name = match str::from_utf8(name.as_slice()) {
Ok(s) => s,
Err(_) => return bad_utf8(),
let name = match name {
Some(name) => match str::from_utf8(name.as_slice()) {
Ok(s) => Some(s),
Err(_) => return bad_utf8(),
},
None => None,
};
handle_result(linker.get_one_by_name(module, name), |which| {
*item_ptr = Box::into_raw(Box::new(wasm_extern_t { which }))
Expand Down
24 changes: 21 additions & 3 deletions crates/c-api/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@ pub extern "C" fn wasmtime_module_new(
handle_result(Module::from_binary(&engine.engine, binary), |module| {
let imports = module
.imports()
.map(|i| wasm_importtype_t::new(i.module().to_owned(), i.name().to_owned(), i.ty()))
.map(|i| {
wasm_importtype_t::new(
i.module().to_owned(),
i.name().map(|s| s.to_owned()),
i.ty(),
)
})
.collect::<Vec<_>>();
let exports = module
.exports()
Expand Down Expand Up @@ -118,7 +124,13 @@ pub extern "C" fn wasm_module_obtain(
}
let imports = module
.imports()
.map(|i| wasm_importtype_t::new(i.module().to_owned(), i.name().to_owned(), i.ty()))
.map(|i| {
wasm_importtype_t::new(
i.module().to_owned(),
i.name().map(|s| s.to_owned()),
i.ty(),
)
})
.collect::<Vec<_>>();
let exports = module
.exports()
Expand Down Expand Up @@ -175,7 +187,13 @@ pub extern "C" fn wasmtime_module_deserialize(
|module| {
let imports = module
.imports()
.map(|i| wasm_importtype_t::new(i.module().to_owned(), i.name().to_owned(), i.ty()))
.map(|i| {
wasm_importtype_t::new(
i.module().to_owned(),
i.name().map(|s| s.to_owned()),
i.ty(),
)
})
.collect::<Vec<_>>();
let exports = module
.exports()
Expand Down
22 changes: 14 additions & 8 deletions crates/c-api/src/types/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use wasmtime::ExternType;
#[derive(Clone)]
pub struct wasm_importtype_t {
pub(crate) module: String,
pub(crate) name: String,
pub(crate) name: Option<String>,
pub(crate) ty: ExternType,
module_cache: OnceCell<wasm_name_t>,
name_cache: OnceCell<wasm_name_t>,
Expand All @@ -16,7 +16,7 @@ pub struct wasm_importtype_t {
wasmtime_c_api_macros::declare_ty!(wasm_importtype_t);

impl wasm_importtype_t {
pub(crate) fn new(module: String, name: String, ty: ExternType) -> wasm_importtype_t {
pub(crate) fn new(module: String, name: Option<String>, ty: ExternType) -> wasm_importtype_t {
wasm_importtype_t {
module,
name,
Expand All @@ -31,13 +31,16 @@ impl wasm_importtype_t {
#[no_mangle]
pub extern "C" fn wasm_importtype_new(
module: &mut wasm_name_t,
name: &mut wasm_name_t,
name: Option<&mut wasm_name_t>,
ty: Box<wasm_externtype_t>,
) -> Option<Box<wasm_importtype_t>> {
let module = module.take();
let name = name.take();
let name = name.map(|n| n.take());
let module = String::from_utf8(module).ok()?;
let name = String::from_utf8(name).ok()?;
let name = match name {
Some(name) => Some(String::from_utf8(name).ok()?),
None => None,
};
Some(Box::new(wasm_importtype_t::new(module, name, ty.ty())))
}

Expand All @@ -48,9 +51,12 @@ pub extern "C" fn wasm_importtype_module(it: &wasm_importtype_t) -> &wasm_name_t
}

#[no_mangle]
pub extern "C" fn wasm_importtype_name(it: &wasm_importtype_t) -> &wasm_name_t {
it.name_cache
.get_or_init(|| wasm_name_t::from_name(it.name.clone()))
pub extern "C" fn wasm_importtype_name(it: &wasm_importtype_t) -> Option<&wasm_name_t> {
let name = it.name.as_ref()?;
Some(
it.name_cache
.get_or_init(|| wasm_name_t::from_name(name.to_string())),
)
}

#[no_mangle]
Expand Down
2 changes: 1 addition & 1 deletion crates/c-api/src/wasi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ pub extern "C" fn wasi_instance_bind_import<'a>(
import: &wasm_importtype_t,
) -> Option<&'a wasm_extern_t> {
let module = &import.module;
let name = str::from_utf8(import.name.as_bytes()).ok()?;
let name = str::from_utf8(import.name.as_ref()?.as_bytes()).ok()?;

let export = match &instance.wasi {
WasiInstance::Preview1(wasi) => {
Expand Down
2 changes: 1 addition & 1 deletion crates/environ/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ pub struct Module {
pub name: Option<String>,

/// All import records, in the order they are declared in the module.
pub imports: Vec<(String, String, EntityIndex)>,
pub imports: Vec<(String, Option<String>, EntityIndex)>,

/// Exported entities.
pub exports: IndexMap<String, EntityIndex>,
Expand Down
Loading

0 comments on commit 62be684

Please sign in to comment.