Skip to content

Commit

Permalink
replace PyTryFrom by splitting PyTypeInfo
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Dec 5, 2023
1 parent 93370d9 commit ed87637
Show file tree
Hide file tree
Showing 20 changed files with 280 additions and 277 deletions.
4 changes: 3 additions & 1 deletion guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self>;
}
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]
Expand Down
36 changes: 36 additions & 0 deletions guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <PyInt as PyTryFrom>::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::<PyInt>()?;
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:
Expand Down
1 change: 1 addition & 0 deletions newsfragments/3600.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Split some `PyTypeInfo` functionality into new traits `HasPyGilRef` and `PyTypeCheck`.
4 changes: 3 additions & 1 deletion pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self>;
}

unsafe impl _pyo3::type_object::PyTypeInfo for #cls {
const NAME: &'static str = #cls_name;
const MODULE: ::std::option::Option<&'static str> = #module;

Expand Down
120 changes: 52 additions & 68 deletions src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ where
T: PyClass,
{
fn extract(obj: &'a PyAny) -> PyResult<Self> {
PyTryFrom::try_from(obj).map_err(Into::into)
obj.downcast().map_err(Into::into)
}
}

Expand All @@ -313,7 +313,7 @@ where
T: PyClass + Clone,
{
fn extract(obj: &'a PyAny) -> PyResult<Self> {
let cell: &PyCell<Self> = PyTryFrom::try_from(obj)?;
let cell: &PyCell<Self> = obj.downcast()?;
Ok(unsafe { cell.try_borrow_unguarded()?.clone() })
}
}
Expand All @@ -323,7 +323,7 @@ where
T: PyClass,
{
fn extract(obj: &'a PyAny) -> PyResult<Self> {
let cell: &PyCell<T> = PyTryFrom::try_from(obj)?;
let cell: &PyCell<T> = obj.downcast()?;
cell.try_borrow().map_err(Into::into)
}
}
Expand All @@ -333,7 +333,7 @@ where
T: PyClass<Frozen = False>,
{
fn extract(obj: &'a PyAny) -> PyResult<Self> {
let cell: &PyCell<T> = PyTryFrom::try_from(obj)?;
let cell: &PyCell<T> = obj.downcast()?;
cell.try_borrow_mut().map_err(Into::into)
}
}
Expand Down Expand Up @@ -381,78 +381,61 @@ pub trait PyTryInto<T>: Sized {
fn try_into_exact(&self) -> Result<&T, PyDowncastError<'_>>;
}

// TryFrom implies TryInto
impl<U> PyTryInto<U> for PyAny
where
U: for<'v> PyTryFrom<'v>,
{
fn try_into(&self) -> Result<&U, PyDowncastError<'_>> {
<U as PyTryFrom<'_>>::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<V: Into<&'v PyAny>>(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<U> PyTryInto<U> for PyAny
where
U: for<'v> PyTryFrom<'v>,
{
fn try_into(&self) -> Result<&U, PyDowncastError<'_>> {
<U as PyTryFrom<'_>>::try_from(self)
}
fn try_into_exact(&self) -> Result<&U, PyDowncastError<'_>> {
U::try_from_exact(self)
}
}

fn try_from_exact<V: Into<&'v PyAny>>(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<AsRefTarget = Self> + PyNativeType,
{
fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> {
value.into().downcast()
}
}

#[inline]
unsafe fn try_from_unchecked<V: Into<&'v PyAny>>(value: V) -> &'v Self {
Self::unchecked_downcast(value.into())
}
}
fn try_from_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> {
value.into().downcast_exact()
}

impl<'v, T> PyTryFrom<'v> for PyCell<T>
where
T: 'v + PyClass,
{
fn try_from<V: Into<&'v PyAny>>(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<V: Into<&'v PyAny>>(value: V) -> &'v Self {
value.into().downcast_unchecked()
}
}
fn try_from_exact<V: Into<&'v PyAny>>(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<T>
where
T: 'v + PyClass,
{
fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> {
value.into().downcast()
}
fn try_from_exact<V: Into<&'v PyAny>>(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<V: Into<&'v PyAny>>(value: V) -> &'v Self {
Self::unchecked_downcast(value.into())
#[inline]
unsafe fn try_from_unchecked<V: Into<&'v PyAny>>(value: V) -> &'v Self {
value.into().downcast_unchecked()
}
}
}

Expand Down Expand Up @@ -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() {
Expand Down
32 changes: 22 additions & 10 deletions src/instance.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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()) }
}
Expand All @@ -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()) }
}
Expand Down Expand Up @@ -437,7 +437,7 @@ where

impl<T> Py<T>
where
T: PyTypeInfo,
T: HasPyGilRef,
{
/// Borrows a GIL-bound reference to the contained `T`.
///
Expand Down Expand Up @@ -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<AsRefTarget = T>,
{
<T as PyTryFrom<'_>>::try_from(self.as_ref(py))
self.as_ref(py).downcast()
}

/// Casts the PyObject to a concrete Python object type without checking validity.
Expand All @@ -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<AsRefTarget = T>,
{
<T as PyTryFrom<'_>>::try_from_unchecked(self.as_ref(py))
self.as_ref(py).downcast_unchecked()
}
}

Expand Down Expand Up @@ -1509,6 +1509,8 @@ a = A()

#[cfg(feature = "macros")]
mod using_macros {
use crate::{PyCell, PyTryInto};

use super::*;

#[crate::pyclass]
Expand All @@ -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<SomeClass> = PyTryInto::try_into(instance).unwrap();
let _: &PyCell<SomeClass> = PyTryInto::try_into_exact(instance).unwrap();
})
}
}
}
7 changes: 3 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand All @@ -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;

Expand Down

0 comments on commit ed87637

Please sign in to comment.