Skip to content

Commit

Permalink
remove toborrowedobject trait
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Apr 25, 2022
1 parent 8ef9e54 commit 132dcd9
Show file tree
Hide file tree
Showing 12 changed files with 217 additions and 173 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Move `PyTypeObject::type_object` method to `PyTypeInfo` trait, and deprecate `PyTypeObject` trait. [#2284](https://github.com/PyO3/pyo3/pull/2284)
- The deprecated `pyproto` feature is now disabled by default. [#2321](https://github.com/PyO3/pyo3/pull/2321)
- Deprecate `ToBorrowedObject` trait (it is only used as a wrapper for `ToPyObject`). [#2330](https://github.com/PyO3/pyo3/pull/2330)

## [0.16.4] - 2022-04-14

Expand Down
11 changes: 7 additions & 4 deletions src/conversion.rs
Expand Up @@ -75,10 +75,12 @@ pub trait ToPyObject {
fn to_object(&self, py: Python<'_>) -> PyObject;
}

/// This trait has two implementations: The slow one is implemented for
/// all [ToPyObject] and creates a new object using [ToPyObject::to_object],
/// while the fast one is only implemented for AsPyPointer (we know
/// that every AsPyPointer is also ToPyObject) and uses [AsPyPointer::as_ptr()]
/// A deprecated conversion trait which relied on the unstable `specialization` feature
/// of the Rust language.
#[deprecated(
since = "0.17.0",
note = "this trait relied no longer used by PyO3, use ToPyObject or IntoPy<PyObject>"
)]
pub trait ToBorrowedObject: ToPyObject {
/// Converts self into a Python object and calls the specified closure
/// on the native FFI pointer underlying the Python object.
Expand All @@ -98,6 +100,7 @@ pub trait ToBorrowedObject: ToPyObject {
}
}

#[allow(deprecated)]
impl<T> ToBorrowedObject for T where T: ToPyObject {}

/// Defines a conversion from a Rust type to a Python object.
Expand Down
12 changes: 5 additions & 7 deletions src/err/mod.rs
Expand Up @@ -7,9 +7,7 @@ use crate::{
exceptions::{self, PyBaseException},
ffi,
};
use crate::{
AsPyPointer, IntoPy, IntoPyPointer, Py, PyAny, PyObject, Python, ToBorrowedObject, ToPyObject,
};
use crate::{AsPyPointer, IntoPy, IntoPyPointer, Py, PyAny, PyObject, Python, ToPyObject};
use std::borrow::Cow;
use std::cell::UnsafeCell;
use std::ffi::CString;
Expand Down Expand Up @@ -411,11 +409,11 @@ impl PyErr {
/// If `exc` is a tuple, all exceptions in the tuple (and recursively in subtuples) are searched for a match.
pub fn matches<T>(&self, py: Python<'_>, exc: T) -> bool
where
T: ToBorrowedObject,
T: ToPyObject,
{
exc.with_borrowed_ptr(py, |exc| unsafe {
ffi::PyErr_GivenExceptionMatches(self.type_ptr(py), exc) != 0
})
unsafe {
ffi::PyErr_GivenExceptionMatches(self.type_ptr(py), exc.to_object(py).as_ptr()) != 0
}
}

/// Returns true if the current exception is instance of `T`.
Expand Down
32 changes: 20 additions & 12 deletions src/instance.rs
@@ -1,5 +1,5 @@
// Copyright (c) 2017-present PyO3 Project and Contributors
use crate::conversion::{PyTryFrom, ToBorrowedObject};
use crate::conversion::PyTryFrom;
use crate::err::{self, PyDowncastError, PyErr, PyResult};
use crate::gil;
use crate::pycell::{PyBorrowError, PyBorrowMutError, PyCell};
Expand Down Expand Up @@ -563,9 +563,12 @@ impl<T> Py<T> {
where
N: ToPyObject,
{
attr_name.with_borrowed_ptr(py, |attr_name| unsafe {
PyObject::from_owned_ptr_or_err(py, ffi::PyObject_GetAttr(self.as_ptr(), attr_name))
})
unsafe {
PyObject::from_owned_ptr_or_err(
py,
ffi::PyObject_GetAttr(self.as_ptr(), attr_name.to_object(py).as_ptr()),
)
}
}

/// Sets an attribute value.
Expand Down Expand Up @@ -595,11 +598,16 @@ impl<T> Py<T> {
N: ToPyObject,
V: ToPyObject,
{
attr_name.with_borrowed_ptr(py, move |attr_name| {
value.with_borrowed_ptr(py, |value| unsafe {
err::error_on_minusone(py, ffi::PyObject_SetAttr(self.as_ptr(), attr_name, value))
})
})
unsafe {
err::error_on_minusone(
py,
ffi::PyObject_SetAttr(
self.as_ptr(),
attr_name.to_object(py).as_ptr(),
value.to_object(py).as_ptr(),
),
)
}
}

/// Calls the object.
Expand Down Expand Up @@ -656,10 +664,10 @@ impl<T> Py<T> {
args: impl IntoPy<Py<PyTuple>>,
kwargs: Option<&PyDict>,
) -> PyResult<PyObject> {
name.with_borrowed_ptr(py, |name| unsafe {
unsafe {
let args = args.into_py(py).into_ptr();
let kwargs = kwargs.into_ptr();
let ptr = ffi::PyObject_GetAttr(self.as_ptr(), name);
let ptr = ffi::PyObject_GetAttr(self.as_ptr(), name.to_object(py).as_ptr());
if ptr.is_null() {
return Err(PyErr::fetch(py));
}
Expand All @@ -668,7 +676,7 @@ impl<T> Py<T> {
ffi::Py_XDECREF(args);
ffi::Py_XDECREF(kwargs);
result
})
}
}

/// Calls a method on the object with only positional arguments.
Expand Down
4 changes: 3 additions & 1 deletion src/lib.rs
Expand Up @@ -294,9 +294,11 @@
//! [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::*;
#[allow(deprecated)]
pub use crate::conversion::ToBorrowedObject;
pub use crate::conversion::{
AsPyPointer, FromPyObject, FromPyPointer, IntoPy, IntoPyPointer, PyTryFrom, PyTryInto,
ToBorrowedObject, ToPyObject,
ToPyObject,
};
pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyResult};
#[cfg(not(PyPy))]
Expand Down
155 changes: 87 additions & 68 deletions src/types/any.rs
@@ -1,7 +1,5 @@
use crate::class::basic::CompareOp;
use crate::conversion::{
AsPyPointer, FromPyObject, IntoPy, IntoPyPointer, PyTryFrom, ToBorrowedObject, ToPyObject,
};
use crate::conversion::{AsPyPointer, FromPyObject, IntoPy, IntoPyPointer, PyTryFrom, ToPyObject};
use crate::err::{PyDowncastError, PyErr, PyResult};
use crate::exceptions::PyTypeError;
use crate::type_object::PyTypeInfo;
Expand Down Expand Up @@ -103,9 +101,9 @@ impl PyAny {
where
N: ToPyObject,
{
attr_name.with_borrowed_ptr(self.py(), |attr_name| unsafe {
Ok(ffi::PyObject_HasAttr(self.as_ptr(), attr_name) != 0)
})
unsafe {
Ok(ffi::PyObject_HasAttr(self.as_ptr(), attr_name.to_object(self.py()).as_ptr()) != 0)
}
}

/// Retrieves an attribute value.
Expand Down Expand Up @@ -134,10 +132,12 @@ impl PyAny {
where
N: ToPyObject,
{
attr_name.with_borrowed_ptr(self.py(), |attr_name| unsafe {
self.py()
.from_owned_ptr_or_err(ffi::PyObject_GetAttr(self.as_ptr(), attr_name))
})
unsafe {
self.py().from_owned_ptr_or_err(ffi::PyObject_GetAttr(
self.as_ptr(),
attr_name.to_object(self.py()).as_ptr(),
))
}
}

/// Sets an attribute value.
Expand All @@ -164,17 +164,20 @@ impl PyAny {
/// ```
pub fn setattr<N, V>(&self, attr_name: N, value: V) -> PyResult<()>
where
N: ToBorrowedObject,
V: ToBorrowedObject,
N: ToPyObject,
V: ToPyObject,
{
attr_name.with_borrowed_ptr(self.py(), move |attr_name| {
value.with_borrowed_ptr(self.py(), |value| unsafe {
err::error_on_minusone(
self.py(),
ffi::PyObject_SetAttr(self.as_ptr(), attr_name, value),
)
})
})
let py = self.py();
unsafe {
err::error_on_minusone(
py,
ffi::PyObject_SetAttr(
self.as_ptr(),
attr_name.to_object(py).as_ptr(),
value.to_object(py).as_ptr(),
),
)
}
}

/// Deletes an attribute.
Expand All @@ -184,9 +187,13 @@ impl PyAny {
where
N: ToPyObject,
{
attr_name.with_borrowed_ptr(self.py(), |attr_name| unsafe {
err::error_on_minusone(self.py(), ffi::PyObject_DelAttr(self.as_ptr(), attr_name))
})
let py = self.py();
unsafe {
err::error_on_minusone(
self.py(),
ffi::PyObject_DelAttr(self.as_ptr(), attr_name.to_object(py).as_ptr()),
)
}
}

/// Returns an [`Ordering`] between `self` and `other`.
Expand Down Expand Up @@ -239,26 +246,29 @@ impl PyAny {
where
O: ToPyObject,
{
self._compare(other.to_object(self.py()))
}

fn _compare(&self, other: PyObject) -> PyResult<Ordering> {
let py = self.py();
let other = other.as_ptr();
// Almost the same as ffi::PyObject_RichCompareBool, but this one doesn't try self == other.
// See https://github.com/PyO3/pyo3/issues/985 for more.
let do_compare = |other, op| unsafe {
PyObject::from_owned_ptr_or_err(py, ffi::PyObject_RichCompare(self.as_ptr(), other, op))
.and_then(|obj| obj.is_true(py))
};
other.with_borrowed_ptr(py, |other| {
if do_compare(other, ffi::Py_EQ)? {
Ok(Ordering::Equal)
} else if do_compare(other, ffi::Py_LT)? {
Ok(Ordering::Less)
} else if do_compare(other, ffi::Py_GT)? {
Ok(Ordering::Greater)
} else {
Err(PyTypeError::new_err(
"PyAny::compare(): All comparisons returned false",
))
}
})
if do_compare(other, ffi::Py_EQ)? {
Ok(Ordering::Equal)
} else if do_compare(other, ffi::Py_LT)? {
Ok(Ordering::Less)
} else if do_compare(other, ffi::Py_GT)? {
Ok(Ordering::Greater)
} else {
Err(PyTypeError::new_err(
"PyAny::compare(): All comparisons returned false",
))
}
}

/// Tests whether two Python objects obey a given [`CompareOp`].
Expand Down Expand Up @@ -296,13 +306,11 @@ impl PyAny {
O: ToPyObject,
{
unsafe {
other.with_borrowed_ptr(self.py(), |other| {
self.py().from_owned_ptr_or_err(ffi::PyObject_RichCompare(
self.as_ptr(),
other,
compare_op as c_int,
))
})
self.py().from_owned_ptr_or_err(ffi::PyObject_RichCompare(
self.as_ptr(),
other.to_object(self.py()).as_ptr(),
compare_op as c_int,
))
}
}

Expand Down Expand Up @@ -519,9 +527,9 @@ impl PyAny {
args: impl IntoPy<Py<PyTuple>>,
kwargs: Option<&PyDict>,
) -> PyResult<&PyAny> {
name.with_borrowed_ptr(self.py(), |name| unsafe {
unsafe {
let py = self.py();
let ptr = ffi::PyObject_GetAttr(self.as_ptr(), name);
let ptr = ffi::PyObject_GetAttr(self.as_ptr(), name.to_object(py).as_ptr());
if ptr.is_null() {
return Err(PyErr::fetch(py));
}
Expand All @@ -533,7 +541,7 @@ impl PyAny {
ffi::Py_XDECREF(args);
ffi::Py_XDECREF(kwargs);
result
})
}
}

/// Calls a method on the object without arguments.
Expand Down Expand Up @@ -639,39 +647,50 @@ impl PyAny {
/// This is equivalent to the Python expression `self[key]`.
pub fn get_item<K>(&self, key: K) -> PyResult<&PyAny>
where
K: ToBorrowedObject,
K: ToPyObject,
{
key.with_borrowed_ptr(self.py(), |key| unsafe {
self.py()
.from_owned_ptr_or_err(ffi::PyObject_GetItem(self.as_ptr(), key))
})
unsafe {
self.py().from_owned_ptr_or_err(ffi::PyObject_GetItem(
self.as_ptr(),
key.to_object(self.py()).as_ptr(),
))
}
}

/// Sets a collection item value.
///
/// This is equivalent to the Python expression `self[key] = value`.
pub fn set_item<K, V>(&self, key: K, value: V) -> PyResult<()>
where
K: ToBorrowedObject,
V: ToBorrowedObject,
K: ToPyObject,
V: ToPyObject,
{
key.with_borrowed_ptr(self.py(), move |key| {
value.with_borrowed_ptr(self.py(), |value| unsafe {
err::error_on_minusone(self.py(), ffi::PyObject_SetItem(self.as_ptr(), key, value))
})
})
let py = self.py();
unsafe {
err::error_on_minusone(
py,
ffi::PyObject_SetItem(
self.as_ptr(),
key.to_object(py).as_ptr(),
value.to_object(py).as_ptr(),
),
)
}
}

/// Deletes an item from the collection.
///
/// This is equivalent to the Python expression `del self[key]`.
pub fn del_item<K>(&self, key: K) -> PyResult<()>
where
K: ToBorrowedObject,
K: ToPyObject,
{
key.with_borrowed_ptr(self.py(), |key| unsafe {
err::error_on_minusone(self.py(), ffi::PyObject_DelItem(self.as_ptr(), key))
})
unsafe {
err::error_on_minusone(
self.py(),
ffi::PyObject_DelItem(self.as_ptr(), key.to_object(self.py()).as_ptr()),
)
}
}

/// Takes an object and returns an iterator for it.
Expand Down Expand Up @@ -789,15 +808,15 @@ impl PyAny {
/// Determines if self contains `value`.
///
/// This is equivalent to the Python expression `value in self`.
#[inline]
pub fn contains<V>(&self, value: V) -> PyResult<bool>
where
V: ToBorrowedObject,
V: ToPyObject,
{
let r = value.with_borrowed_ptr(self.py(), |ptr| unsafe {
ffi::PySequence_Contains(self.as_ptr(), ptr)
});
match r {
self._contains(value.to_object(self.py()))
}

fn _contains(&self, value: PyObject) -> PyResult<bool> {
match unsafe { ffi::PySequence_Contains(self.as_ptr(), value.as_ptr()) } {
0 => Ok(false),
1 => Ok(true),
_ => Err(PyErr::fetch(self.py())),
Expand Down

0 comments on commit 132dcd9

Please sign in to comment.