Skip to content

Commit

Permalink
Merge pull request #2446 from alexcrichton/option-name
Browse files Browse the repository at this point in the history
Propagate optional import names to the wasmtime/C API
  • Loading branch information
fitzgen authored Nov 24, 2020
2 parents 2db6a09 + ba141ec commit 128c3bd
Show file tree
Hide file tree
Showing 18 changed files with 134 additions and 95 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 128c3bd

Please sign in to comment.