Skip to content

replace PyTryFrom by splitting PyTypeInfo #3600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading