Skip to content

Commit

Permalink
address review feedback
Browse files Browse the repository at this point in the history
- add `type RawRep<T> = Option<T>` alias
- rename `Resource::take` to `Resource::into_inner`
- add `Resource::lift_borrow` and use it in code generator

Signed-off-by: Joel Dice <[email protected]>
  • Loading branch information
dicej committed Nov 13, 2023
1 parent 29d50d6 commit c1887d5
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 21 deletions.
20 changes: 15 additions & 5 deletions crates/guest-rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ pub mod rt {
}
}

type RawRep<T> = Option<T>;

/// A type which represents a component model resource, either imported or
/// exported into this component.
///
Expand Down Expand Up @@ -250,19 +252,27 @@ impl<T: WasmResource> Resource<T> {
where
T: RustResource,
{
let _ = Box::from_raw(rep as *mut Option<T>);
let _ = Box::from_raw(rep as *mut RawRep<T>);
}

/// Takes back ownership of the object, dropping the resource handle.
pub fn take(resource: Self) -> T
pub fn into_inner(resource: Self) -> T
where
T: RustResource,
{
unsafe {
let rep = T::rep(resource.handle);
Option::take(&mut *(rep as *mut Option<T>)).unwrap()
RawRep::take(&mut *(rep as *mut RawRep<T>)).unwrap()
}
}

#[doc(hidden)]
pub unsafe fn lift_borrow<'a>(rep: usize) -> &'a T
where
T: RustResource,
{
RawRep::as_ref(&*(rep as *const RawRep<T>)).unwrap()
}
}

impl<T: RustResource> Deref for Resource<T> {
Expand All @@ -271,7 +281,7 @@ impl<T: RustResource> Deref for Resource<T> {
fn deref(&self) -> &T {
unsafe {
let rep = T::rep(self.handle);
Option::as_ref(&*(rep as *const Option<T>)).unwrap()
RawRep::as_ref(&*(rep as *const RawRep<T>)).unwrap()
}
}
}
Expand All @@ -280,7 +290,7 @@ impl<T: RustResource> DerefMut for Resource<T> {
fn deref_mut(&mut self) -> &mut T {
unsafe {
let rep = T::rep(self.handle);
Option::as_mut(&mut *(rep as *mut Option<T>)).unwrap()
RawRep::as_mut(&mut *(rep as *mut RawRep<T>)).unwrap()
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion crates/rust/src/bindgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,10 @@ impl Bindgen for FunctionBindgen<'_, '_> {
.as_deref()
.unwrap()
.to_upper_camel_case();
format!("Option::as_ref(&*({op} as u32 as usize as *const Option<{name}>)).unwrap()")
let rt = self.gen.gen.runtime_path();
format!(
"{rt}::Resource::<{name}>::lift_borrow({op} as u32 as usize)"
)
}
Handle::Own(_) => {
let name = self.gen.type_path(resource, true);
Expand Down
3 changes: 3 additions & 0 deletions crates/test-rust-wasm/src/bin/resource_into_inner.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
include!("../../../../tests/runtime/resource_into_inner/wasm.rs");

fn main() {}
3 changes: 0 additions & 3 deletions crates/test-rust-wasm/src/bin/resource_take.rs

This file was deleted.

2 changes: 1 addition & 1 deletion tests/runtime/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ mod resource_borrow_import;
mod resource_borrow_in_record;
mod resource_floats;
mod resource_import_and_export;
mod resource_take;
mod resource_into_inner;
mod resource_with_lists;
mod resources;
mod smoke;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
use wasmtime::Store;

wasmtime::component::bindgen!(in "tests/runtime/resource_take");
wasmtime::component::bindgen!(in "tests/runtime/resource_into_inner");

use exports::test::resource_take::test::Test;
use exports::test::resource_into_inner::test::Test;

#[test]
fn run() -> anyhow::Result<()> {
crate::run_test(
"resource_take",
"resource_into_inner",
|_| Ok(()),
|store, component, linker| {
let (u, e) = ResourceTake::instantiate(store, component, linker)?;
let (u, e) = ResourceIntoInner::instantiate(store, component, linker)?;
Ok((u.interface0, e))
},
run_test,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
wit_bindgen::generate!({
path: "../../tests/runtime/resource_take",
path: "../../tests/runtime/resource_into_inner",
exports: {
world: Test,
"test:resource-take/test": Test,
"test:resource-take/test/thing": MyThing,
"test:resource-into-inner/test": Test,
"test:resource-into-inner/test/thing": MyThing,
},
});

use exports::test::resource_take::test::{Guest, GuestThing};
use exports::test::resource_into_inner::test::{Guest, GuestThing};
use wit_bindgen::rt::Resource;

pub struct Test;
Expand All @@ -17,7 +17,7 @@ impl Guest for Test {
let text = "Jabberwocky";
assert_eq!(
text,
&Resource::take(Resource::new(MyThing(text.to_string()))).0
&Resource::into_inner(Resource::new(MyThing(text.to_string()))).0
);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package test:resource-take;
package test:resource-into-inner;

interface test {
resource thing {
Expand All @@ -8,6 +8,6 @@ interface test {
test: func();
}

world resource-take {
world resource-into-inner {
export test;
}

0 comments on commit c1887d5

Please sign in to comment.