Skip to content

Commit

Permalink
Fix taking owned resource handles in Rust imports
Browse files Browse the repository at this point in the history
I went ahead and did a bit more refactoring at the logic here since I
think the old conditions aren't as applicable any more (they haven't
aged well)

Closes #668
  • Loading branch information
alexcrichton committed Sep 14, 2023
1 parent 9e20991 commit 3e353c0
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 8 deletions.
9 changes: 8 additions & 1 deletion crates/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ pub struct TypeInfo {

/// Whether or not this type (transitively) has a borrow handle.
pub has_borrow_handle: bool,

/// Whether or not this type (transitively) has an own handle.
pub has_own_handle: bool,
}

impl std::ops::BitOrAssign for TypeInfo {
Expand All @@ -52,6 +55,7 @@ impl std::ops::BitOrAssign for TypeInfo {
self.has_list |= rhs.has_list;
self.has_resource |= rhs.has_resource;
self.has_borrow_handle |= rhs.has_borrow_handle;
self.has_own_handle |= rhs.has_own_handle;
}
}

Expand Down Expand Up @@ -164,7 +168,10 @@ impl Types {
info.has_resource = true;
}
TypeDefKind::Handle(handle) => {
info.has_borrow_handle = matches!(handle, Handle::Borrow(_));
match handle {
Handle::Borrow(_) => info.has_borrow_handle = true,
Handle::Own(_) => info.has_own_handle = true,
}
info.has_resource = true;
}
TypeDefKind::Tuple(t) => {
Expand Down
23 changes: 16 additions & 7 deletions crates/rust-lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,12 +305,21 @@ pub trait RustGenerator<'a> {
let lt = self.lifetime_for(&info, mode);
let ty = &self.resolve().types[id];
if ty.name.is_some() {
// If this type has a list internally, no lifetime is being printed,
// but we're in a borrowed mode, then that means we're in a borrowed
// context and don't want ownership of the type but we're using an
// owned type definition. Inject a `&` in front to indicate that, at
// the API level, ownership isn't required.
if (info.has_list || info.has_borrow_handle) && lt.is_none() {
// If `mode` is borrowed then that means literal ownership of the
// input type is not necessarily required. In this situation we
// ideally want to put a `&` in front to statically indicate this.
// That's not required in all situations however and is only really
// critical for lists which otherwise would transfer ownership of
// the allocation to this function.
//
// Note, though, that if the type has an `own<T>` inside of it then
// it is actually required that we take ownership since Rust is
// losing access to those handles.
//
// Check here if the type has the right shape and if we're in the
// right mode, and if those conditions are met a lifetime is
// printed.
if info.has_list && !info.has_own_handle {
if let TypeMode::AllBorrowed(lt)
| TypeMode::LeafBorrowed(lt)
| TypeMode::HandlesBorrowed(lt) = mode
Expand Down Expand Up @@ -1032,7 +1041,7 @@ pub trait RustGenerator<'a> {
TypeMode::AllBorrowed(s) | TypeMode::LeafBorrowed(s) | TypeMode::HandlesBorrowed(s) => {
s
}
_ => return None,
TypeMode::Owned => return None,
};
if info.has_borrow_handle {
return Some(lt);
Expand Down
20 changes: 20 additions & 0 deletions tests/codegen/issue668.wit
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package test:test

interface test-import {
resource resource-a {
constructor(id: u32)
}

record record-a {
resource-a: resource-a,
resources: list<resource-a>,
}

resource resource-b {
make: static func(record-a: record-a)
}
}

world test-world {
import test-import
}

0 comments on commit 3e353c0

Please sign in to comment.