diff --git a/guide/src/class.md b/guide/src/class.md index ed71cb2e344..e3f3502780b 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -1108,8 +1108,10 @@ struct MyClass { # #[allow(dead_code)] num: i32, } -unsafe impl pyo3::type_object::PyTypeInfo for MyClass { +unsafe impl pyo3::type_object::HasPyGilRef for MyClass { type AsRefTarget = pyo3::PyCell; +} +unsafe impl pyo3::type_object::PyTypeInfo for MyClass { const NAME: &'static str = "MyClass"; const MODULE: ::std::option::Option<&'static str> = ::std::option::Option::None; #[inline] diff --git a/guide/src/migration.md b/guide/src/migration.md index 75fe7453aad..4538f799877 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -5,6 +5,42 @@ For a detailed list of all changes, see the [CHANGELOG](changelog.md). ## from 0.20.* to 0.21 +### `PyTypeInfo` and `PyTryFrom` have been adjusted + +The `PyTryFrom` trait has aged poorly, its [`try_from`] method now conflicts with `try_from` in the 2021 edition prelude. A lot of its functionality was also duplicated with `PyTypeInfo`. + +To tighten up the PyO3 traits ahead of [a proposed upcoming API change](https://github.com/PyO3/pyo3/issues/3382) the `PyTypeInfo` trait has had a simpler companion `PyTypeCheck`. The methods [`PyAny::downcast`]({{#PYO3_DOCS_URL}}/pyo3/types/struct.PyAny.html#method.downcast) and [`PyAny::downcast_exact`]({{#PYO3_DOCS_URL}}/pyo3/types/struct.PyAny.html#method.downcast_exact) no longer use `PyTryFrom` as a bound, instead using `PyTypeCheck` and `PyTypeInfo` respectively. + +To migrate, switch all type casts to use `obj.downcast()` instead of `try_from(obj)` (and similar for `downcast_exact`). + +Before: + +```rust +# use pyo3::prelude::*; +# use pyo3::types::{PyInt, PyList}; +# fn main() -> PyResult<()> { +Python::with_gil(|py| { + let list = PyList::new(py, 0..5); + let b = ::try_from(list.get_item(0).unwrap())?; + Ok(()) +}) +# } +``` + +After: + +```rust +# use pyo3::prelude::*; +# use pyo3::types::{PyInt, PyList}; +# fn main() -> PyResult<()> { +Python::with_gil(|py| { + let list = PyList::new(py, 0..5); + let b = list.get_item(0).unwrap().downcast::()?; + Ok(()) +}) +# } +``` + ### `py.None()`, `py.NotImplemented()` and `py.Ellipsis()` now return typed singletons Previously `py.None()`, `py.NotImplemented()` and `py.Ellipsis()` would return `PyObject`. This had a few downsides: diff --git a/newsfragments/3600.changed.md b/newsfragments/3600.changed.md new file mode 100644 index 00000000000..c8701ef4b25 --- /dev/null +++ b/newsfragments/3600.changed.md @@ -0,0 +1 @@ +Split some `PyTypeInfo` functionality into new traits `HasPyGilRef` and `PyTypeCheck`. diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index f037335e23a..239514ae38a 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -741,9 +741,11 @@ fn impl_pytypeinfo( }; quote! { - unsafe impl _pyo3::type_object::PyTypeInfo for #cls { + unsafe impl _pyo3::type_object::HasPyGilRef for #cls { type AsRefTarget = _pyo3::PyCell; + } + unsafe impl _pyo3::type_object::PyTypeInfo for #cls { const NAME: &'static str = #cls_name; const MODULE: ::std::option::Option<&'static str> = #module; diff --git a/src/conversion.rs b/src/conversion.rs index 4ae166ebd0d..665cdeca781 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -304,7 +304,7 @@ where T: PyClass, { fn extract(obj: &'a PyAny) -> PyResult { - PyTryFrom::try_from(obj).map_err(Into::into) + obj.downcast().map_err(Into::into) } } @@ -313,7 +313,7 @@ where T: PyClass + Clone, { fn extract(obj: &'a PyAny) -> PyResult { - let cell: &PyCell = PyTryFrom::try_from(obj)?; + let cell: &PyCell = obj.downcast()?; Ok(unsafe { cell.try_borrow_unguarded()?.clone() }) } } @@ -323,7 +323,7 @@ where T: PyClass, { fn extract(obj: &'a PyAny) -> PyResult { - let cell: &PyCell = PyTryFrom::try_from(obj)?; + let cell: &PyCell = obj.downcast()?; cell.try_borrow().map_err(Into::into) } } @@ -333,7 +333,7 @@ where T: PyClass, { fn extract(obj: &'a PyAny) -> PyResult { - let cell: &PyCell = PyTryFrom::try_from(obj)?; + let cell: &PyCell = obj.downcast()?; cell.try_borrow_mut().map_err(Into::into) } } @@ -381,78 +381,61 @@ pub trait PyTryInto: Sized { fn try_into_exact(&self) -> Result<&T, PyDowncastError<'_>>; } -// TryFrom implies TryInto -impl PyTryInto for PyAny -where - U: for<'v> PyTryFrom<'v>, -{ - fn try_into(&self) -> Result<&U, PyDowncastError<'_>> { - >::try_from(self) - } - fn try_into_exact(&self) -> Result<&U, PyDowncastError<'_>> { - U::try_from_exact(self) - } -} +mod implementations { + use super::*; -impl<'v, T> PyTryFrom<'v> for T -where - T: PyTypeInfo + PyNativeType, -{ - fn try_from>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { - let value = value.into(); - unsafe { - if T::is_type_of(value) { - Ok(Self::try_from_unchecked(value)) - } else { - Err(PyDowncastError::new(value, T::NAME)) - } + // TryFrom implies TryInto + impl PyTryInto for PyAny + where + U: for<'v> PyTryFrom<'v>, + { + fn try_into(&self) -> Result<&U, PyDowncastError<'_>> { + >::try_from(self) + } + fn try_into_exact(&self) -> Result<&U, PyDowncastError<'_>> { + U::try_from_exact(self) } } - fn try_from_exact>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { - let value = value.into(); - unsafe { - if T::is_exact_type_of(value) { - Ok(Self::try_from_unchecked(value)) - } else { - Err(PyDowncastError::new(value, T::NAME)) - } + impl<'v, T> PyTryFrom<'v> for T + where + T: PyTypeInfo + PyNativeType, + { + fn try_from>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { + value.into().downcast() } - } - #[inline] - unsafe fn try_from_unchecked>(value: V) -> &'v Self { - Self::unchecked_downcast(value.into()) - } -} + fn try_from_exact>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { + value.into().downcast_exact() + } -impl<'v, T> PyTryFrom<'v> for PyCell -where - T: 'v + PyClass, -{ - fn try_from>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { - let value = value.into(); - unsafe { - if T::is_type_of(value) { - Ok(Self::try_from_unchecked(value)) - } else { - Err(PyDowncastError::new(value, T::NAME)) - } + #[inline] + unsafe fn try_from_unchecked>(value: V) -> &'v Self { + value.into().downcast_unchecked() } } - fn try_from_exact>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { - let value = value.into(); - unsafe { - if T::is_exact_type_of(value) { - Ok(Self::try_from_unchecked(value)) - } else { - Err(PyDowncastError::new(value, T::NAME)) + + impl<'v, T> PyTryFrom<'v> for PyCell + where + T: 'v + PyClass, + { + fn try_from>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { + value.into().downcast() + } + fn try_from_exact>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { + let value = value.into(); + unsafe { + if T::is_exact_type_of(value) { + Ok(Self::try_from_unchecked(value)) + } else { + Err(PyDowncastError::new(value, T::NAME)) + } } } - } - #[inline] - unsafe fn try_from_unchecked>(value: V) -> &'v Self { - Self::unchecked_downcast(value.into()) + #[inline] + unsafe fn try_from_unchecked>(value: V) -> &'v Self { + value.into().downcast_unchecked() + } } } @@ -572,10 +555,11 @@ mod test_no_clone {} #[cfg(test)] mod tests { - use crate::types::{IntoPyDict, PyAny, PyDict, PyList}; - use crate::{PyObject, Python, ToPyObject}; + use crate::PyObject; - use super::PyTryFrom; + use super::super::PyTryFrom; + use crate::types::{IntoPyDict, PyAny, PyDict, PyList}; + use crate::{Python, ToPyObject}; #[test] fn test_try_from() { diff --git a/src/instance.rs b/src/instance.rs index 4c43ef33cf1..d80f6d519e1 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -1,14 +1,14 @@ -use crate::conversion::PyTryFrom; use crate::err::{self, PyDowncastError, PyErr, PyResult}; -use crate::gil; 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::{PyDict, PyString, PyTuple}; use crate::{ ffi, AsPyPointer, FromPyObject, IntoPy, PyAny, PyClass, PyClassInitializer, PyRef, PyRefMut, PyTypeInfo, Python, ToPyObject, }; +use crate::{gil, PyTypeCheck}; use std::marker::PhantomData; use std::mem::{self, ManuallyDrop}; use std::ops::Deref; @@ -185,7 +185,7 @@ impl<'py, T> Py2<'py, T> { #[doc(hidden)] // public and doc(hidden) to use in examples and tests for now pub fn as_gil_ref(&'py self) -> &'py T::AsRefTarget where - T: PyTypeInfo, + T: HasPyGilRef, { unsafe { self.py().from_borrowed_ptr(self.as_ptr()) } } @@ -194,7 +194,7 @@ impl<'py, T> Py2<'py, T> { #[doc(hidden)] // public but hidden, to use for tests for now pub fn into_gil_ref(self) -> &'py T::AsRefTarget where - T: PyTypeInfo, + T: HasPyGilRef, { unsafe { self.py().from_owned_ptr(self.into_ptr()) } } @@ -437,7 +437,7 @@ where impl Py where - T: PyTypeInfo, + T: HasPyGilRef, { /// Borrows a GIL-bound reference to the contained `T`. /// @@ -1314,11 +1314,11 @@ impl PyObject { /// # } /// ``` #[inline] - pub fn downcast<'p, T>(&'p self, py: Python<'p>) -> Result<&T, PyDowncastError<'_>> + pub fn downcast<'py, T>(&'py self, py: Python<'py>) -> Result<&'py T, PyDowncastError<'py>> where - T: PyTryFrom<'p>, + T: PyTypeCheck, { - >::try_from(self.as_ref(py)) + self.as_ref(py).downcast() } /// Casts the PyObject to a concrete Python object type without checking validity. @@ -1329,9 +1329,9 @@ impl PyObject { #[inline] pub unsafe fn downcast_unchecked<'p, T>(&'p self, py: Python<'p>) -> &T where - T: PyTryFrom<'p>, + T: HasPyGilRef, { - >::try_from_unchecked(self.as_ref(py)) + self.as_ref(py).downcast_unchecked() } } @@ -1509,6 +1509,8 @@ a = A() #[cfg(feature = "macros")] mod using_macros { + use crate::{PyCell, PyTryInto}; + use super::*; #[crate::pyclass] @@ -1533,5 +1535,15 @@ a = A() assert_eq!(instance.try_borrow_mut(py).unwrap().0, 123); }) } + + #[test] + fn cell_tryfrom() { + // More detailed tests of the underlying semantics in pycell.rs + Python::with_gil(|py| { + let instance: &PyAny = Py::new(py, SomeClass(0)).unwrap().into_ref(py); + let _: &PyCell = PyTryInto::try_into(instance).unwrap(); + let _: &PyCell = PyTryInto::try_into_exact(instance).unwrap(); + }) + } } } diff --git a/src/lib.rs b/src/lib.rs index 762644f1a2c..fcf75610656 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -294,9 +294,8 @@ //! [Features chapter of the guide]: https://pyo3.rs/latest/features.html#features-reference "Features Reference - PyO3 user guide" //! [`Ungil`]: crate::marker::Ungil pub use crate::class::*; -pub use crate::conversion::{ - AsPyPointer, FromPyObject, FromPyPointer, IntoPy, PyTryFrom, PyTryInto, ToPyObject, -}; +pub use crate::conversion::{AsPyPointer, FromPyObject, FromPyPointer, IntoPy, ToPyObject}; +pub use crate::conversion::{PyTryFrom, PyTryInto}; pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyResult}; pub use crate::gil::GILPool; #[cfg(not(PyPy))] @@ -306,7 +305,7 @@ pub use crate::marker::Python; pub use crate::pycell::{PyCell, PyRef, PyRefMut}; pub use crate::pyclass::PyClass; pub use crate::pyclass_init::PyClassInitializer; -pub use crate::type_object::PyTypeInfo; +pub use crate::type_object::{PyTypeCheck, PyTypeInfo}; pub use crate::types::PyAny; pub use crate::version::PythonVersionInfo; diff --git a/src/marker.rs b/src/marker.rs index a789e565a84..a6517fa1844 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -118,11 +118,12 @@ use crate::err::{self, PyDowncastError, PyErr, PyResult}; use crate::gil::{GILGuard, GILPool, SuspendGIL}; use crate::impl_::not_send::NotSend; +use crate::type_object::HasPyGilRef; use crate::types::{ PyAny, PyDict, PyEllipsis, PyModule, PyNone, PyNotImplemented, PyString, PyType, }; use crate::version::PythonVersionInfo; -use crate::{ffi, FromPyPointer, IntoPy, Py, PyNativeType, PyObject, PyTryFrom, PyTypeInfo}; +use crate::{ffi, FromPyPointer, IntoPy, Py, PyObject, PyTypeCheck, PyTypeInfo}; use std::ffi::{CStr, CString}; use std::marker::PhantomData; use std::os::raw::c_int; @@ -759,10 +760,9 @@ impl<'py> Python<'py> { /// Registers the object in the release pool, and tries to downcast to specific type. pub fn checked_cast_as(self, obj: PyObject) -> Result<&'py T, PyDowncastError<'py>> where - T: PyTryFrom<'py>, + T: PyTypeCheck, { - let any: &PyAny = unsafe { self.from_owned_ptr(obj.into_ptr()) }; - ::try_from(any) + obj.into_ref(self).downcast() } /// Registers the object in the release pool, and does an unchecked downcast @@ -773,10 +773,9 @@ impl<'py> Python<'py> { /// Callers must ensure that ensure that the cast is valid. pub unsafe fn cast_as(self, obj: PyObject) -> &'py T where - T: PyNativeType + PyTypeInfo, + T: HasPyGilRef, { - let any: &PyAny = self.from_owned_ptr(obj.into_ptr()); - T::unchecked_downcast(any) + obj.into_ref(self).downcast_unchecked() } /// Registers the object pointer in the release pool, diff --git a/src/prelude.rs b/src/prelude.rs index fc0199e59e3..1ef60bc659a 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -8,7 +8,8 @@ //! use pyo3::prelude::*; //! ``` -pub use crate::conversion::{FromPyObject, IntoPy, PyTryFrom, PyTryInto, ToPyObject}; +pub use crate::conversion::{FromPyObject, IntoPy, ToPyObject}; +pub use crate::conversion::{PyTryFrom, PyTryInto}; pub use crate::err::{PyErr, PyResult}; pub use crate::instance::{Py, PyObject}; pub use crate::marker::Python; diff --git a/src/pycell.rs b/src/pycell.rs index 8a4ceb6b374..2d5c0536069 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -207,7 +207,7 @@ use crate::{ type_object::get_tp_free, PyTypeInfo, }; -use crate::{ffi, IntoPy, PyErr, PyNativeType, PyObject, PyResult, Python}; +use crate::{ffi, IntoPy, PyErr, PyNativeType, PyObject, PyResult, PyTypeCheck, Python}; use std::cell::UnsafeCell; use std::fmt; use std::mem::ManuallyDrop; @@ -527,6 +527,17 @@ impl PyCell { unsafe impl PyLayout for PyCell {} impl PySizedLayout for PyCell {} +impl PyTypeCheck for PyCell +where + T: PyClass, +{ + const NAME: &'static str = ::NAME; + + fn type_check(object: &PyAny) -> bool { + ::type_check(object) + } +} + unsafe impl AsPyPointer for PyCell { fn as_ptr(&self) -> *mut ffi::PyObject { (self as *const _) as *mut _ diff --git a/src/type_object.rs b/src/type_object.rs index f867dcca001..428de86accf 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -19,6 +19,27 @@ pub unsafe trait PyLayout {} /// In addition, that `T` is a concrete representation of `U`. pub trait PySizedLayout: PyLayout + Sized {} +/// Specifies that this type has a "GIL-bound Reference" form. +/// +/// This is expected to be deprecated in the near future, see +/// +/// +/// # Safety +/// +/// - `Py::as_ref` will hand out references to `Self::AsRefTarget`. +/// - `Self::AsRefTarget` must have the same layout as `UnsafeCell`. +pub unsafe trait HasPyGilRef { + /// Utility type to make Py::as_ref work. + type AsRefTarget: PyNativeType; +} + +unsafe impl HasPyGilRef for T +where + T: PyNativeType, +{ + type AsRefTarget = Self; +} + /// Python type information. /// All Python native types (e.g., `PyDict`) and `#[pyclass]` structs implement this trait. /// @@ -32,16 +53,13 @@ pub trait PySizedLayout: PyLayout + Sized {} /// /// Implementations must provide an implementation for `type_object_raw` which infallibly produces a /// non-null pointer to the corresponding Python type object. -pub unsafe trait PyTypeInfo: Sized { +pub unsafe trait PyTypeInfo: Sized + HasPyGilRef { /// Class name. const NAME: &'static str; /// Module name, if any. const MODULE: Option<&'static str>; - /// Utility type to make Py::as_ref work. - type AsRefTarget: PyNativeType; - /// Returns the PyTypeObject instance for this type. fn type_object_raw(py: Python<'_>) -> *mut ffi::PyTypeObject; @@ -64,6 +82,29 @@ pub unsafe trait PyTypeInfo: Sized { } } +/// Implemented by types which can be used as a concrete Python type inside `Py` smart pointers. +pub trait PyTypeCheck: HasPyGilRef { + /// Name of self. This is used in error messages, for example. + const NAME: &'static str; + + /// Checks if `object` is an instance of `Self`, which may include a subtype. + /// + /// This should be equivalent to the Python expression `isinstance(object, Self)`. + fn type_check(object: &PyAny) -> bool; +} + +impl PyTypeCheck for T +where + T: PyTypeInfo, +{ + const NAME: &'static str = ::NAME; + + #[inline] + fn type_check(object: &PyAny) -> bool { + ::is_type_of(object) + } +} + #[inline] pub(crate) unsafe fn get_tp_alloc(tp: *mut ffi::PyTypeObject) -> Option { #[cfg(not(Py_LIMITED_API))] diff --git a/src/types/any.rs b/src/types/any.rs index 8d3cfc6d1a0..dc29b1dea56 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -1,9 +1,9 @@ use crate::class::basic::CompareOp; -use crate::conversion::{AsPyPointer, FromPyObject, IntoPy, PyTryFrom, ToPyObject}; +use crate::conversion::{AsPyPointer, FromPyObject, IntoPy, ToPyObject}; use crate::err::{PyDowncastError, PyErr, PyResult}; use crate::exceptions::{PyAttributeError, PyTypeError}; use crate::instance::Py2; -use crate::type_object::PyTypeInfo; +use crate::type_object::{HasPyGilRef, PyTypeCheck, PyTypeInfo}; #[cfg(not(PyPy))] use crate::types::PySuper; use crate::types::{PyDict, PyIterator, PyList, PyString, PyTuple, PyType}; @@ -776,11 +776,16 @@ impl PyAny { /// # } /// ``` #[inline] - pub fn downcast<'p, T>(&'p self) -> Result<&'p T, PyDowncastError<'_>> + pub fn downcast(&self) -> Result<&T, PyDowncastError<'_>> where - T: PyTryFrom<'p>, + T: PyTypeCheck, { - ::try_from(self) + if T::type_check(self) { + // Safety: type_check is responsible for ensuring that the type is correct + Ok(unsafe { self.downcast_unchecked() }) + } else { + Err(PyDowncastError::new(self, T::NAME)) + } } /// Downcast this `PyAny` to a concrete Python type or pyclass (but not a subclass of it). @@ -814,11 +819,16 @@ impl PyAny { /// }); /// ``` #[inline] - pub fn downcast_exact<'p, T>(&'p self) -> Result<&'p T, PyDowncastError<'_>> + pub fn downcast_exact(&self) -> Result<&T, PyDowncastError<'_>> where - T: PyTryFrom<'p>, + T: PyTypeInfo, { - ::try_from_exact(self) + if T::is_exact_type_of(self) { + // Safety: type_check is responsible for ensuring that the type is correct + Ok(unsafe { self.downcast_unchecked() }) + } else { + Err(PyDowncastError::new(self, T::NAME)) + } } /// Converts this `PyAny` to a concrete Python type without checking validity. @@ -827,16 +837,17 @@ impl PyAny { /// /// Callers must ensure that the type is valid or risk type confusion. #[inline] - pub unsafe fn downcast_unchecked<'p, T>(&'p self) -> &'p T + pub unsafe fn downcast_unchecked(&self) -> &T where - T: PyTryFrom<'p>, + T: HasPyGilRef, { - ::try_from_unchecked(self) + &*(self.as_ptr() as *const T) } /// Extracts some type from the Python object. /// /// This is a wrapper function around [`FromPyObject::extract()`]. + #[inline] pub fn extract<'a, D>(&'a self) -> PyResult where D: FromPyObject<'a>, @@ -1549,12 +1560,12 @@ pub(crate) trait PyAnyMethods<'py> { /// ``` fn downcast(&self) -> Result<&Py2<'py, T>, PyDowncastError<'py>> where - T: PyTypeInfo; + T: PyTypeCheck; /// Like `downcast` but takes ownership of `self`. fn downcast_into(self) -> Result, PyDowncastError<'py>> where - T: PyTypeInfo; + T: PyTypeCheck; /// Downcast this `PyAny` to a concrete Python type or pyclass (but not a subclass of it). /// @@ -2031,10 +2042,10 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> { #[inline] fn downcast(&self) -> Result<&Py2<'py, T>, PyDowncastError<'py>> where - T: PyTypeInfo, + T: PyTypeCheck, { - if self.is_instance_of::() { - // Safety: is_instance_of is responsible for ensuring that the type is correct + if T::type_check(self.as_gil_ref()) { + // Safety: type_check is responsible for ensuring that the type is correct Ok(unsafe { self.downcast_unchecked() }) } else { Err(PyDowncastError::new(self.clone().into_gil_ref(), T::NAME)) @@ -2044,10 +2055,10 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> { #[inline] fn downcast_into(self) -> Result, PyDowncastError<'py>> where - T: PyTypeInfo, + T: PyTypeCheck, { - if self.is_instance_of::() { - // Safety: is_instance_of is responsible for ensuring that the type is correct + if T::type_check(self.as_gil_ref()) { + // Safety: type_check is responsible for ensuring that the type is correct Ok(unsafe { self.downcast_into_unchecked() }) } else { Err(PyDowncastError::new(self.clone().into_gil_ref(), T::NAME)) diff --git a/src/types/ellipsis.rs b/src/types/ellipsis.rs index c1c594f8f24..e31f9d4183b 100644 --- a/src/types/ellipsis.rs +++ b/src/types/ellipsis.rs @@ -20,8 +20,6 @@ unsafe impl PyTypeInfo for PyEllipsis { const MODULE: Option<&'static str> = None; - type AsRefTarget = PyEllipsis; - fn type_object_raw(_py: Python<'_>) -> *mut ffi::PyTypeObject { unsafe { ffi::Py_TYPE(ffi::Py_Ellipsis()) } } diff --git a/src/types/iterator.rs b/src/types/iterator.rs index d88240f3880..310dbdf8204 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -1,5 +1,6 @@ -use crate::{ffi, AsPyPointer, Py, Py2, PyAny, PyErr, PyNativeType, PyResult, Python}; -use crate::{PyDowncastError, PyTryFrom}; +use crate::{ + ffi, AsPyPointer, Py2, PyAny, PyDowncastError, PyErr, PyNativeType, PyResult, PyTypeCheck, +}; use super::any::PyAnyMethods; @@ -33,11 +34,7 @@ impl PyIterator { /// /// Equivalent to Python's built-in `iter` function. pub fn from_object(obj: &PyAny) -> PyResult<&PyIterator> { - Self::from_object2(Py2::borrowed_from_gil_ref(&obj)).map(|py2| { - // Can't use into_gil_ref here because T: PyTypeInfo bound is not satisfied - // Safety: into_ptr produces a valid pointer to PyIterator object - unsafe { obj.py().from_owned_ptr(py2.into_ptr()) } - }) + Self::from_object2(Py2::borrowed_from_gil_ref(&obj)).map(Py2::into_gil_ref) } pub(crate) fn from_object2<'py>(obj: &Py2<'py, PyAny>) -> PyResult> { @@ -73,8 +70,15 @@ impl<'p> Iterator for &'p PyIterator { } } -// PyIter_Check does not exist in the limited API until 3.8 -impl<'v> PyTryFrom<'v> for PyIterator { +impl PyTypeCheck for PyIterator { + const NAME: &'static str = "Iterator"; + + fn type_check(object: &PyAny) -> bool { + unsafe { ffi::PyIter_Check(object.as_ptr()) != 0 } + } +} + +impl<'v> crate::PyTryFrom<'v> for PyIterator { fn try_from>(value: V) -> Result<&'v PyIterator, PyDowncastError<'v>> { let value = value.into(); unsafe { @@ -97,22 +101,6 @@ impl<'v> PyTryFrom<'v> for PyIterator { } } -impl Py { - /// Borrows a GIL-bound reference to the PyIterator. By binding to the GIL lifetime, this - /// allows the GIL-bound reference to not require `Python` for any of its methods. - pub fn as_ref<'py>(&'py self, _py: Python<'py>) -> &'py PyIterator { - let any = self.as_ptr() as *const PyAny; - unsafe { PyNativeType::unchecked_downcast(&*any) } - } - - /// Similar to [`as_ref`](#method.as_ref), and also consumes this `Py` and registers the - /// Python object reference in PyO3's object storage. The reference count for the Python - /// object will not be decreased until the GIL lifetime ends. - pub fn into_ref(self, py: Python<'_>) -> &PyIterator { - unsafe { py.from_owned_ptr(self.into_ptr()) } - } -} - #[cfg(test)] mod tests { use super::PyIterator; @@ -235,28 +223,6 @@ def fibonacci(target): }); } - #[test] - fn test_as_ref() { - Python::with_gil(|py| { - let iter: Py = PyAny::iter(PyList::empty(py)).unwrap().into(); - let mut iter_ref: &PyIterator = iter.as_ref(py); - assert!(iter_ref.next().is_none()); - }) - } - - #[test] - fn test_into_ref() { - Python::with_gil(|py| { - let bare_iter = PyAny::iter(PyList::empty(py)).unwrap(); - assert_eq!(bare_iter.get_refcnt(), 1); - let iter: Py = bare_iter.into(); - assert_eq!(bare_iter.get_refcnt(), 2); - let mut iter_ref = iter.into_ref(py); - assert!(iter_ref.next().is_none()); - assert_eq!(iter_ref.get_refcnt(), 2); - }) - } - #[test] #[cfg(feature = "macros")] fn python_class_not_iterator() { diff --git a/src/types/mapping.rs b/src/types/mapping.rs index 22e86d61e7f..e837ef911e9 100644 --- a/src/types/mapping.rs +++ b/src/types/mapping.rs @@ -2,7 +2,7 @@ use crate::err::{PyDowncastError, PyResult}; use crate::sync::GILOnceCell; use crate::type_object::PyTypeInfo; use crate::types::{PyAny, PyDict, PySequence, PyType}; -use crate::{ffi, Py, PyNativeType, PyTryFrom, Python, ToPyObject}; +use crate::{ffi, Py, PyNativeType, PyTypeCheck, Python, ToPyObject}; /// Represents a reference to a Python object supporting the mapping protocol. #[repr(transparent)] @@ -120,21 +120,29 @@ fn get_mapping_abc(py: Python<'_>) -> PyResult<&PyType> { .map(|ty| ty.as_ref(py)) } -impl<'v> PyTryFrom<'v> for PyMapping { +impl PyTypeCheck for PyMapping { + const NAME: &'static str = "Mapping"; + + #[inline] + fn type_check(object: &PyAny) -> bool { + // Using `is_instance` for `collections.abc.Mapping` is slow, so provide + // optimized case dict as a well-known mapping + PyDict::is_type_of(object) + || get_mapping_abc(object.py()) + .and_then(|abc| object.is_instance(abc)) + // TODO: surface errors in this chain to the user + .unwrap_or(false) + } +} + +impl<'v> crate::PyTryFrom<'v> for PyMapping { /// Downcasting to `PyMapping` requires the concrete class to be a subclass (or registered /// subclass) of `collections.abc.Mapping` (from the Python standard library) - i.e. /// `isinstance(, collections.abc.Mapping) == True`. fn try_from>(value: V) -> Result<&'v PyMapping, PyDowncastError<'v>> { let value = value.into(); - // Using `is_instance` for `collections.abc.Mapping` is slow, so provide - // optimized case dict as a well-known mapping - if PyDict::is_type_of(value) - || get_mapping_abc(value.py()) - .and_then(|abc| value.is_instance(abc)) - // TODO: surface errors in this chain to the user - .unwrap_or(false) - { + if PyMapping::type_check(value) { unsafe { return Ok(value.downcast_unchecked()) } } @@ -153,22 +161,6 @@ impl<'v> PyTryFrom<'v> for PyMapping { } } -impl Py { - /// Borrows a GIL-bound reference to the PyMapping. By binding to the GIL lifetime, this - /// allows the GIL-bound reference to not require `Python` for any of its methods. - pub fn as_ref<'py>(&'py self, _py: Python<'py>) -> &'py PyMapping { - let any = self.as_ptr() as *const PyAny; - unsafe { PyNativeType::unchecked_downcast(&*any) } - } - - /// Similar to [`as_ref`](#method.as_ref), and also consumes this `Py` and registers the - /// Python object reference in PyO3's object storage. The reference count for the Python - /// object will not be decreased until the GIL lifetime ends. - pub fn into_ref(self, py: Python<'_>) -> &PyMapping { - unsafe { py.from_owned_ptr(self.into_ptr()) } - } -} - #[cfg(test)] mod tests { use std::collections::HashMap; @@ -176,7 +168,7 @@ mod tests { use crate::{ exceptions::PyKeyError, types::{PyDict, PyTuple}, - Python, + PyTryFrom, Python, }; use super::*; @@ -326,24 +318,11 @@ mod tests { } #[test] - fn test_as_ref() { - Python::with_gil(|py| { - let mapping: Py = PyDict::new(py).as_mapping().into(); - let mapping_ref: &PyMapping = mapping.as_ref(py); - assert_eq!(mapping_ref.len().unwrap(), 0); - }) - } - - #[test] - fn test_into_ref() { + fn test_mapping_try_from() { Python::with_gil(|py| { - let bare_mapping = PyDict::new(py).as_mapping(); - assert_eq!(bare_mapping.get_refcnt(), 1); - let mapping: Py = bare_mapping.into(); - assert_eq!(bare_mapping.get_refcnt(), 2); - let mapping_ref = mapping.into_ref(py); - assert_eq!(mapping_ref.len().unwrap(), 0); - assert_eq!(mapping_ref.get_refcnt(), 2); - }) + let dict = PyDict::new(py); + let _ = ::try_from(dict).unwrap(); + let _ = PyMapping::try_from_exact(dict).unwrap(); + }); } } diff --git a/src/types/mod.rs b/src/types/mod.rs index c6bc1972991..b20fdf37305 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -192,8 +192,6 @@ macro_rules! pyobject_native_static_type_object( macro_rules! pyobject_native_type_info( ($name:ty, $typeobject:expr, $module:expr $(, #checkfunction=$checkfunction:path)? $(;$generics:ident)*) => { unsafe impl<$($generics,)*> $crate::type_object::PyTypeInfo for $name { - type AsRefTarget = Self; - const NAME: &'static str = stringify!($name); const MODULE: ::std::option::Option<&'static str> = $module; @@ -221,6 +219,7 @@ macro_rules! pyobject_native_type_info( macro_rules! pyobject_native_type_extract { ($name:ty $(;$generics:ident)*) => { impl<'py, $($generics,)*> $crate::FromPyObject<'py> for &'py $name { + #[inline] fn extract(obj: &'py $crate::PyAny) -> $crate::PyResult { obj.downcast().map_err(::std::convert::Into::into) } diff --git a/src/types/none.rs b/src/types/none.rs index fa189911151..19ee80ede57 100644 --- a/src/types/none.rs +++ b/src/types/none.rs @@ -20,8 +20,6 @@ unsafe impl PyTypeInfo for PyNone { const MODULE: Option<&'static str> = None; - type AsRefTarget = PyNone; - fn type_object_raw(_py: Python<'_>) -> *mut ffi::PyTypeObject { unsafe { ffi::Py_TYPE(ffi::Py_None()) } } diff --git a/src/types/notimplemented.rs b/src/types/notimplemented.rs index 2d4b7737053..0c61fd79bee 100644 --- a/src/types/notimplemented.rs +++ b/src/types/notimplemented.rs @@ -17,11 +17,8 @@ impl PyNotImplemented { unsafe impl PyTypeInfo for PyNotImplemented { const NAME: &'static str = "NotImplementedType"; - const MODULE: Option<&'static str> = None; - type AsRefTarget = PyNotImplemented; - fn type_object_raw(_py: Python<'_>) -> *mut ffi::PyTypeObject { unsafe { ffi::Py_TYPE(ffi::Py_NotImplemented()) } } diff --git a/src/types/sequence.rs b/src/types/sequence.rs index 80fd17a8e2f..6515f6b4bf3 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -6,9 +6,7 @@ use crate::internal_tricks::get_ssize_index; use crate::sync::GILOnceCell; use crate::type_object::PyTypeInfo; use crate::types::{PyAny, PyList, PyString, PyTuple, PyType}; -use crate::{ffi, PyNativeType, PyObject, ToPyObject}; -use crate::{FromPyObject, PyTryFrom}; -use crate::{Py, Python}; +use crate::{ffi, FromPyObject, Py, PyNativeType, PyObject, PyTypeCheck, Python, ToPyObject}; /// Represents a reference to a Python object supporting the sequence protocol. #[repr(transparent)] @@ -312,22 +310,30 @@ fn get_sequence_abc(py: Python<'_>) -> PyResult<&PyType> { .map(|ty| ty.as_ref(py)) } -impl<'v> PyTryFrom<'v> for PySequence { +impl PyTypeCheck for PySequence { + const NAME: &'static str = "Sequence"; + + #[inline] + fn type_check(object: &PyAny) -> bool { + // Using `is_instance` for `collections.abc.Sequence` is slow, so provide + // optimized cases for list and tuples as common well-known sequences + PyList::is_type_of(object) + || PyTuple::is_type_of(object) + || get_sequence_abc(object.py()) + .and_then(|abc| object.is_instance(abc)) + // TODO: surface errors in this chain to the user + .unwrap_or(false) + } +} + +impl<'v> crate::PyTryFrom<'v> for PySequence { /// Downcasting to `PySequence` requires the concrete class to be a subclass (or registered /// subclass) of `collections.abc.Sequence` (from the Python standard library) - i.e. /// `isinstance(, collections.abc.Sequence) == True`. fn try_from>(value: V) -> Result<&'v PySequence, PyDowncastError<'v>> { let value = value.into(); - // Using `is_instance` for `collections.abc.Sequence` is slow, so provide - // optimized cases for list and tuples as common well-known sequences - if PyList::is_type_of(value) - || PyTuple::is_type_of(value) - || get_sequence_abc(value.py()) - .and_then(|abc| value.is_instance(abc)) - // TODO: surface errors in this chain to the user - .unwrap_or(false) - { + if PySequence::type_check(value) { unsafe { return Ok(value.downcast_unchecked::()) } } @@ -345,36 +351,10 @@ impl<'v> PyTryFrom<'v> for PySequence { } } -impl Py { - /// Borrows a GIL-bound reference to the PySequence. By binding to the GIL lifetime, this - /// allows the GIL-bound reference to not require `Python` for any of its methods. - /// - /// ``` - /// # use pyo3::prelude::*; - /// # use pyo3::types::{PyList, PySequence}; - /// # Python::with_gil(|py| { - /// let seq: Py = PyList::empty(py).as_sequence().into(); - /// let seq: &PySequence = seq.as_ref(py); - /// assert_eq!(seq.len().unwrap(), 0); - /// # }); - /// ``` - pub fn as_ref<'py>(&'py self, _py: Python<'py>) -> &'py PySequence { - let any = self.as_ptr() as *const PyAny; - unsafe { PyNativeType::unchecked_downcast(&*any) } - } - - /// Similar to [`as_ref`](#method.as_ref), and also consumes this `Py` and registers the - /// Python object reference in PyO3's object storage. The reference count for the Python - /// object will not be decreased until the GIL lifetime ends. - pub fn into_ref(self, py: Python<'_>) -> &PySequence { - unsafe { py.from_owned_ptr(self.into_ptr()) } - } -} - #[cfg(test)] mod tests { use crate::types::{PyList, PySequence, PyTuple}; - use crate::{Py, PyObject, Python, ToPyObject}; + use crate::{PyObject, PyTryFrom, Python, ToPyObject}; fn get_object() -> PyObject { // Convenience function for getting a single unique object @@ -875,24 +855,11 @@ mod tests { } #[test] - fn test_as_ref() { - Python::with_gil(|py| { - let seq: Py = PyList::empty(py).as_sequence().into(); - let seq_ref: &PySequence = seq.as_ref(py); - assert_eq!(seq_ref.len().unwrap(), 0); - }) - } - - #[test] - fn test_into_ref() { + fn test_seq_try_from() { Python::with_gil(|py| { - let bare_seq = PyList::empty(py).as_sequence(); - assert_eq!(bare_seq.get_refcnt(), 1); - let seq: Py = bare_seq.into(); - assert_eq!(bare_seq.get_refcnt(), 2); - let seq_ref = seq.into_ref(py); - assert_eq!(seq_ref.len().unwrap(), 0); - assert_eq!(seq_ref.get_refcnt(), 2); - }) + let list = PyList::empty(py); + let _ = ::try_from(list).unwrap(); + let _ = PySequence::try_from_exact(list).unwrap(); + }); } } diff --git a/tests/test_gc.rs b/tests/test_gc.rs index bcb5c54350a..8fd4622f65e 100644 --- a/tests/test_gc.rs +++ b/tests/test_gc.rs @@ -3,7 +3,7 @@ use pyo3::class::PyTraverseError; use pyo3::class::PyVisit; use pyo3::prelude::*; -use pyo3::{py_run, PyCell, PyTryInto}; +use pyo3::{py_run, PyCell}; use std::cell::Cell; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; @@ -215,7 +215,7 @@ fn inheritance_with_new_methods_with_drop() { let typeobj = py.get_type::(); let inst = typeobj.call((), None).unwrap(); - let obj: &PyCell = PyTryInto::try_into(inst).unwrap(); + let obj: &PyCell = inst.downcast().unwrap(); let mut obj_ref_mut = obj.borrow_mut(); obj_ref_mut.data = Some(Arc::clone(&drop_called1)); let base: &mut BaseClassWithDrop = obj_ref_mut.as_mut();