Avoid unnecessarily coping result inner value if it doesn't require ownership

crates/luaffi/src/future.rs

@@ -177,6 +177,15 @@ unsafe impl<F: Future<Output: IntoFfi> + 'static> IntoFfi for lua_future<F> {
         self
     }
 
+    fn require_owned() -> bool {
+        // future always requires full ownership of itself even if it's a "temporary", because we
+        // must yield a full cdata to the runtime not a cdata containing a pointer to the future. if
+        // this is set to false, postlude might receive a reference lua_future cdata instead of a
+        // full lua_future cdata, and the runtime might incorrectly read the pointer value as
+        // lua_future itself (it does not dereference it).
+        true
+    }
+
     fn postlude(ret: &str) -> impl Display {
         // When returning a future from Rust to Lua, yield it immediately to the runtime which will
         // poll it to completion in the background, then take the fulfilled value once the thread

crates/luaffi/src/lib.rs

@@ -417,6 +417,10 @@ pub unsafe trait IntoFfi: Sized {
         }
     }
 
+    fn require_owned() -> bool {
+        true
+    }
+
     fn postlude(_ret: &str) -> impl Display {
         ""
     }

crates/luaffi/src/result.rs

@@ -1,6 +1,6 @@
 use crate::{
     __internal::{disp, display},
-    Cdef, CdefBuilder, IntoFfi, Type, TypeBuilder, TypeType,
+    Cdef, CdefBuilder, IntoFfi, KEEP_FN, Type, TypeBuilder, TypeType,
     string::{DROP_BUFFER_FN, lua_buffer},
 };
 use std::{ffi::c_int, fmt::Display};
@@ -49,22 +49,44 @@ unsafe impl<T: IntoFfi, E: Display> IntoFfi for Result<T, E> {
         }
     }
 
+    fn require_owned() -> bool {
+        // lua_result is only used to transmit information about whether an operation succeeded or
+        // not and is forgotten immediately after use, so there is no need for an owned result
+        false
+    }
+
     fn postlude(ret: &str) -> impl Display {
         disp(move |f| {
-            let ct = T::Into::name();
             write!(f, "if {ret}.__tag ~= 0 then ")?;
             match T::Into::ty() {
-                TypeType::Void => write!(f, "{ret} = nil; "), // for void results, we don't have a __value
+                TypeType::Void => write!(f, "{ret} = nil; ")?, // for void results, we don't have a __value
-                TypeType::Primitive => write!(f, "{ret} = {ret}.__value; "),
+                TypeType::Primitive => {
-                TypeType::Aggregate => write!(f, "{ret} = __new(__ct.{ct}, {ret}.__value); "),
+                    // can always copy primitives to stack
-            }?;
+                    write!(f, "{ret} = {ret}.__value; {}", T::postlude(ret))?;
+                }
+                TypeType::Aggregate => {
+                    let ct = T::Into::name();
+                    if T::require_owned() {
+                        // inner value requires ownership; copy it into its own cdata and forget
+                        // result.
+                        write!(f, "{ret} = __new(__ct.{ct}, {ret}.__value); ")?;
                         write!(f, "{}", T::postlude(ret))?;
+                    } else {
+                        // inner value is a "temporary" itself and doesn't require full ownership of
+                        // itself. we just need to keep the result object alive until its postlude
+                        // completes.
+                        write!(f, "local __{ret} = {ret}; {ret} = {ret}.__value; ")?;
+                        write!(f, "do {}end; ", T::postlude(ret))?;
+                        write!(f, "__C.{KEEP_FN}(__{ret}); ")?; // keep original result alive
+                    }
+                }
+            }
             write!(
                 f,
                 "else \
-                    local __{ret}_msg = __intern({ret}.__err.__ptr, {ret}.__err.__len); \
+                    local {ret}_err = __intern({ret}.__err.__ptr, {ret}.__err.__len); \
                     __C.{DROP_BUFFER_FN}({ret}.__err); \
-                    return error(__{ret}_msg); \
+                    return error({ret}_err); \
                  end; "
             )
         })

crates/luaffi/src/string.rs

@@ -154,8 +154,14 @@ unsafe impl IntoFfi for &'static [u8] {
         lua_buf::new(self)
     }
 
+    fn require_owned() -> bool {
+        // lua_buf is only used to have its contents interned then forgotten immediately; no need
+        // for ownership of it
+        false
+    }
+
     fn postlude(ret: &str) -> impl Display {
-        display!("{ret} = __intern({ret}.__ptr, {ret}.__len)")
+        display!("{ret} = __intern({ret}.__ptr, {ret}.__len); ")
     }
 }
 
@@ -166,9 +172,15 @@ unsafe impl IntoFfi for Vec<u8> {
         lua_buffer::new(self)
     }
 
+    fn require_owned() -> bool {
+        // lua_buffer is only used to have its contents interned then forgotten immediately; no need
+        // for ownership of it
+        false
+    }
+
     fn postlude(ret: &str) -> impl Display {
         display!(
-            "do local __{ret} = {ret}; {ret} = __intern({ret}.__ptr, {ret}.__len); __C.{DROP_BUFFER_FN}(__{ret}); end; "
+            "local {ret}_buf = {ret}; {ret} = __intern({ret}.__ptr, {ret}.__len); __C.{DROP_BUFFER_FN}({ret}_buf); "
         )
     }
 }