Fix taking owned resource handles in Rust imports (#669)

* Fix taking owned resource handles in Rust imports

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

* Fix test compilation

* Ignore some more resources tests

Author: alexcrichton

crates/core/src/lib.rs

@@ -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 {
@@ -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;
     }
 }
 
@@ -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) => {

crates/go/tests/codegen.rs

@@ -18,6 +18,7 @@ macro_rules! codegen_test {
     (resource_borrow_in_record $name:tt $test:tt) => {};
     (resource_borrow_in_record_export $name:tt $test:tt) => {};
     (resources_in_aggregates $name:tt $test:tt) => {};
+    (issue668 $name:tt $test:tt) => {};
 
     ($id:ident $name:tt $test:tt) => {
         #[test]

crates/rust-lib/src/lib.rs

@@ -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
@@ -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);

crates/teavm-java/tests/codegen.rs

@@ -17,6 +17,7 @@ macro_rules! codegen_test {
     (resource_borrow_in_record_export $name:tt $test:tt) => {};
     (same_names5 $name:tt $test:tt) => {};
     (resources_in_aggregates $name:tt $test:tt) => {};
+    (issue668 $name:tt $test:tt) => {};
 
     ($id:ident $name:tt $test:tt) => {
         #[test]

tests/codegen/issue668.wit

@@ -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
+}

tests/runtime/ownership/borrowing-duplicate-if-necessary.rs

@@ -24,7 +24,7 @@ impl Guest for Exports {
             lists::foo(value)
         );
 
-        thing_in::bar(thing_in::Thing {
+        thing_in::bar(&thing_in::Thing {
             name: "thing 1",
             value: &["some value", "another value"],
         });
@@ -38,7 +38,7 @@ impl Guest for Exports {
                 name: "thing 1".to_owned(),
                 value: vec!["some value".to_owned(), "another value".to_owned()],
             },
-            thing_in_and_out::baz(value)
+            thing_in_and_out::baz(&value)
         );
     }
 }

tests/runtime/ownership/borrowing.rs

@@ -24,7 +24,7 @@ impl Guest for Exports {
             lists::foo(value)
         );
 
-        thing_in::bar(thing_in::Thing {
+        thing_in::bar(&thing_in::Thing {
             name: "thing 1",
             value: &["some value", "another value"],
         });