Skip to content

Commit

Permalink
Fix generated bindings for list-of-borrows (#670)
Browse files Browse the repository at this point in the history
This commit fixes an issue for imported functions taking
`list<borrow<T>>` arguments. Previously they were erroneously passed
through as lists-of-pointers and now they're correctly mapped to
list-of-handle-indexes in the ABI. This required rejiggering a bit how
types are generated and what's considered owned when, namely requiring
that all own handles use `Vec<T>` at the boundary since they need to
take ownership.
  • Loading branch information
alexcrichton authored Sep 18, 2023
1 parent f6004cd commit 0b19bc3
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 19 deletions.
57 changes: 39 additions & 18 deletions crates/rust-lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,9 @@ pub trait RustGenerator<'a> {
// If the type recursively owns data and it's a
// variant/record/list, then we need to place the
// lifetime parameter on the type as well.
if (info.has_list || info.has_borrow_handle) && needs_generics(self.resolve(), &ty.kind)
if (info.has_list || info.has_borrow_handle)
&& !info.has_own_handle
&& needs_generics(self.resolve(), &ty.kind)
{
self.print_generics(lt);
}
Expand Down Expand Up @@ -470,6 +472,11 @@ pub trait RustGenerator<'a> {
} else {
mode
};
// Lists with `own` handles must always be owned
let mode = match *ty {
Type::Id(id) if self.info(id).has_own_handle => TypeMode::Owned,
_ => mode,
};
match mode {
TypeMode::AllBorrowed(lt) => {
self.print_borrowed_slice(false, ty, lt, next_mode);
Expand Down Expand Up @@ -529,21 +536,29 @@ pub trait RustGenerator<'a> {

fn modes_of(&self, ty: TypeId) -> Vec<(String, TypeMode)> {
let info = self.info(ty);
// If this type isn't actually used, no need to generate it.
if !info.owned && !info.borrowed {
return Vec::new();
}
let mut result = Vec::new();
let first_mode =
if info.owned || !info.borrowed || matches!(self.ownership(), Ownership::Owning) {
if info.has_borrow_handle {
TypeMode::HandlesBorrowed("'a")
} else {
TypeMode::Owned
}

// Prioritize generating an "owned" type. This is done to simplify
// generated bindings by default. Borrowed handles always use a borrow,
// however.
let first_mode = if info.owned
|| !info.borrowed
|| matches!(self.ownership(), Ownership::Owning)
|| info.has_own_handle
{
if info.has_borrow_handle {
TypeMode::HandlesBorrowed("'a")
} else {
assert!(!self.uses_two_names(&info));
TypeMode::AllBorrowed("'a")
};
TypeMode::Owned
}
} else {
assert!(!self.uses_two_names(&info));
TypeMode::AllBorrowed("'a")
};
result.push((self.result_name(ty), first_mode));
if self.uses_two_names(&info) {
result.push((self.param_name(ty), TypeMode::AllBorrowed("'a")));
Expand Down Expand Up @@ -1025,15 +1040,21 @@ pub trait RustGenerator<'a> {
}

fn uses_two_names(&self, info: &TypeInfo) -> bool {
info.has_list
// Types are only duplicated if explicitly requested ...
matches!(
self.ownership(),
Ownership::Borrowing {
duplicate_if_necessary: true
}
)
// ... and if they're both used in a borrowed/owned context
&& info.borrowed
&& info.owned
&& matches!(
self.ownership(),
Ownership::Borrowing {
duplicate_if_necessary: true
}
)
// ... and they have a list ...
&& info.has_list
// ... and if there's NOT an `own` handle since those are always
// done by ownership.
&& !info.has_own_handle
}

fn lifetime_for(&self, info: &TypeInfo, mode: TypeMode) -> Option<&'static str> {
Expand Down
8 changes: 7 additions & 1 deletion crates/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1462,7 +1462,13 @@ impl Bindgen for FunctionBindgen<'_, '_> {
}

fn is_list_canonical(&self, resolve: &Resolve, ty: &Type) -> bool {
resolve.all_bits_valid(ty)
if !resolve.all_bits_valid(ty) {
return false;
}
match ty {
Type::Id(id) => !self.gen.gen.types.get(*id).has_resource,
_ => true,
}
}

fn emit(
Expand Down

0 comments on commit 0b19bc3

Please sign in to comment.