Merge #3142

3142: Do not store return values in locals so that holders benefit from lifetime extension for temporaries. r=davidhewitt a=adamreichold

Closes #3138

Co-authored-by: Adam Reichold <adam.reichold@t-online.de>

newsfragments/3142.fixed.md

@@ -0,0 +1 @@
+Extend lifetime of holder variables to avoid "temporary value dropped while borrowed" errors when `#[pyfunction]`s take references into `#[pyclass]`es

pyo3-macros-backend/src/method.rs

@@ -425,9 +425,8 @@ impl<'a> FnSpec<'a> {
 
         let rust_call = |args: Vec<TokenStream>| {
             quote! {
-                let mut ret = function(#self_arg #(#args),*);
-                let owned = _pyo3::impl_::pymethods::OkWrap::wrap(ret, #py);
-                owned.map(|obj| _pyo3::conversion::IntoPyPointer::into_ptr(obj))
+                _pyo3::impl_::pymethods::OkWrap::wrap(function(#self_arg #(#args),*), #py)
+                    .map(|obj| _pyo3::conversion::IntoPyPointer::into_ptr(obj))
                     .map_err(::core::convert::Into::into)
             }
         };

pyo3-macros-backend/src/pymethod.rs

@@ -456,9 +456,8 @@ fn impl_py_class_attribute(cls: &syn::Type, spec: &FnSpec<'_>) -> syn::Result<Me
         fn #wrapper_ident(py: _pyo3::Python<'_>) -> _pyo3::PyResult<_pyo3::PyObject> {
             let function = #cls::#name; // Shadow the method name to avoid #3017
             #deprecations
-            let mut ret = #fncall;
-            let owned = _pyo3::impl_::pymethods::OkWrap::wrap(ret, py);
-            owned.map_err(::core::convert::Into::into)
+            _pyo3::impl_::pymethods::OkWrap::wrap(#fncall, py)
+                .map_err(::core::convert::Into::into)
         }
     };
 

tests/test_pyfunction.rs

@@ -1,5 +1,7 @@
 #![cfg(feature = "macros")]
 
+use std::collections::HashMap;
+
 #[cfg(not(Py_LIMITED_API))]
 use pyo3::buffer::PyBuffer;
 use pyo3::prelude::*;
@@ -512,3 +514,34 @@ fn required_argument_after_option() {
         py_assert!(py, f, "f(x=None, y=5) == 5");
     })
 }
+
+#[pyclass]
+struct Key(String);
+
+#[pyclass]
+struct Value(i32);
+
+#[pyfunction]
+fn return_value_borrows_from_arguments<'py>(
+    py: Python<'py>,
+    key: &'py Key,
+    value: &'py Value,
+) -> HashMap<&'py str, i32> {
+    py.allow_threads(move || {
+        let mut map = HashMap::new();
+        map.insert(key.0.as_str(), value.0);
+        map
+    })
+}
+
+#[test]
+fn test_return_value_borrows_from_arguments() {
+    Python::with_gil(|py| {
+        let function = wrap_pyfunction!(return_value_borrows_from_arguments, py).unwrap();
+
+        let key = Py::new(py, Key("key".to_owned())).unwrap();
+        let value = Py::new(py, Value(42)).unwrap();
+
+        py_assert!(py, function key value, "function(key, value) == { \"key\": 42 }");
+    });
+}