diff --git a/src/instance.rs b/src/instance.rs index d0421ec40f6..c6f648f65bb 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -67,54 +67,42 @@ impl<'py> Py2<'py, PyAny> { } impl<'py, T> Py2<'py, T> { - /// Helper to cast to Py2<'py, T> + /// Helper to cast to Py2<'py, PyAny> pub(crate) fn as_any(&self) -> &Py2<'py, PyAny> { + // Safety: all Py2 have the same memory layout, and all Py2 are valid Py2 unsafe { std::mem::transmute(self) } } } -impl<'py, T> std::fmt::Debug for Py2<'py, T> -where - Self: Deref>, -{ +impl<'py, T> std::fmt::Debug for Py2<'py, T> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { - let s = self.repr().or(Err(std::fmt::Error))?; - f.write_str(&s.as_gil_ref().to_string_lossy()) + let any = self.as_any(); + python_format(any, any.repr(), f) } } -impl<'py, T> std::fmt::Display for Py2<'py, T> -where - Self: Deref>, -{ +impl<'py, T> std::fmt::Display for Py2<'py, T> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { - match self.str() { - Result::Ok(s) => return f.write_str(&s.as_gil_ref().to_string_lossy()), - Result::Err(err) => { - err.write_unraisable(self.py(), std::option::Option::Some(self.as_gil_ref())) - } - } - - match self.get_type().name() { - Result::Ok(name) => std::write!(f, "", name), - Result::Err(_err) => f.write_str(""), - } + let any = self.as_any(); + python_format(any, any.str(), f) } } -impl<'py> std::fmt::Display for Py2<'py, PyAny> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { - match self.str() { - Result::Ok(s) => return f.write_str(&s.as_gil_ref().to_string_lossy()), - Result::Err(err) => { - err.write_unraisable(self.py(), std::option::Option::Some(self.as_gil_ref())) - } +fn python_format( + any: &Py2<'_, PyAny>, + format_result: PyResult>, + f: &mut std::fmt::Formatter<'_>, +) -> Result<(), std::fmt::Error> { + match format_result { + Result::Ok(s) => return f.write_str(&s.as_gil_ref().to_string_lossy()), + Result::Err(err) => { + err.write_unraisable(any.py(), std::option::Option::Some(any.as_gil_ref())) } + } - match self.get_type().name() { - Result::Ok(name) => std::write!(f, "", name), - Result::Err(_err) => f.write_str(""), - } + match any.get_type().name() { + Result::Ok(name) => std::write!(f, "", name), + Result::Err(_err) => f.write_str(""), } } @@ -126,14 +114,16 @@ where #[inline] fn deref(&self) -> &Py2<'py, PyAny> { - ::std::convert::AsRef::as_ref(self) + self.as_any() } } -impl<'py, T> AsRef> for Py2<'py, T> { +impl<'py, T> AsRef> for Py2<'py, T> +where + T: AsRef, +{ fn as_ref(&self) -> &Py2<'py, PyAny> { - // Safety: &Py2 has the same layout as &Py2 - unsafe { std::mem::transmute(self) } + self.as_any() } } @@ -176,9 +166,7 @@ impl<'py, T> Py2<'py, T> { /// of the pointer or decrease the reference count (e.g. with [`pyo3::ffi::Py_DecRef`](crate::ffi::Py_DecRef)). #[inline] pub fn into_ptr(self) -> *mut ffi::PyObject { - let ptr = (self.1).0.as_ptr(); - std::mem::forget(self); - ptr + self.into_non_null().as_ptr() } /// Internal helper to convert e.g. &'a &'py PyDict to &'a Py2<'py, PyDict> for @@ -211,10 +199,12 @@ impl<'py, T> Py2<'py, T> { unsafe { self.py().from_owned_ptr(self.into_ptr()) } } + // Internal helper to convert `self` into a `NonNull` which owns the + // Python reference. pub(crate) fn into_non_null(self) -> NonNull { - let ptr = (self.1).0; - std::mem::forget(self); - ptr + // wrap in ManuallyDrop to avoid running Drop for self and decreasing + // the reference count + ManuallyDrop::new(self).1 .0 } } @@ -1477,6 +1467,22 @@ a = A() }); } + #[test] + fn test_debug_fmt() { + Python::with_gil(|py| { + let obj = "hello world".to_object(py).attach_into(py); + assert_eq!(format!("{:?}", obj), "'hello world'"); + }); + } + + #[test] + fn test_display_fmt() { + Python::with_gil(|py| { + let obj = "hello world".to_object(py).attach_into(py); + assert_eq!(format!("{}", obj), "hello world"); + }); + } + #[cfg(feature = "macros")] mod using_macros { use super::*; diff --git a/src/types/any.rs b/src/types/any.rs index e15a5516e5d..ea03974357f 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -702,7 +702,13 @@ impl PyAny { /// This is typically a new iterator but if the argument is an iterator, /// this returns itself. pub fn iter(&self) -> PyResult<&PyIterator> { - PyIterator::from_object(self) + Py2::::borrowed_from_gil_ref(&self) + .iter() + .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 { self.py().from_owned_ptr(py2.into_ptr()) } + }) } /// Returns the Python type object for this object's type. @@ -1804,42 +1810,48 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> { where O: ToPyObject, { - self.rich_compare(other, CompareOp::Lt)?.is_true() + self.rich_compare(other, CompareOp::Lt) + .and_then(|any| any.is_true()) } fn le(&self, other: O) -> PyResult where O: ToPyObject, { - self.rich_compare(other, CompareOp::Le)?.is_true() + self.rich_compare(other, CompareOp::Le) + .and_then(|any| any.is_true()) } fn eq(&self, other: O) -> PyResult where O: ToPyObject, { - self.rich_compare(other, CompareOp::Eq)?.is_true() + self.rich_compare(other, CompareOp::Eq) + .and_then(|any| any.is_true()) } fn ne(&self, other: O) -> PyResult where O: ToPyObject, { - self.rich_compare(other, CompareOp::Ne)?.is_true() + self.rich_compare(other, CompareOp::Ne) + .and_then(|any| any.is_true()) } fn gt(&self, other: O) -> PyResult where O: ToPyObject, { - self.rich_compare(other, CompareOp::Gt)?.is_true() + self.rich_compare(other, CompareOp::Gt) + .and_then(|any| any.is_true()) } fn ge(&self, other: O) -> PyResult where O: ToPyObject, { - self.rich_compare(other, CompareOp::Ge)?.is_true() + self.rich_compare(other, CompareOp::Ge) + .and_then(|any| any.is_true()) } fn is_callable(&self) -> bool { @@ -2169,7 +2181,7 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> { mod tests { use crate::{ types::{IntoPyDict, PyAny, PyBool, PyList, PyLong, PyModule}, - Python, ToPyObject, + PyTypeInfo, Python, ToPyObject, }; #[test] @@ -2542,4 +2554,28 @@ class SimpleClass: assert!(!not_ellipsis.is_ellipsis()); }); } + + #[test] + fn test_is_callable() { + Python::with_gil(|py| { + assert!(PyList::type_object(py).is_callable()); + + let not_callable = 5.to_object(py).into_ref(py); + assert!(!not_callable.is_callable()); + }); + } + + #[test] + fn test_is_empty() { + Python::with_gil(|py| { + let empty_list: &PyAny = PyList::empty(py); + assert!(empty_list.is_empty().unwrap()); + + let list: &PyAny = PyList::new(py, vec![1, 2, 3]); + assert!(!list.is_empty().unwrap()); + + let not_container = 5.to_object(py).into_ref(py); + assert!(not_container.is_empty().is_err()); + }); + } } diff --git a/src/types/iterator.rs b/src/types/iterator.rs index d04205fd580..d88240f3880 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -33,10 +33,11 @@ impl PyIterator { /// /// Equivalent to Python's built-in `iter` function. pub fn from_object(obj: &PyAny) -> PyResult<&PyIterator> { - unsafe { - obj.py() - .from_owned_ptr_or_err(ffi::PyObject_GetIter(obj.as_ptr())) - } + 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()) } + }) } pub(crate) fn from_object2<'py>(obj: &Py2<'py, PyAny>) -> PyResult> { diff --git a/src/types/pysuper.rs b/src/types/pysuper.rs index 40a0cb819f4..e53eda8b719 100644 --- a/src/types/pysuper.rs +++ b/src/types/pysuper.rs @@ -1,7 +1,7 @@ -use crate::ffi; use crate::instance::Py2; use crate::types::any::PyAnyMethods; use crate::types::PyType; +use crate::{ffi, PyTypeInfo}; use crate::{PyAny, PyResult}; /// Represents a Python `super` object. @@ -57,20 +57,22 @@ impl PySuper { /// } /// ``` pub fn new<'py>(ty: &'py PyType, obj: &'py PyAny) -> PyResult<&'py PySuper> { - let py = ty.py(); - let super_ = py.get_type::().call1((ty, obj))?; - let super_ = super_.downcast::()?; - Ok(super_) + Self::new2( + Py2::borrowed_from_gil_ref(&ty), + Py2::borrowed_from_gil_ref(&obj), + ) + .map(Py2::into_gil_ref) } pub(crate) fn new2<'py>( ty: &Py2<'py, PyType>, obj: &Py2<'py, PyAny>, ) -> PyResult> { - let py = ty.py(); - let super_ = - Py2::::borrowed_from_gil_ref(&py.get_type::()).call1((ty, obj))?; - let super_ = super_.downcast_into::()?; - Ok(super_) + Py2::::borrowed_from_gil_ref(&PySuper::type_object(ty.py())) + .call1((ty, obj)) + .map(|any| { + // Safety: super() always returns instance of super + unsafe { any.downcast_into_unchecked() } + }) } }