review: LilyFoote, Icxolu feedback

PyO3 · Feb 28, 2024 · 101563c · 101563c
1 parent 108f1e7
commit 101563c
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 4 deletions.
diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs
@@ -196,7 +196,7 @@ impl SelfType {
                 holders.push(quote_spanned! { *span =>
                     #[allow(clippy::let_unit_value)]
                     let mut #holder = _pyo3::impl_::extract_argument::FunctionArgumentHolder::INIT;
-                    let mut #slf = _pyo3::Borrowed::from_ptr(#py, #slf);
+                    let mut #slf = _pyo3::impl_::pymethods::BoundRef::ref_from_ptr(#py, &#slf);
                 });
                 error_mode.handle_error(quote_spanned! { *span =>
                     _pyo3::impl_::extract_argument::#method::<#cls>(

diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs
@@ -44,8 +44,8 @@ pub fn impl_arg_params(
             .collect::<Result<_>>()?;
         return Ok((
             quote! {
-                let _args = _pyo3::Borrowed::from_ptr(py, _args);
-                let _kwargs = _pyo3::Borrowed::from_ptr_or_opt(py, _kwargs);
+                let _args = _pyo3::impl_::pymethods::BoundRef::ref_from_ptr(py, &_args);
+                let _kwargs = _pyo3::impl_::pymethods::BoundRef::ref_from_ptr_or_opt(py, &_kwargs);
             },
             arg_convert,
         ));

diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs
@@ -990,7 +990,7 @@ fn extract_object(
     });
     extract_error_mode.handle_error(quote! {
         _pyo3::impl_::extract_argument::extract_argument(
-            &_pyo3::Borrowed::from_ptr(py, #source_ptr),
+            &_pyo3::impl_::pymethods::BoundRef::ref_from_ptr(py, &#source_ptr),
             &mut #holder,
             #name
         )

diff --git a/src/impl_/pymethods.rs b/src/impl_/pymethods.rs
@@ -561,3 +561,11 @@ impl<T> From<BoundRef<'_, '_, T>> for Py<T> {
         bound.0.clone().unbind()
     }
 }
+
+impl<'py, T> std::ops::Deref for BoundRef<'_, 'py, T> {
+    type Target = Bound<'py, T>;
+    #[inline]
+    fn deref(&self) -> &Self::Target {
+        self.0
+    }
+}
diff --git a/src/types/dict.rs b/src/types/dict.rs
@@ -692,11 +692,15 @@ mod borrowed_iter {
             let mut key: *mut ffi::PyObject = std::ptr::null_mut();
             let mut value: *mut ffi::PyObject = std::ptr::null_mut();
 
+            // Safety: self.dict lives sufficiently long that the pointer is not dangling
             if unsafe { ffi::PyDict_Next(self.dict.as_ptr(), &mut self.ppos, &mut key, &mut value) }
                 != 0
             {
                 let py = self.dict.py();
                 self.len -= 1;
+                // Safety:
+                // - PyDict_Next returns borrowed values
+                // - we have already checked that `PyDict_Next` succeeded, so we can assume these to be non-null
                 Some(unsafe { (key.assume_borrowed(py), value.assume_borrowed(py)) })
             } else {
                 None