Skip to content

Commit

Permalink
fix: allow dynamic owned resources to be used as borrowed parameters (b…
Browse files Browse the repository at this point in the history
…ytecodealliance#7783)

* fix: allow dynamic owned resources to be used as borrowed parameters

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>

* tests: add `can_use_own_for_borrow` test

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>

---------

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
(cherry picked from commit 2c86e26)
  • Loading branch information
rvolosatovs authored and vigoo committed Feb 23, 2024
1 parent b5eace2 commit df86b3e
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 9 deletions.
3 changes: 2 additions & 1 deletion crates/wasmtime/src/component/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,8 @@ impl Func {
}

for (param, ty) in params.iter().zip(param_tys.iter()) {
ty.check(param).context("type mismatch with parameters")?;
ty.is_supertype_of(param)
.context("type mismatch with parameters")?;
}

self.call_raw(
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmtime/src/component/func/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ where
let mut cx = LowerContext::new(store, &options, types, instance);
let instance = cx.instance_type();
for (val, ty) in result_vals.iter().zip(result_tys.types.iter()) {
Type::from(ty, &instance).check(val)?;
Type::from(ty, &instance).is_supertype_of(val)?;
}
if let Some(cnt) = result_tys.abi.flat_count(MAX_FLAT_RESULTS) {
let mut dst = storage[..cnt].iter_mut();
Expand Down
11 changes: 9 additions & 2 deletions crates/wasmtime/src/component/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -675,9 +675,16 @@ impl Type {
}
}

pub(crate) fn check(&self, value: &Val) -> Result<()> {
/// Checks whether type of `value` is a subtype of `self`.
///
/// # Errors
///
/// Returns an error in case of a type mismatch
pub(crate) fn is_supertype_of(&self, value: &Val) -> Result<()> {
let other = &value.ty();
if self == other {
if self == other
|| matches!((self, other), (Self::Borrow(s), Self::Own(other)) if s == other)
{
Ok(())
} else if mem::discriminant(self) != mem::discriminant(other) {
Err(anyhow!(
Expand Down
12 changes: 7 additions & 5 deletions crates/wasmtime/src/component/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl List {
let element_type = ty.ty();
for (index, value) in values.iter().enumerate() {
element_type
.check(value)
.is_supertype_of(value)
.with_context(|| format!("type mismatch for element {index} of list"))?;
}

Expand Down Expand Up @@ -83,7 +83,7 @@ impl Record {
if name == field.name {
field
.ty
.check(&value)
.is_supertype_of(&value)
.with_context(|| format!("type mismatch for field {name} of record"))?;

values.push(value);
Expand Down Expand Up @@ -150,7 +150,7 @@ impl Tuple {
}

for (index, (value, ty)) in values.iter().zip(ty.types()).enumerate() {
ty.check(value)
ty.is_supertype_of(value)
.with_context(|| format!("type mismatch for field {index} of tuple"))?;
}

Expand Down Expand Up @@ -256,7 +256,7 @@ impl Variant {
fn typecheck_payload(name: &str, case_type: Option<&Type>, value: Option<&Val>) -> Result<()> {
match (case_type, value) {
(Some(expected), Some(actual)) => expected
.check(&actual)
.is_supertype_of(&actual)
.with_context(|| format!("type mismatch for case {name} of variant")),
(None, None) => Ok(()),
(Some(_), None) => bail!("expected a payload for case `{name}`"),
Expand Down Expand Up @@ -341,7 +341,9 @@ impl OptionVal {
pub fn new(ty: &types::OptionType, value: Option<Val>) -> Result<Self> {
let value = value
.map(|value| {
ty.ty().check(&value).context("type mismatch for option")?;
ty.ty()
.is_supertype_of(&value)
.context("type mismatch for option")?;

Ok::<_, Error>(value)
})
Expand Down
66 changes: 66 additions & 0 deletions tests/all/component_model/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,72 @@ fn cannot_use_borrow_for_own() -> Result<()> {
Ok(())
}

#[test]
fn can_use_own_for_borrow() -> Result<()> {
let engine = super::engine();
let c = Component::new(
&engine,
r#"
(component
(import "t" (type $t (sub resource)))
(core func $drop (canon resource.drop $t))
(core module $m
(import "" "drop" (func $drop (param i32)))
(func (export "f") (param i32)
(call $drop (local.get 0))
)
)
(core instance $i (instantiate $m
(with "" (instance
(export "drop" (func $drop))
))
))
(func (export "f") (param "x" (borrow $t))
(canon lift (core func $i "f")))
)
"#,
)?;

struct MyType;

let mut store = Store::new(&engine, ());
let mut linker = Linker::new(&engine);
let ty_idx = linker
.root()
.resource("t", ResourceType::host::<MyType>(), |_, _| Ok(()))?;
let i_pre = linker.instantiate_pre(&c)?;
let i = i_pre.instantiate(&mut store)?;

let f = i.get_func(&mut store, "f").unwrap();
let f_typed = i.get_typed_func::<(&Resource<MyType>,), ()>(&mut store, "f")?;

let resource = Resource::new_own(100);
f_typed.call(&mut store, (&resource,))?;
f_typed.post_return(&mut store)?;

let resource = Resource::new_borrow(200);
f_typed.call(&mut store, (&resource,))?;
f_typed.post_return(&mut store)?;

let resource =
Resource::<MyType>::new_own(300).try_into_resource_any(&mut store, &i_pre, ty_idx)?;
f.call(&mut store, &[Val::Resource(resource)], &mut [])?;
f.post_return(&mut store)?;
resource.resource_drop(&mut store)?;

// TODO: Enable once https://github.com/bytecodealliance/wasmtime/issues/7793 is fixed
//let resource =
// Resource::<MyType>::new_borrow(400).try_into_resource_any(&mut store, &i_pre, ty_idx)?;
//f.call(&mut store, &[Val::Resource(resource)], &mut [])?;
//f.post_return(&mut store)?;
//resource.resource_drop(&mut store)?;

Ok(())
}

#[test]
fn passthrough_wrong_type() -> Result<()> {
let engine = super::engine();
Expand Down

0 comments on commit df86b3e

Please sign in to comment.