From d1e360f7bf12eaba946edf124b29f855f1c9b9d3 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 21 Nov 2023 16:19:56 +0000 Subject: [PATCH 1/9] implement `PyTypeMethods` --- src/prelude.rs | 1 + src/types/mod.rs | 2 +- src/types/typeobject.rs | 127 +++++++++++++++++++++++++++++++--------- 3 files changed, 100 insertions(+), 30 deletions(-) diff --git a/src/prelude.rs b/src/prelude.rs index 6be820ae92a..d730f09ab4d 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -39,3 +39,4 @@ pub use crate::types::sequence::PySequenceMethods; pub use crate::types::set::PySetMethods; pub use crate::types::string::PyStringMethods; pub use crate::types::tuple::PyTupleMethods; +pub use crate::types::typeobject::PyTypeMethods; diff --git a/src/types/mod.rs b/src/types/mod.rs index 00fe81cf37e..d4379fcb27c 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -306,4 +306,4 @@ mod slice; pub(crate) mod string; mod traceback; pub(crate) mod tuple; -mod typeobject; +pub(crate) mod typeobject; diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index a1a053f93a4..2b1a9c015a8 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -1,5 +1,8 @@ use crate::err::{self, PyResult}; -use crate::{ffi, PyAny, PyTypeInfo, Python}; +use crate::ffi_ptr_ext::FfiPtrExt; +use crate::instance::Borrowed; +use crate::types::any::PyAnyMethods; +use crate::{ffi, Bound, PyAny, PyNativeType, PyTypeInfo, Python}; use std::borrow::Cow; #[cfg(not(any(Py_LIMITED_API, PyPy)))] use std::ffi::CStr; @@ -20,7 +23,7 @@ impl PyType { /// Retrieves the underlying FFI pointer associated with this Python object. #[inline] pub fn as_type_ptr(&self) -> *mut ffi::PyTypeObject { - self.as_ptr() as *mut ffi::PyTypeObject + self.as_borrowed().as_type_ptr() } /// Retrieves the `PyType` instance for the given FFI pointer. @@ -35,14 +38,81 @@ impl PyType { /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`. pub fn qualname(&self) -> PyResult { + self.as_borrowed().qualname() + } + + /// Gets the full name, which includes the module, of the `PyType`. + pub fn name(&self) -> PyResult> { + self.as_borrowed().name() + } + + /// Checks whether `self` is a subclass of `other`. + /// + /// Equivalent to the Python expression `issubclass(self, other)`. + pub fn is_subclass(&self, other: &PyAny) -> PyResult { + self.as_borrowed().is_subclass(&other.as_borrowed()) + } + + /// Checks whether `self` is a subclass of type `T`. + /// + /// Equivalent to the Python expression `issubclass(self, T)`, if the type + /// `T` is known at compile time. + pub fn is_subclass_of(&self) -> PyResult + where + T: PyTypeInfo, + { + self.as_borrowed().is_subclass_of::() + } +} + +/// Implementation of functionality for [`PyType`]. +/// +/// These methods are defined for the `Bound<'py, PyType>` smart pointer, so to use method call +/// syntax these methods are separated into a trait, because stable Rust does not yet support +/// `arbitrary_self_types`. +#[doc(alias = "PyType")] +pub trait PyTypeMethods<'py> { + /// Retrieves the underlying FFI pointer associated with this Python object. + fn as_type_ptr(&self) -> *mut ffi::PyTypeObject; + + /// Gets the full name, which includes the module, of the `PyType`. + fn name(&self) -> PyResult>; + + /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`. + fn qualname(&self) -> PyResult; + + /// Checks whether `self` is a subclass of `other`. + /// + /// Equivalent to the Python expression `issubclass(self, other)`. + fn is_subclass(&self, other: &Bound<'_, PyAny>) -> PyResult; + + /// Checks whether `self` is a subclass of type `T`. + /// + /// Equivalent to the Python expression `issubclass(self, T)`, if the type + /// `T` is known at compile time. + fn is_subclass_of(&self) -> PyResult + where + T: PyTypeInfo; +} + +impl<'py> PyTypeMethods<'py> for Bound<'py, PyType> { + /// Retrieves the underlying FFI pointer associated with this Python object. + #[inline] + fn as_type_ptr(&self) -> *mut ffi::PyTypeObject { + self.as_ptr() as *mut ffi::PyTypeObject + } + + /// Gets the name of the `PyType`. + fn name(&self) -> PyResult> { + Borrowed::from(self).name() + } + + fn qualname(&self) -> PyResult { #[cfg(any(Py_LIMITED_API, PyPy, not(Py_3_11)))] let name = self.getattr(intern!(self.py(), "__qualname__"))?.extract(); #[cfg(not(any(Py_LIMITED_API, PyPy, not(Py_3_11))))] let name = { - use crate::ffi_ptr_ext::FfiPtrExt; - use crate::types::any::PyAnyMethods; - let obj = unsafe { ffi::PyType_GetQualName(self.as_type_ptr()).assume_owned_or_err(self.py())? }; @@ -53,8 +123,29 @@ impl PyType { name } - /// Gets the full name, which includes the module, of the `PyType`. - pub fn name(&self) -> PyResult> { + /// Checks whether `self` is a subclass of `other`. + /// + /// Equivalent to the Python expression `issubclass(self, other)`. + fn is_subclass(&self, other: &Bound<'_, PyAny>) -> PyResult { + let result = unsafe { ffi::PyObject_IsSubclass(self.as_ptr(), other.as_ptr()) }; + err::error_on_minusone(self.py(), result)?; + Ok(result == 1) + } + + /// Checks whether `self` is a subclass of type `T`. + /// + /// Equivalent to the Python expression `issubclass(self, T)`, if the type + /// `T` is known at compile time. + fn is_subclass_of(&self) -> PyResult + where + T: PyTypeInfo, + { + self.is_subclass(&T::type_object(self.py()).as_borrowed()) + } +} + +impl<'a> Borrowed<'a, '_, PyType> { + fn name(self) -> PyResult> { #[cfg(not(any(Py_LIMITED_API, PyPy)))] { let ptr = self.as_type_ptr(); @@ -78,34 +169,12 @@ impl PyType { #[cfg(Py_3_11)] let name = { - use crate::ffi_ptr_ext::FfiPtrExt; - unsafe { ffi::PyType_GetName(self.as_type_ptr()).assume_owned_or_err(self.py())? } }; Ok(Cow::Owned(format!("{}.{}", module, name))) } } - - /// Checks whether `self` is a subclass of `other`. - /// - /// Equivalent to the Python expression `issubclass(self, other)`. - pub fn is_subclass(&self, other: &PyAny) -> PyResult { - let result = unsafe { ffi::PyObject_IsSubclass(self.as_ptr(), other.as_ptr()) }; - err::error_on_minusone(self.py(), result)?; - Ok(result == 1) - } - - /// Checks whether `self` is a subclass of type `T`. - /// - /// Equivalent to the Python expression `issubclass(self, T)`, if the type - /// `T` is known at compile time. - pub fn is_subclass_of(&self) -> PyResult - where - T: PyTypeInfo, - { - self.is_subclass(T::type_object(self.py())) - } } #[cfg(test)] From 0ae71bff7193ec5ee8483a0cd846fd54553b2937 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 28 Nov 2023 16:19:50 +0000 Subject: [PATCH 2/9] introduce `PyType` bound constructors --- Cargo.toml | 2 +- pyo3-macros-backend/Cargo.toml | 1 + pyo3-macros-backend/src/method.rs | 6 +++++ pyo3-macros/Cargo.toml | 1 + src/err/mod.rs | 6 ++--- src/instance.rs | 7 +++-- src/types/any.rs | 10 +++---- src/types/typeobject.rs | 44 ++++++++++++++++++++++++++++--- tests/test_class_basics.rs | 2 +- 9 files changed, 63 insertions(+), 16 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fd1daeb1451..e9a42a46e85 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -97,7 +97,7 @@ generate-import-lib = ["pyo3-ffi/generate-import-lib"] auto-initialize = [] # Allows use of the deprecated "GIL Refs" APIs. -gil-refs = [] +gil-refs = ["pyo3-macros/gil-refs"] # Optimizes PyObject to Vec conversion and so on. nightly = [] diff --git a/pyo3-macros-backend/Cargo.toml b/pyo3-macros-backend/Cargo.toml index 3263365dc80..cf2ec5dd774 100644 --- a/pyo3-macros-backend/Cargo.toml +++ b/pyo3-macros-backend/Cargo.toml @@ -25,6 +25,7 @@ features = ["derive", "parsing", "printing", "clone-impls", "full", "extra-trait [features] abi3 = [] +gil-refs = [] [lints] workspace = true diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 7050be23d5c..5c8e6bd4b14 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -125,8 +125,14 @@ impl FnType { FnType::FnClass(span) | FnType::FnNewClass(span) => { let py = syn::Ident::new("py", Span::call_site()); let slf: Ident = syn::Ident::new("_slf", Span::call_site()); + let allow_deprecated = if cfg!(feature = "gil-refs") { + quote!() + } else { + quote!(#[allow(deprecated)]) + }; quote_spanned! { *span => #[allow(clippy::useless_conversion)] + #allow_deprecated ::std::convert::Into::into(_pyo3::types::PyType::from_type_ptr(#py, #slf.cast())), } } diff --git a/pyo3-macros/Cargo.toml b/pyo3-macros/Cargo.toml index f34d483dd04..9fd1453cbdb 100644 --- a/pyo3-macros/Cargo.toml +++ b/pyo3-macros/Cargo.toml @@ -17,6 +17,7 @@ proc-macro = true multiple-pymethods = [] abi3 = ["pyo3-macros-backend/abi3"] +gil-refs = ["pyo3-macros-backend/gil-refs"] [dependencies] proc-macro2 = { version = "1", default-features = false } diff --git a/src/err/mod.rs b/src/err/mod.rs index 39975183f07..5e6cff0eff7 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -2,7 +2,7 @@ use crate::instance::Bound; use crate::panic::PanicException; use crate::type_object::PyTypeInfo; use crate::types::any::PyAnyMethods; -use crate::types::{PyTraceback, PyType}; +use crate::types::{typeobject::PyTypeMethods, PyTraceback, PyType}; use crate::{ exceptions::{self, PyBaseException}, ffi, @@ -202,7 +202,7 @@ impl PyErr { /// assert_eq!(err.to_string(), "TypeError: some type error"); /// /// // Case #2: Exception type - /// let err = PyErr::from_value(PyType::new::(py)); + /// let err = PyErr::from_value(PyType::new_bound::(py).as_gil_ref()); /// assert_eq!(err.to_string(), "TypeError: "); /// /// // Case #3: Invalid exception value @@ -233,7 +233,7 @@ impl PyErr { /// /// Python::with_gil(|py| { /// let err: PyErr = PyTypeError::new_err(("some type error",)); - /// assert!(err.get_type(py).is(PyType::new::(py))); + /// assert!(err.get_type(py).is(&PyType::new_bound::(py))); /// }); /// ``` pub fn get_type<'py>(&'py self, py: Python<'py>) -> &'py PyType { diff --git a/src/instance.rs b/src/instance.rs index 5779e9ada6f..602556f5c33 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -2,8 +2,7 @@ use crate::err::{self, PyDowncastError, PyErr, PyResult}; use crate::pycell::{PyBorrowError, PyBorrowMutError, PyCell}; use crate::pyclass::boolean_struct::{False, True}; use crate::type_object::HasPyGilRef; -use crate::types::any::PyAnyMethods; -use crate::types::string::PyStringMethods; +use crate::types::{any::PyAnyMethods, string::PyStringMethods, typeobject::PyTypeMethods}; use crate::types::{PyDict, PyString, PyTuple}; use crate::{ ffi, AsPyPointer, FromPyObject, IntoPy, PyAny, PyClass, PyClassInitializer, PyRef, PyRefMut, @@ -312,6 +311,10 @@ impl<'a, 'py> Borrowed<'a, 'py, PyAny> { pub(crate) unsafe fn from_ptr_unchecked(py: Python<'py>, ptr: *mut ffi::PyObject) -> Self { Self(NonNull::new_unchecked(ptr), PhantomData, py) } + + pub(crate) unsafe fn downcast_into_unchecked(self) -> Borrowed<'a, 'py, T> { + Borrowed(self.0, PhantomData, self.py()) + } } impl<'a, 'py, T> From<&'a Bound<'py, T>> for Borrowed<'a, 'py, T> { diff --git a/src/types/any.rs b/src/types/any.rs index 9cdf78d6c6b..d506a36f88f 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -712,7 +712,7 @@ impl PyAny { /// Returns the Python type object for this object's type. pub fn get_type(&self) -> &PyType { - self.as_borrowed().get_type() + self.as_borrowed().get_type().into_gil_ref() } /// Returns the Python type pointer for this object. @@ -1542,7 +1542,7 @@ pub trait PyAnyMethods<'py> { fn iter(&self) -> PyResult>; /// Returns the Python type object for this object's type. - fn get_type(&self) -> &'py PyType; + fn get_type(&self) -> Bound<'py, PyType>; /// Returns the Python type pointer for this object. fn get_type_ptr(&self) -> *mut ffi::PyTypeObject; @@ -2129,8 +2129,8 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { PyIterator::from_bound_object(self) } - fn get_type(&self) -> &'py PyType { - unsafe { PyType::from_type_ptr(self.py(), ffi::Py_TYPE(self.as_ptr())) } + fn get_type(&self) -> Bound<'py, PyType> { + unsafe { PyType::from_type_ptr_borrowed(self.py(), ffi::Py_TYPE(self.as_ptr())) }.to_owned() } #[inline] @@ -2287,7 +2287,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { #[cfg(not(PyPy))] fn py_super(&self) -> PyResult> { - PySuper::new_bound(&self.get_type().as_borrowed(), self) + PySuper::new_bound(&self.get_type(), self) } } diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 2b1a9c015a8..2bec6fdb56c 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -14,26 +14,62 @@ pub struct PyType(PyAny); pyobject_native_type_core!(PyType, pyobject_native_static_type_object!(ffi::PyType_Type), #checkfunction=ffi::PyType_Check); impl PyType { - /// Creates a new type object. + /// Deprecated form of [`PyType::new_bound`]. #[inline] + #[cfg_attr( + not(feature = "gil-refs"), + deprecated( + since = "0.21.0", + note = "`new` will be replaced by `new_bound` in a future PyO3 version" + ) + )] pub fn new(py: Python<'_>) -> &PyType { T::type_object(py) } + /// Creates a new type object. + #[inline] + pub fn new_bound(py: Python<'_>) -> Bound<'_, PyType> { + T::type_object(py).as_borrowed().to_owned() + } + /// Retrieves the underlying FFI pointer associated with this Python object. #[inline] pub fn as_type_ptr(&self) -> *mut ffi::PyTypeObject { self.as_borrowed().as_type_ptr() } + /// Deprecated form of [`PyType::from_type_ptr_borrowed`]. + /// + /// # Safety + /// + /// See [`PyType::from_type_ptr_borrowed`]. + #[inline] + #[cfg_attr( + not(feature = "gil-refs"), + deprecated( + since = "0.21.0", + note = "`from_type_ptr` will be replaced by `from_type_ptr_borrowed` in a future PyO3 version" + ) + )] + pub unsafe fn from_type_ptr(py: Python<'_>, p: *mut ffi::PyTypeObject) -> &PyType { + Self::from_type_ptr_borrowed(py, p).into_gil_ref() + } + /// Retrieves the `PyType` instance for the given FFI pointer. /// /// # Safety /// - The pointer must be non-null. - /// - The pointer must be valid for the entire of the lifetime for which the reference is used. + /// - The pointer must be valid for the entire of the lifetime 'a for which the reference is used, + /// as with `std::slice::from_raw_parts`. #[inline] - pub unsafe fn from_type_ptr(py: Python<'_>, p: *mut ffi::PyTypeObject) -> &PyType { - py.from_borrowed_ptr(p as *mut ffi::PyObject) + pub unsafe fn from_type_ptr_borrowed<'a>( + py: Python<'_>, + p: *mut ffi::PyTypeObject, + ) -> Borrowed<'a, '_, PyType> { + (p as *mut ffi::PyObject) + .assume_borrowed_unchecked(py) + .downcast_into_unchecked() } /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`. diff --git a/tests/test_class_basics.rs b/tests/test_class_basics.rs index bbf37a2d66b..df109442335 100644 --- a/tests/test_class_basics.rs +++ b/tests/test_class_basics.rs @@ -230,7 +230,7 @@ impl UnsendableChild { fn test_unsendable() -> PyResult<()> { let obj = Python::with_gil(|py| -> PyResult<_> { - let obj: Py = PyType::new::(py).call1((5,))?.extract()?; + let obj: Py = PyType::new_bound::(py).call1((5,))?.extract()?; // Accessing the value inside this thread should not panic let caught_panic = From 09e318052470fc896e3a0f79391a7094d1cea5b5 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 16 Feb 2024 08:06:22 +0000 Subject: [PATCH 3/9] `from_type_ptr_bound` instead of `from_type_ptr_borrowed` --- src/types/any.rs | 2 +- src/types/typeobject.rs | 23 +++++++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/types/any.rs b/src/types/any.rs index eac9c2d829b..8625ff62f9e 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -2108,7 +2108,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { } fn get_type(&self) -> Bound<'py, PyType> { - unsafe { PyType::from_type_ptr_borrowed(self.py(), ffi::Py_TYPE(self.as_ptr())) }.to_owned() + unsafe { PyType::from_type_ptr_bound(self.py(), &ffi::Py_TYPE(self.as_ptr())) }.to_owned() } #[inline] diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 5706739d4ce..3ed85375571 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -39,37 +39,36 @@ impl PyType { self.as_borrowed().as_type_ptr() } - /// Deprecated form of [`PyType::from_type_ptr_borrowed`]. + /// Deprecated form of [`PyType::from_type_ptr_bound`]. /// /// # Safety /// - /// See [`PyType::from_type_ptr_borrowed`]. + /// - The pointer must a valid non-null reference to a `PyTypeObject`. #[inline] #[cfg_attr( not(feature = "gil-refs"), deprecated( since = "0.21.0", - note = "`from_type_ptr` will be replaced by `from_type_ptr_borrowed` in a future PyO3 version" + note = "`from_type_ptr` will be replaced by `from_type_ptr_bound` in a future PyO3 version" ) )] pub unsafe fn from_type_ptr(py: Python<'_>, p: *mut ffi::PyTypeObject) -> &PyType { - Self::from_type_ptr_borrowed(py, p).into_gil_ref() + Self::from_type_ptr_bound(py, &p).to_owned().into_gil_ref() } - /// Retrieves the `PyType` instance for the given FFI pointer. + /// Converts the given FFI pointer into `&Bound`, to use in safe code. /// /// # Safety /// - The pointer must be non-null. /// - The pointer must be valid for the entire of the lifetime 'a for which the reference is used, /// as with `std::slice::from_raw_parts`. #[inline] - pub unsafe fn from_type_ptr_borrowed<'a>( - py: Python<'_>, - p: *mut ffi::PyTypeObject, - ) -> Borrowed<'a, '_, PyType> { - (p as *mut ffi::PyObject) - .assume_borrowed_unchecked(py) - .downcast_unchecked() + pub unsafe fn from_type_ptr_bound<'a, 'py>( + py: Python<'py>, + p: &'a *mut ffi::PyTypeObject, + ) -> &'a Bound<'py, PyType> { + let object_ptr = &*(p as *const *mut ffi::PyTypeObject as *const *mut ffi::PyObject); + Bound::ref_from_ptr(py, object_ptr).downcast_unchecked() } /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`. From f8c68108d5b45cbe0baaaddbabee19c5564a9fe7 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 16 Feb 2024 23:29:50 +0000 Subject: [PATCH 4/9] correct conditional code --- src/err/err_state.rs | 3 +-- src/types/typeobject.rs | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/err/err_state.rs b/src/err/err_state.rs index 863b276307e..fd965014fb1 100644 --- a/src/err/err_state.rs +++ b/src/err/err_state.rs @@ -22,9 +22,8 @@ impl PyErrStateNormalized { #[cfg(Py_3_12)] pub(crate) fn ptype<'py>(&self, py: Python<'py>) -> Bound<'py, PyType> { - use crate::instance::PyNativeType; use crate::types::any::PyAnyMethods; - self.pvalue.bind(py).get_type().as_borrowed().to_owned() + self.pvalue.bind(py).get_type() } #[cfg(not(Py_3_12))] diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 3ed85375571..2f8ded2fa40 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -1,5 +1,4 @@ use crate::err::{self, PyResult}; -use crate::ffi_ptr_ext::FfiPtrExt; use crate::instance::Borrowed; use crate::types::any::PyAnyMethods; use crate::{ffi, Bound, PyAny, PyNativeType, PyTypeInfo, Python}; @@ -148,6 +147,7 @@ impl<'py> PyTypeMethods<'py> for Bound<'py, PyType> { #[cfg(not(any(Py_LIMITED_API, PyPy, not(Py_3_11))))] let name = { + use crate::ffi_ptr_ext::FfiPtrExt; let obj = unsafe { ffi::PyType_GetQualName(self.as_type_ptr()).assume_owned_or_err(self.py())? }; @@ -204,6 +204,7 @@ impl<'a> Borrowed<'a, '_, PyType> { #[cfg(Py_3_11)] let name = { + use crate::ffi_ptr_ext::FfiPtrExt; unsafe { ffi::PyType_GetName(self.as_type_ptr()).assume_owned_or_err(self.py())? } }; From 5e908806e6252b56b0e7b0f58ef044fde06eeef1 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Sat, 17 Feb 2024 22:48:14 +0000 Subject: [PATCH 5/9] just make `from_type_ptr_bound` create an owned `Bound` --- src/types/any.rs | 2 +- src/types/typeobject.rs | 23 +++++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/types/any.rs b/src/types/any.rs index 8625ff62f9e..cd7338ee209 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -2108,7 +2108,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { } fn get_type(&self) -> Bound<'py, PyType> { - unsafe { PyType::from_type_ptr_bound(self.py(), &ffi::Py_TYPE(self.as_ptr())) }.to_owned() + unsafe { PyType::from_type_ptr_bound(self.py(), ffi::Py_TYPE(self.as_ptr())) } } #[inline] diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 2f8ded2fa40..2190726887f 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -19,7 +19,7 @@ impl PyType { not(feature = "gil-refs"), deprecated( since = "0.21.0", - note = "`new` will be replaced by `new_bound` in a future PyO3 version" + note = "`PyType::new` will be replaced by `PyType::new_bound` in a future PyO3 version" ) )] pub fn new(py: Python<'_>) -> &PyType { @@ -52,22 +52,25 @@ impl PyType { ) )] pub unsafe fn from_type_ptr(py: Python<'_>, p: *mut ffi::PyTypeObject) -> &PyType { - Self::from_type_ptr_bound(py, &p).to_owned().into_gil_ref() + Self::from_type_ptr_bound(py, p).into_gil_ref() } /// Converts the given FFI pointer into `&Bound`, to use in safe code. /// + /// This does not take ownership of the FFI pointer's "reference". The target type + /// object will have its reference count increased by 1, which will be released when + /// the `Bound` is dropped. + /// /// # Safety - /// - The pointer must be non-null. - /// - The pointer must be valid for the entire of the lifetime 'a for which the reference is used, - /// as with `std::slice::from_raw_parts`. + /// - The pointer must be a valid non-null reference to a `PyTypeObject` #[inline] - pub unsafe fn from_type_ptr_bound<'a, 'py>( + pub unsafe fn from_type_ptr_bound<'py>( py: Python<'py>, - p: &'a *mut ffi::PyTypeObject, - ) -> &'a Bound<'py, PyType> { - let object_ptr = &*(p as *const *mut ffi::PyTypeObject as *const *mut ffi::PyObject); - Bound::ref_from_ptr(py, object_ptr).downcast_unchecked() + p: *mut ffi::PyTypeObject, + ) -> Bound<'py, PyType> { + Borrowed::from_ptr_unchecked(py, p.cast()) + .downcast_unchecked() + .to_owned() } /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`. From 4a6b26be67638e31ae4e35bdd5ca125ef8d64c8c Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Sat, 17 Feb 2024 23:16:24 +0000 Subject: [PATCH 6/9] correct docstrings Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com> --- src/types/typeobject.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 2190726887f..4f85fd729e9 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -48,14 +48,14 @@ impl PyType { not(feature = "gil-refs"), deprecated( since = "0.21.0", - note = "`from_type_ptr` will be replaced by `from_type_ptr_bound` in a future PyO3 version" + note = "`PyType::from_type_ptr` will be replaced by `PyType::from_type_ptr_bound` in a future PyO3 version" ) )] pub unsafe fn from_type_ptr(py: Python<'_>, p: *mut ffi::PyTypeObject) -> &PyType { Self::from_type_ptr_bound(py, p).into_gil_ref() } - /// Converts the given FFI pointer into `&Bound`, to use in safe code. + /// Converts the given FFI pointer into `Bound`, to use in safe code. /// /// This does not take ownership of the FFI pointer's "reference". The target type /// object will have its reference count increased by 1, which will be released when From 2a8b9d069e6a64051d4c5411f15f66e7bca9b7fa Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Sat, 17 Feb 2024 23:19:39 +0000 Subject: [PATCH 7/9] Rework as `PyType::from_borrowed_type_ptr` --- src/types/any.rs | 2 +- src/types/typeobject.rs | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/types/any.rs b/src/types/any.rs index cd7338ee209..1cb6bb779f1 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -2108,7 +2108,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { } fn get_type(&self) -> Bound<'py, PyType> { - unsafe { PyType::from_type_ptr_bound(self.py(), ffi::Py_TYPE(self.as_ptr())) } + unsafe { PyType::from_borrowed_type_ptr(self.py(), ffi::Py_TYPE(self.as_ptr())) } } #[inline] diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 4f85fd729e9..bef4d62325d 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -48,23 +48,22 @@ impl PyType { not(feature = "gil-refs"), deprecated( since = "0.21.0", - note = "`PyType::from_type_ptr` will be replaced by `PyType::from_type_ptr_bound` in a future PyO3 version" + note = "Use `PyType::from_borrowed_type_ptr` instead" ) )] pub unsafe fn from_type_ptr(py: Python<'_>, p: *mut ffi::PyTypeObject) -> &PyType { - Self::from_type_ptr_bound(py, p).into_gil_ref() + Self::from_borrowed_type_ptr(py, p).into_gil_ref() } /// Converts the given FFI pointer into `Bound`, to use in safe code. /// - /// This does not take ownership of the FFI pointer's "reference". The target type - /// object will have its reference count increased by 1, which will be released when - /// the `Bound` is dropped. + /// The function creates a new reference from the given pointer, and returns + /// it as a `Bound`. /// /// # Safety /// - The pointer must be a valid non-null reference to a `PyTypeObject` #[inline] - pub unsafe fn from_type_ptr_bound<'py>( + pub unsafe fn from_borrowed_type_ptr<'py>( py: Python<'py>, p: *mut ffi::PyTypeObject, ) -> Bound<'py, PyType> { From 7c1f2595a1256941ed50de53c3529adec84913b4 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Sat, 17 Feb 2024 23:55:39 +0000 Subject: [PATCH 8/9] correct doc link to `from_borrowed_type_ptr` Co-authored-by: Lily Foote --- src/types/typeobject.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index bef4d62325d..1b7f51fb3e7 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -38,7 +38,7 @@ impl PyType { self.as_borrowed().as_type_ptr() } - /// Deprecated form of [`PyType::from_type_ptr_bound`]. + /// Deprecated form of [`PyType::from_borrowed_type_ptr`]. /// /// # Safety /// From 276d4dfd09a89140fc74863583e69913cef62fac Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Sat, 17 Feb 2024 23:56:42 +0000 Subject: [PATCH 9/9] remove unneeded lifetime name --- src/types/typeobject.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 1b7f51fb3e7..f0e4a8c0a2c 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -63,10 +63,10 @@ impl PyType { /// # Safety /// - The pointer must be a valid non-null reference to a `PyTypeObject` #[inline] - pub unsafe fn from_borrowed_type_ptr<'py>( - py: Python<'py>, + pub unsafe fn from_borrowed_type_ptr( + py: Python<'_>, p: *mut ffi::PyTypeObject, - ) -> Bound<'py, PyType> { + ) -> Bound<'_, PyType> { Borrowed::from_ptr_unchecked(py, p.cast()) .downcast_unchecked() .to_owned()