T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improve type imports into components (#5777)

Browse Source 
This commit fixes a panic related to type imports where an import of a
type didn't correctly declare the new type index on the Wasmtime side of
things. Additionally this plumbs more support throughout Wasmtime to
support type imports, namely that they do not need to be supplied
through a `Linker`. This additionally implements a feature where empty
instances, even transitively, do not need to be supplied by a Wasmtime
embedder. This means that instances which only have types, for example,
do not need to be supplied into a `Linker` since no runtime information
for them is required anyway.

Closes #5775
This commit is contained in:
Alex Crichton
2023-02-14 12:02:19 -06:00
committed by GitHub
parent e40a838beb
commit b5e9fb710b
7 changed files with 89 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
crates/environ/src/component/translate.rs
View File

@@ -484,7 +484,7 @@ impl<'a, 'data> Translator<'a, 'data> {
for import in s {
let import = import?;
let ty = self.types.component_type_ref(&import.ty);
self.result.push_typedef(ty);
self.push_typedef(ty);
self.result
.initializers
.push(LocalInitializer::Import(import.name, ty));
@@ -687,6 +687,28 @@ impl<'a, 'data> Translator<'a, 'data> {
Ok(Action::KeepGoing)
}
fn push_typedef(&mut self, ty: TypeDef) {
match ty {
TypeDef::ComponentInstance(idx) => {
self.result
.component_instances
.push(ComponentInstanceType::Index(idx));
}
TypeDef::ComponentFunc(idx) => {
self.result.component_funcs.push(idx);
}
TypeDef::Component(idx) => {
self.result.components.push(ComponentType::Index(idx));
}
TypeDef::Interface(_) => {
self.types.push_component_typedef(ty);
}
// not processed here
TypeDef::CoreFunc(_) | TypeDef::Module(_) => {}
}
}
fn instantiate_module(
&mut self,
module: ModuleIndex,
@@ -852,10 +874,7 @@ impl<'a, 'data> Translator<'a, 'data> {
// the aliased item is directly available from the instance type.
ComponentInstanceType::Index(ty) => {
let (_url, ty) = &self.types[ty].exports[name];
self.result.push_typedef(*ty);
if let TypeDef::Interface(_) = ty {
self.types.push_component_typedef(*ty);
}
self.push_typedef(*ty);
}
// An imported component was instantiated so the type of the aliased
@@ -863,10 +882,7 @@ impl<'a, 'data> Translator<'a, 'data> {
// original component.
ComponentInstanceType::InstantiatedIndex(ty) => {
let (_, ty) = self.types[ty].exports[name];
self.result.push_typedef(ty);
if let TypeDef::Interface(_) = ty {
self.types.push_component_typedef(ty);
}
self.push_typedef(ty);
}
// A static nested component was instantiated which means that the
@@ -1025,23 +1041,3 @@ impl<'a, 'data> Translator<'a, 'data> {
return ret;
}
}
impl Translation<'_> {
fn push_typedef(&mut self, ty: TypeDef) {
match ty {
TypeDef::ComponentInstance(idx) => {
self.component_instances
.push(ComponentInstanceType::Index(idx));
}
TypeDef::ComponentFunc(idx) => {
self.component_funcs.push(idx);
}
TypeDef::Component(idx) => {
self.components.push(ComponentType::Index(idx));
}
// not processed here
TypeDef::Interface(_) | TypeDef::CoreFunc(_) | TypeDef::Module(_) => {}
}
}
}

20
crates/environ/src/component/translate/inline.rs
View File

@@ -75,6 +75,10 @@ pub(super) fn run(
let mut args = HashMap::with_capacity(result.exports.len());
for init in result.initializers.iter() {
let (name, ty) = match *init {
// Imports of types (which are currently always equality-bounded)
// are not required to be specified by the host since it's just for
// type information within the component.
LocalInitializer::Import(_, TypeDef::Interface(_)) => continue,
LocalInitializer::Import(name, ty) => (name, ty),
_ => continue,
};
@@ -355,6 +359,13 @@ impl<'a> Inliner<'a> {
use LocalInitializer::*;
match initializer {
// Importing a type into a component is ignored. All type imports
// are equality-bound right now which means that it's purely
// informational name about the type such as a name to assign it.
// Otherwise type imports have no effect on runtime or such, so skip
// them.
Import(_, TypeDef::Interface(_)) => {}
// When a component imports an item the actual definition of the
// item is looked up here (not at runtime) via its name. The
// arguments provided in our `InlinerFrame` describe how each
@@ -378,12 +389,7 @@ impl<'a> Inliner<'a> {
ComponentItemDef::Func(i) => {
frame.component_funcs.push(i.clone());
}
// The type structure of a component does not change depending
// on how it is instantiated at this time so importing a type
// does not affect the component being instantiated so it's
// ignored.
ComponentItemDef::Type(_ty) => {}
ComponentItemDef::Type(_ty) => unreachable!(),
},
// Lowering a component function to a core wasm function is
@@ -1045,7 +1051,7 @@ impl<'a> ComponentItemDef<'a> {
// FIXME(#4283) should commit one way or another to how this
// should be treated.
TypeDef::Component(_ty) => bail!("root-level component imports are not supported"),
TypeDef::Interface(_ty) => unimplemented!("import of a type"),
TypeDef::Interface(ty) => ComponentItemDef::Type(TypeDef::Interface(ty)),
TypeDef::CoreFunc(_ty) => unreachable!(),
};
Ok(item)

3
crates/wasmtime/src/component/linker.rs
View File

@@ -134,8 +134,7 @@ impl<T> Linker<T> {
let import = self
.strings
.lookup(name)
.and_then(|name| self.map.get(&name))
.ok_or_else(|| anyhow!("import `{name}` not defined"))?;
.and_then(|name| self.map.get(&name));
cx.definition(ty, import)
.with_context(|| format!("import `{name}` has the wrong type"))?;
}

31
crates/wasmtime/src/component/matching.rs
View File

@@ -14,22 +14,23 @@ pub struct TypeChecker<'a> {
}
impl TypeChecker<'_> {
pub fn definition(&self, expected: &TypeDef, actual: &Definition) -> Result<()> {
pub fn definition(&self, expected: &TypeDef, actual: Option<&Definition>) -> Result<()> {
match *expected {
TypeDef::Module(t) => match actual {
Definition::Module(actual) => self.module(&self.types[t], actual),
_ => bail!("expected module found {}", actual.desc()),
Some(Definition::Module(actual)) => self.module(&self.types[t], actual),
_ => bail!("expected module found {}", desc(actual)),
},
TypeDef::ComponentInstance(t) => match actual {
Definition::Instance(actual) => self.instance(&self.types[t], actual),
_ => bail!("expected instance found {}", actual.desc()),
Some(Definition::Instance(actual)) => self.instance(&self.types[t], Some(actual)),
None => self.instance(&self.types[t], None),
_ => bail!("expected instance found {}", desc(actual)),
},
TypeDef::ComponentFunc(t) => match actual {
Definition::Func(actual) => self.func(t, actual),
_ => bail!("expected func found {}", actual.desc()),
Some(Definition::Func(actual)) => self.func(t, actual),
_ => bail!("expected func found {}", desc(actual)),
},
TypeDef::Component(_) => bail!("expected component found {}", actual.desc()),
TypeDef::Interface(_) => bail!("expected type found {}", actual.desc()),
TypeDef::Component(_) => bail!("expected component found {}", desc(actual)),
TypeDef::Interface(_) => bail!("expected type found {}", desc(actual)),
// not possible for valid components to import
TypeDef::CoreFunc(_) => unreachable!(),
@@ -67,7 +68,7 @@ impl TypeChecker<'_> {
Ok(())
}
fn instance(&self, expected: &TypeComponentInstance, actual: &NameMap) -> Result<()> {
fn instance(&self, expected: &TypeComponentInstance, actual: Option<&NameMap>) -> Result<()> {
// Like modules, every export in the expected type must be present in
// the actual type. It's ok, though, to have extra exports in the actual
// type.
@@ -81,8 +82,7 @@ impl TypeChecker<'_> {
let actual = self
.strings
.lookup(name)
.and_then(|name| actual.get(&name))
.ok_or_else(|| anyhow!("instance export `{name}` not defined"))?;
.and_then(|name| actual?.get(&name));
self.definition(expected, actual)
.with_context(|| format!("instance export `{name}` has the wrong type"))?;
}
@@ -94,6 +94,13 @@ impl TypeChecker<'_> {
}
}
fn desc(def: Option<&Definition>) -> &'static str {
match def {
Some(def) => def.desc(),
None => "nothing",
}
}
impl Definition {
fn desc(&self) -> &'static str {
match self {

15
tests/misc_testsuite/component-model/import.wast
View File

@@ -3,3 +3,18 @@
(import "host-return-two" (func $f (result u32)))
(export "x" (func $f)))
"component export `x` is a reexport of an imported function which is not implemented")
(assert_invalid
(component
(import "host-return-two" (instance))
)
"expected instance found func")
;; empty instances don't need to be supplied by the host, even recursively
;; empty instances.
(component
(import "not-provided-by-the-host" (instance))
(import "not-provided-by-the-host2" (instance
(export "x" (instance))
))
)

4
tests/misc_testsuite/component-model/linking.wast
View File

@@ -2,7 +2,7 @@
(component
(import "undefined-name" (core module))
)
"import `undefined-name` not defined")
"expected module found nothing")
(component $i)
(component
(import "i" (instance))
@@ -15,4 +15,4 @@
"expected func found instance")
(assert_unlinkable
(component (import "i" (instance (export "x" (func)))))
"export `x` not defined")
"expected func found nothing")

14
tests/misc_testsuite/component-model/types.wast
View File

@@ -320,3 +320,17 @@
(instance $c (instantiate $c (with "x" (component $x))))
)
(component
(type $t1 u64)
(import "a" (type $t2 (eq $t1)))
(import "b" (type $t3 (eq $t2)))
)
(component
(import "a" (instance
(type $t1 u64)
(export $t2 "a" (type (eq $t1)))
(export "b" (type (eq $t2)))
))
)