From 8e2e3af67436906815cef30c5ade108e93431b82 Mon Sep 17 00:00:00 2001 From: mejrs Date: Thu, 21 Apr 2022 02:51:22 +0200 Subject: [PATCH 1/7] Allow more methods to take interned arguments --- src/conversions/path.rs | 7 +- src/err/mod.rs | 7 +- src/instance.rs | 64 ++++++---- src/marker.rs | 11 +- src/types/any.rs | 249 +++++++++++++++++++++++-------------- src/types/function.rs | 2 +- src/types/module.rs | 9 +- src/types/string.rs | 9 +- src/types/traceback.rs | 7 +- tests/test_frompyobject.rs | 6 +- 10 files changed, 233 insertions(+), 138 deletions(-) diff --git a/src/conversions/path.rs b/src/conversions/path.rs index a5f04a5db38..a82d2942bf3 100644 --- a/src/conversions/path.rs +++ b/src/conversions/path.rs @@ -1,3 +1,4 @@ +use crate::intern; use crate::types::PyType; use crate::{FromPyObject, IntoPy, PyAny, PyObject, PyResult, Python, ToPyObject}; use std::borrow::Cow; @@ -18,10 +19,10 @@ impl FromPyObject<'_> for PathBuf { Ok(s) => s, Err(err) => { let py = ob.py(); - let pathlib = py.import("pathlib")?; - let pathlib_path: &PyType = pathlib.getattr("Path")?.downcast()?; + let pathlib = py.import(intern!(py, "pathlib"))?; + let pathlib_path: &PyType = pathlib.getattr(intern!(py, "Path"))?.downcast()?; if ob.is_instance(pathlib_path)? { - let path_str = ob.call_method0("__str__")?; + let path_str = ob.call_method0(intern!(py, "__str__"))?; OsString::extract(path_str)? } else { return Err(err); diff --git a/src/err/mod.rs b/src/err/mod.rs index eed6798ad58..12dba87cb1f 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -137,7 +137,10 @@ impl PyErr { /// /// # Examples /// ```rust - /// use pyo3::{exceptions::PyTypeError, types::PyType, IntoPy, PyErr, Python}; + /// use pyo3::prelude::*; + /// use pyo3::exceptions::PyTypeError; + /// use pyo3::types::{PyType, PyString}; + /// /// Python::with_gil(|py| { /// // Case #1: Exception object /// let err = PyErr::from_value(PyTypeError::new_err("some type error").value(py)); @@ -148,7 +151,7 @@ impl PyErr { /// assert_eq!(err.to_string(), "TypeError: "); /// /// // Case #3: Invalid exception value - /// let err = PyErr::from_value("foo".into_py(py).as_ref(py)); + /// let err = PyErr::from_value(PyString::new(py, "foo").into()); /// assert_eq!( /// err.to_string(), /// "TypeError: exceptions must derive from BaseException" diff --git a/src/instance.rs b/src/instance.rs index a8c0997bf1f..dce0efd2770 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -3,7 +3,7 @@ use crate::conversion::{PyTryFrom, ToBorrowedObject}; use crate::err::{self, PyDowncastError, PyErr, PyResult}; use crate::gil; use crate::pycell::{PyBorrowError, PyBorrowMutError, PyCell}; -use crate::types::{PyDict, PyTuple}; +use crate::types::{PyDict, PyString, PyTuple}; use crate::{ ffi, AsPyPointer, FromPyObject, IntoPy, IntoPyPointer, PyAny, PyClass, PyClassInitializer, PyRef, PyRefMut, PyTypeInfo, Python, ToPyObject, @@ -566,8 +566,8 @@ impl Py { /// /// This is equivalent to the Python expression `self.attr_name = value`. /// - /// If calling this method becomes performance-critical, the [`intern!`] macro can be used - /// to intern `attr_name`, thereby avoiding repeated temporary allocations of Python strings. + /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used + /// to intern `attr_name`. /// /// # Example: `intern!`ing the attribute name /// @@ -643,49 +643,65 @@ impl Py { /// Calls a method on the object. /// /// This is equivalent to the Python expression `self.name(*args, **kwargs)`. - pub fn call_method( + /// + /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used + /// to intern `name`. + pub fn call_method( &self, py: Python<'_>, - name: &str, - args: impl IntoPy>, + name: N, + args: A, kwargs: Option<&PyDict>, - ) -> PyResult { - name.with_borrowed_ptr(py, |name| unsafe { - let args = args.into_py(py).into_ptr(); - let kwargs = kwargs.into_ptr(); - let ptr = ffi::PyObject_GetAttr(self.as_ptr(), name); - if ptr.is_null() { - return Err(PyErr::fetch(py)); - } + ) -> PyResult + where + N: IntoPy>, + A: IntoPy>, + { + let name: Py = name.into_py(py); + let args: Py = args.into_py(py); + + let ptr = self.getattr(py, name)?.into_ptr(); + let args = args.into_ptr(); + let kwargs = kwargs.into_ptr(); + + unsafe { let result = PyObject::from_owned_ptr_or_err(py, ffi::PyObject_Call(ptr, args, kwargs)); ffi::Py_DECREF(ptr); ffi::Py_XDECREF(args); ffi::Py_XDECREF(kwargs); result - }) + } } /// Calls a method on the object with only positional arguments. /// /// This is equivalent to the Python expression `self.name(*args)`. - pub fn call_method1( - &self, - py: Python<'_>, - name: &str, - args: impl IntoPy>, - ) -> PyResult { + /// + /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used + /// to intern `name`. + pub fn call_method1(&self, py: Python<'_>, name: N, args: A) -> PyResult + where + N: IntoPy>, + A: IntoPy>, + { self.call_method(py, name, args, None) } /// Calls a method on the object with no arguments. /// /// This is equivalent to the Python expression `self.name()`. - pub fn call_method0(&self, py: Python<'_>, name: &str) -> PyResult { + /// + /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used + /// to intern `name`. + pub fn call_method0(&self, py: Python<'_>, name: N) -> PyResult + where + N: IntoPy>, + { cfg_if::cfg_if! { if #[cfg(all(Py_3_9, not(any(Py_LIMITED_API, PyPy))))] { // Optimized path on python 3.9+ unsafe { - let name = name.into_py(py); + let name: Py = name.into_py(py); PyObject::from_owned_ptr_or_err(py, ffi::PyObject_CallMethodNoArgs(self.as_ptr(), name.as_ptr())) } } else { @@ -714,7 +730,7 @@ impl Py { /// Create a `Py` instance by taking ownership of the given FFI pointer. /// - /// If `ptr` is null then the current Python exception is fetched as a `PyErr`. + /// If `ptr` is null then the current Python exception is fetched as a [`PyErr`]. /// /// # Safety /// If non-null, `ptr` must be a pointer to a Python object of type T. diff --git a/src/marker.rs b/src/marker.rs index f159d89e85e..760586d109e 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -123,9 +123,11 @@ use crate::err::{self, PyDowncastError, PyErr, PyResult}; use crate::gil::{self, GILGuard, GILPool}; use crate::impl_::not_send::NotSend; use crate::type_object::{PyTypeInfo, PyTypeObject}; -use crate::types::{PyAny, PyDict, PyModule, PyType}; +use crate::types::{PyAny, PyDict, PyModule, PyString, PyType}; use crate::version::PythonVersionInfo; -use crate::{ffi, AsPyPointer, FromPyPointer, IntoPyPointer, PyNativeType, PyObject, PyTryFrom}; +use crate::{ + ffi, AsPyPointer, FromPyPointer, IntoPy, IntoPyPointer, Py, PyNativeType, PyObject, PyTryFrom, +}; use std::ffi::{CStr, CString}; use std::marker::PhantomData; use std::os::raw::c_int; @@ -589,7 +591,10 @@ impl<'py> Python<'py> { } /// Imports the Python module with the specified name. - pub fn import(self, name: &str) -> PyResult<&'py PyModule> { + pub fn import(self, name: N) -> PyResult<&'py PyModule> + where + N: IntoPy>, + { PyModule::import(self, name) } diff --git a/src/types/any.rs b/src/types/any.rs index 9ba9b515944..2d4cd81d353 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -99,6 +99,9 @@ impl PyAny { /// Determines whether this object has the given attribute. /// /// This is equivalent to the Python expression `hasattr(self, attr_name)`. + /// + /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used + /// to intern `attr_name`. pub fn hasattr(&self, attr_name: N) -> PyResult where N: ToPyObject, @@ -112,8 +115,8 @@ impl PyAny { /// /// This is equivalent to the Python expression `self.attr_name`. /// - /// If calling this method becomes performance-critical, the [`intern!`] macro can be used - /// to intern `attr_name`, thereby avoiding repeated temporary allocations of Python strings. + /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used + /// to intern `attr_name`. /// /// # Example: `intern!`ing the attribute name /// @@ -144,8 +147,8 @@ impl PyAny { /// /// This is equivalent to the Python expression `self.attr_name = value`. /// - /// If calling this method becomes performance-critical, the [`intern!`] macro can be used - /// to intern `attr_name`, thereby avoiding repeated temporary allocations of Python strings. + /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used + /// to intern `name`. /// /// # Example: `intern!`ing the attribute name /// @@ -180,6 +183,9 @@ impl PyAny { /// Deletes an attribute. /// /// This is equivalent to the Python statement `del self.attr_name`. + /// + /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used + /// to intern `attr_name`. pub fn delattr(&self, attr_name: N) -> PyResult<()> where N: ToPyObject, @@ -263,6 +269,10 @@ impl PyAny { /// Tests whether two Python objects obey a given [`CompareOp`]. /// + /// [`lt`](Self::lt), [`le`](Self::le), [`eq`](Self::eq), [`ne`](Self::ne), + /// [`gt`](Self::gt) and [`ge`](Self::ge) are the specialized versions + /// of this function. + /// /// Depending on the value of `compare_op`, this is equivalent to one of the /// following Python expressions: /// @@ -400,6 +410,33 @@ impl PyAny { /// Calls the object. /// /// This is equivalent to the Python expression `self(*args, **kwargs)`. + /// + /// # Examples + /// + /// ```rust + /// use pyo3::prelude::*; + /// use pyo3::types::PyDict; + /// + /// const CODE: &str = r#" + /// def function(*args, **kwargs): + /// assert args == ("hello",) + /// assert kwargs == {"cruel": "world"} + /// return "called with args and kwargs" + /// "#; + /// + /// # fn main() -> PyResult<()> { + /// Python::with_gil(|py| { + /// let module = PyModule::from_code(py, CODE, "", "")?; + /// let fun = module.getattr("function")?; + /// let args = ("hello",); + /// let kwargs = PyDict::new(py); + /// kwargs.set_item("cruel", "world")?; + /// let result = fun.call(args, Some(kwargs))?; + /// assert_eq!(result.extract::<&str>()?, "called with args and kwargs"); + /// Ok(()) + /// }) + /// # } + /// ``` pub fn call( &self, args: impl IntoPy>, @@ -460,25 +497,23 @@ impl PyAny { /// ```rust /// use pyo3::prelude::*; /// + /// const CODE: &str = r#" + /// def function(*args, **kwargs): + /// assert args == ("hello",) + /// assert kwargs == {} + /// return "called with args" + /// "#; + /// /// # fn main() -> PyResult<()> { - /// Python::with_gil(|py| -> PyResult<()> { - /// let module = PyModule::import(py, "operator")?; - /// let add = module.getattr("add")?; - /// let args = (1, 2); - /// let value = add.call1(args)?; - /// assert_eq!(value.extract::()?, 3); + /// Python::with_gil(|py| { + /// let module = PyModule::from_code(py, CODE, "", "")?; + /// let fun = module.getattr("function")?; + /// let args = ("hello",); + /// let result = fun.call1(args)?; + /// assert_eq!(result.extract::<&str>()?, "called with args"); /// Ok(()) - /// })?; - /// # Ok(())} - /// ``` - /// - /// This is equivalent to the following Python code: - /// - /// ```python - /// from operator import add - /// - /// value = add(1,2) - /// assert value == 3 + /// }) + /// # } /// ``` pub fn call1(&self, args: impl IntoPy>) -> PyResult<&PyAny> { self.call(args, None) @@ -488,91 +523,104 @@ impl PyAny { /// /// This is equivalent to the Python expression `self.name(*args, **kwargs)`. /// + /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used + /// to intern `name`. + /// /// # Examples /// /// ```rust /// use pyo3::prelude::*; - /// use pyo3::types::{IntoPyDict, PyList}; + /// use pyo3::types::PyDict; /// - /// # fn main() -> PyResult<()> { - /// Python::with_gil(|py| -> PyResult<()> { - /// let list = PyList::new(py, vec![3, 6, 5, 4, 7]); - /// let kwargs = vec![("reverse", true)].into_py_dict(py); + /// const CODE: &str = r#" + /// class A: + /// def method(self, *args, **kwargs): + /// assert args == ("hello",) + /// assert kwargs == {"cruel": "world"} + /// return "called with args and kwargs" + /// a = A() + /// "#; /// - /// list.call_method("sort", (), Some(kwargs))?; - /// assert_eq!(list.extract::>()?, vec![7, 6, 5, 4, 3]); + /// # fn main() -> PyResult<()> { + /// Python::with_gil(|py| { + /// let module = PyModule::from_code(py, CODE, "", "")?; + /// let instance = module.getattr("a")?; + /// let args = ("hello",); + /// let kwargs = PyDict::new(py); + /// kwargs.set_item("cruel", "world")?; + /// let result = instance.call_method("method", args, Some(kwargs))?; + /// assert_eq!(result.extract::<&str>()?, "called with args and kwargs"); /// Ok(()) - /// })?; - /// # Ok(())} - /// ``` - /// - /// This is equivalent to the following Python code: - /// - /// ```python - /// my_list = [3, 6, 5, 4, 7] - /// my_list.sort(reverse = True) - /// assert my_list == [7, 6, 5, 4, 3] + /// }) + /// # } /// ``` - pub fn call_method( - &self, - name: &str, - args: impl IntoPy>, - kwargs: Option<&PyDict>, - ) -> PyResult<&PyAny> { - name.with_borrowed_ptr(self.py(), |name| unsafe { - let py = self.py(); - let ptr = ffi::PyObject_GetAttr(self.as_ptr(), name); - if ptr.is_null() { - return Err(PyErr::fetch(py)); - } - let args = args.into_py(py).into_ptr(); - let kwargs = kwargs.into_ptr(); + pub fn call_method(&self, name: N, args: A, kwargs: Option<&PyDict>) -> PyResult<&PyAny> + where + N: IntoPy>, + A: IntoPy>, + { + let py = self.py(); + let name: Py = name.into_py(py); + let args: Py = args.into_py(py); + + let ptr = self.getattr(name)?.into_ptr(); + let args = args.into_ptr(); + let kwargs = kwargs.into_ptr(); + + unsafe { let result_ptr = ffi::PyObject_Call(ptr, args, kwargs); let result = py.from_owned_ptr_or_err(result_ptr); ffi::Py_DECREF(ptr); ffi::Py_XDECREF(args); ffi::Py_XDECREF(kwargs); result - }) + } } /// Calls a method on the object without arguments. /// /// This is equivalent to the Python expression `self.name()`. /// + /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used + /// to intern `name`. + /// /// # Examples /// /// ```rust /// use pyo3::prelude::*; - /// use pyo3::types::PyFloat; - /// use std::f64::consts::PI; + /// + /// const CODE: &str = r#" + /// class A: + /// def method(self, *args, **kwargs): + /// assert args == () + /// assert kwargs == {} + /// return "called with no arguments" + /// a = A() + /// "#; /// /// # fn main() -> PyResult<()> { - /// Python::with_gil(|py| -> PyResult<()> { - /// let pi = PyFloat::new(py, PI); - /// let ratio = pi.call_method0("as_integer_ratio")?; - /// let (a, b) = ratio.extract::<(u64, u64)>()?; - /// assert_eq!(a, 884_279_719_003_555); - /// assert_eq!(b, 281_474_976_710_656); + /// Python::with_gil(|py| { + /// let module = PyModule::from_code(py, CODE, "", "")?; + /// let instance = module.getattr("a")?; + /// let result = instance.call_method0("method")?; + /// assert_eq!(result.extract::<&str>()?, "called with no arguments"); /// Ok(()) - /// })?; - /// # Ok(())} + /// }) + /// # } /// ``` - /// - /// This is equivalent to the following Python code: - /// - /// ```python - /// import math - /// - /// a, b = math.pi.as_integer_ratio() - /// ``` - pub fn call_method0(&self, name: &str) -> PyResult<&PyAny> { + pub fn call_method0(&self, name: N) -> PyResult<&PyAny> + where + N: IntoPy>, + { cfg_if::cfg_if! { if #[cfg(all(Py_3_9, not(any(Py_LIMITED_API, PyPy))))] { + let py = self.py(); + // Optimized path on python 3.9+ unsafe { - let name = name.into_py(self.py()); - self.py().from_owned_ptr_or_err(ffi::PyObject_CallMethodNoArgs(self.as_ptr(), name.as_ptr())) + let name: Py = name.into_py(py); + let ptr = ffi::PyObject_CallMethodNoArgs(self.as_ptr(), name.as_ptr()); + py.from_owned_ptr_or_err(ptr) } } else { self.call_method(name, (), None) @@ -584,30 +632,39 @@ impl PyAny { /// /// This is equivalent to the Python expression `self.name(*args)`. /// + /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used + /// to intern `name`. + /// /// # Examples /// /// ```rust /// use pyo3::prelude::*; - /// use pyo3::types::PyList; + /// + /// const CODE: &str = r#" + /// class A: + /// def method(self, *args, **kwargs): + /// assert args == ("hello",) + /// assert kwargs == {} + /// return "called with args" + /// a = A() + /// "#; /// /// # fn main() -> PyResult<()> { - /// Python::with_gil(|py| -> PyResult<()> { - /// let list = PyList::new(py, vec![1, 3, 4]); - /// list.call_method1("insert", (1, 2))?; - /// assert_eq!(list.extract::>()?, [1, 2, 3, 4]); + /// Python::with_gil(|py| { + /// let module = PyModule::from_code(py, CODE, "", "")?; + /// let instance = module.getattr("a")?; + /// let args = ("hello",); + /// let result = instance.call_method1("method", args)?; + /// assert_eq!(result.extract::<&str>()?, "called with args"); /// Ok(()) - /// })?; - /// # Ok(()) } + /// }) + /// # } /// ``` - /// - /// This is equivalent to the following Python code: - /// - /// ```python - /// list_ = [1,3,4] - /// list_.insert(1,2) - /// assert list_ == [1,2,3,4] - /// ``` - pub fn call_method1(&self, name: &str, args: impl IntoPy>) -> PyResult<&PyAny> { + pub fn call_method1(&self, name: N, args: A) -> PyResult<&PyAny> + where + N: IntoPy>, + A: IntoPy>, + { self.call_method(name, args, None) } @@ -693,7 +750,7 @@ impl PyAny { unsafe { ffi::Py_TYPE(self.as_ptr()) } } - /// Casts the PyObject to a concrete Python object type. + /// Casts `self` to a concrete Python object type. /// /// This can cast only to native Python types, not types implemented in Rust. pub fn cast_as<'a, D>(&'a self) -> Result<&'a D, PyDowncastError<'_>> @@ -705,7 +762,7 @@ impl PyAny { /// Extracts some type from the Python object. /// - /// This is a wrapper function around `FromPyObject::extract()`. + /// This is a wrapper function around [`FromPyObject::extract()`]. pub fn extract<'a, D>(&'a self) -> PyResult where D: FromPyObject<'a>, @@ -769,11 +826,11 @@ impl PyAny { unsafe { self.py().from_owned_ptr(ffi::PyObject_Dir(self.as_ptr())) } } - /// Checks whether this object is an instance of type `typ`. + /// Checks whether this object is an instance of type `ty`. /// - /// This is equivalent to the Python expression `isinstance(self, typ)`. - pub fn is_instance(&self, typ: &PyType) -> PyResult { - let result = unsafe { ffi::PyObject_IsInstance(self.as_ptr(), typ.as_ptr()) }; + /// This is equivalent to the Python expression `isinstance(self, ty)`. + pub fn is_instance(&self, ty: &PyType) -> PyResult { + let result = unsafe { ffi::PyObject_IsInstance(self.as_ptr(), ty.as_ptr()) }; err::error_on_minusone(self.py(), result)?; Ok(result == 1) } diff --git a/src/types/function.rs b/src/types/function.rs index 297b23c9242..667c8ceebba 100644 --- a/src/types/function.rs +++ b/src/types/function.rs @@ -155,7 +155,7 @@ impl PyCFunction { let (py, module) = py_or_module.into_py_and_maybe_module(); let (mod_ptr, module_name) = if let Some(m) = module { let mod_ptr = m.as_ptr(); - let name = m.name()?.into_py(py); + let name: Py = m.name()?.into_py(py); (mod_ptr, name.as_ptr()) } else { (std::ptr::null_mut(), std::ptr::null_mut()) diff --git a/src/types/module.rs b/src/types/module.rs index 2838734b473..0ee74355019 100644 --- a/src/types/module.rs +++ b/src/types/module.rs @@ -9,7 +9,7 @@ use crate::ffi; use crate::pyclass::PyClass; use crate::type_object::PyTypeObject; use crate::types::{PyAny, PyCFunction, PyDict, PyList, PyString}; -use crate::{AsPyPointer, IntoPy, PyObject, Python}; +use crate::{AsPyPointer, IntoPy, Py, PyObject, Python}; use std::ffi::{CStr, CString}; use std::str; @@ -66,8 +66,11 @@ impl PyModule { /// ```python /// import antigravity /// ``` - pub fn import<'p>(py: Python<'p>, name: &str) -> PyResult<&'p PyModule> { - let name: PyObject = name.into_py(py); + pub fn import(py: Python<'_>, name: N) -> PyResult<&PyModule> + where + N: IntoPy>, + { + let name: Py = name.into_py(py); unsafe { py.from_owned_ptr_or_err(ffi::PyImport_Import(name.as_ptr())) } } diff --git a/src/types/string.rs b/src/types/string.rs index 4e906f8ce51..413e1d193ce 100644 --- a/src/types/string.rs +++ b/src/types/string.rs @@ -4,7 +4,7 @@ use crate::exceptions::PyUnicodeDecodeError; use crate::types::PyBytes; use crate::{ - ffi, AsPyPointer, FromPyObject, IntoPy, PyAny, PyObject, PyResult, PyTryFrom, Python, + ffi, AsPyPointer, FromPyObject, IntoPy, Py, PyAny, PyObject, PyResult, PyTryFrom, Python, ToPyObject, }; use std::borrow::Cow; @@ -298,6 +298,13 @@ impl<'a> IntoPy for &'a str { } } +impl<'a> IntoPy> for &'a str { + #[inline] + fn into_py(self, py: Python<'_>) -> Py { + PyString::new(py, self).into() + } +} + /// Converts a Rust `Cow<'_, str>` to a Python object. /// See `PyString::new` for details on the conversion. impl<'a> ToPyObject for Cow<'a, str> { diff --git a/src/types/traceback.rs b/src/types/traceback.rs index 6d799a703f4..7e4393eb98d 100644 --- a/src/types/traceback.rs +++ b/src/types/traceback.rs @@ -49,11 +49,14 @@ impl PyTraceback { /// ``` pub fn format(&self) -> PyResult { let py = self.py(); - let string_io = py.import("io")?.getattr("StringIO")?.call0()?; + let string_io = py + .import(intern!(py, "io"))? + .getattr(intern!(py, "StringIO"))? + .call0()?; let result = unsafe { ffi::PyTraceBack_Print(self.as_ptr(), string_io.as_ptr()) }; error_on_minusone(py, result)?; let formatted = string_io - .getattr("getvalue")? + .getattr(intern!(py, "getvalue"))? .call0()? .downcast::()? .to_str()? diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index 7215ba82e24..b93be185087 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -74,7 +74,7 @@ pub struct B { #[test] fn test_transparent_named_field_struct() { Python::with_gil(|py| { - let test = "test".into_py(py); + let test: PyObject = "test".into_py(py); let b: B = FromPyObject::extract(test.as_ref(py)).expect("Failed to extract B from String"); assert_eq!(b.test, "test"); let test: PyObject = 1.into_py(py); @@ -92,7 +92,7 @@ pub struct D { #[test] fn test_generic_transparent_named_field_struct() { Python::with_gil(|py| { - let test = "test".into_py(py); + let test: PyObject = "test".into_py(py); let d: D = D::extract(test.as_ref(py)).expect("Failed to extract D from String"); assert_eq!(d.test, "test"); @@ -180,7 +180,7 @@ fn test_transparent_tuple_struct() { let tup: PyObject = 1.into_py(py); let tup = TransparentTuple::extract(tup.as_ref(py)); assert!(tup.is_err()); - let test = "test".into_py(py); + let test: PyObject = "test".into_py(py); let tup = TransparentTuple::extract(test.as_ref(py)) .expect("Failed to extract TransparentTuple from PyTuple"); assert_eq!(tup.0, "test"); From 7f96584ec862bd55acf28017d1086c26c9697e30 Mon Sep 17 00:00:00 2001 From: mejrs Date: Thu, 21 Apr 2022 03:00:49 +0200 Subject: [PATCH 2/7] Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddfece05260..cd159d7f40c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Implement `ToPyObject` for `[T; N]`. [#2313](https://github.com/PyO3/pyo3/pull/2313) +### Changed + +Several methods of `Py` and `PyAny` now accept `impl IntoPy>` rather than just `&str` to allow use of the `intern!` macro. [#2312](https://github.com/PyO3/pyo3/pull/2312) + ## [0.16.4] - 2022-04-14 ### Added From fe49a81faec88fb502f1556006568d94854663fe Mon Sep 17 00:00:00 2001 From: mejrs Date: Fri, 22 Apr 2022 00:59:53 +0200 Subject: [PATCH 3/7] Unify name bounds --- src/types/any.rs | 65 ++++++++++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 24 deletions(-) diff --git a/src/types/any.rs b/src/types/any.rs index 2d4cd81d353..2f66eae15ca 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -104,11 +104,16 @@ impl PyAny { /// to intern `attr_name`. pub fn hasattr(&self, attr_name: N) -> PyResult where - N: ToPyObject, + N: IntoPy>, { - attr_name.with_borrowed_ptr(self.py(), |attr_name| unsafe { - Ok(ffi::PyObject_HasAttr(self.as_ptr(), attr_name) != 0) - }) + let py = self.py(); + let attr_name = attr_name.into_py(py).into_ptr(); + + unsafe { + let ret = ffi::PyObject_HasAttr(self.as_ptr(), attr_name) != 0; + ffi::Py_DECREF(attr_name); + Ok(ret) + } } /// Retrieves an attribute value. @@ -135,12 +140,16 @@ impl PyAny { /// ``` pub fn getattr(&self, attr_name: N) -> PyResult<&PyAny> where - N: ToPyObject, + N: IntoPy>, { - 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)) - }) + let py = self.py(); + let attr_name = attr_name.into_py(py).into_ptr(); + + unsafe { + let ret = ffi::PyObject_GetAttr(self.as_ptr(), attr_name); + ffi::Py_DECREF(attr_name); + py.from_owned_ptr_or_err(ret) + } } /// Sets an attribute value. @@ -167,17 +176,20 @@ impl PyAny { /// ``` pub fn setattr(&self, attr_name: N, value: V) -> PyResult<()> where - N: ToBorrowedObject, - V: ToBorrowedObject, + N: IntoPy>, + 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(); + + let attr_name = attr_name.into_py(py).into_ptr(); + let value = value.to_object(py).into_ptr(); + + unsafe { + let ret = ffi::PyObject_SetAttr(self.as_ptr(), attr_name, value); + ffi::Py_DECREF(attr_name); + ffi::Py_XDECREF(value); + err::error_on_minusone(py, ret) + } } /// Deletes an attribute. @@ -188,11 +200,17 @@ impl PyAny { /// to intern `attr_name`. pub fn delattr(&self, attr_name: N) -> PyResult<()> where - N: ToPyObject, + N: IntoPy>, { - 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(); + + let attr_name = attr_name.into_py(py).into_ptr(); + + unsafe { + let ret = ffi::PyObject_DelAttr(self.as_ptr(), attr_name); + ffi::Py_DECREF(attr_name); + err::error_on_minusone(py, ret) + } } /// Returns an [`Ordering`] between `self` and `other`. @@ -560,7 +578,6 @@ impl PyAny { A: IntoPy>, { let py = self.py(); - let name: Py = name.into_py(py); let args: Py = args.into_py(py); let ptr = self.getattr(name)?.into_ptr(); From c7319e16b47fc5bd82b189c11bf12151c04fef2d Mon Sep 17 00:00:00 2001 From: mejrs Date: Mon, 25 Apr 2022 00:59:43 +0200 Subject: [PATCH 4/7] Resolve merge conflict --- src/marker.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/marker.rs b/src/marker.rs index de6a183a723..79b5489f573 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -125,7 +125,8 @@ use crate::impl_::not_send::NotSend; use crate::types::{PyAny, PyDict, PyModule, PyString, PyType}; use crate::version::PythonVersionInfo; use crate::{ - ffi, AsPyPointer, FromPyPointer, IntoPyPointer, PyNativeType, PyObject, PyTryFrom, PyTypeInfo, + ffi, AsPyPointer, FromPyPointer, IntoPy, IntoPyPointer, Py, PyNativeType, PyObject, PyTryFrom, + PyTypeInfo, }; use std::ffi::{CStr, CString}; use std::marker::PhantomData; From b75c3e9aa67cda3243c57518ee77cff044c34060 Mon Sep 17 00:00:00 2001 From: mejrs Date: Thu, 28 Apr 2022 12:17:49 +0200 Subject: [PATCH 5/7] reduce use of py_decref --- src/instance.rs | 44 ++++++++++++++++++++--------------------- src/types/any.rs | 51 ++++++++++++++++++------------------------------ 2 files changed, 41 insertions(+), 54 deletions(-) diff --git a/src/instance.rs b/src/instance.rs index 4ee57c8894b..4a2f3a86671 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -561,12 +561,14 @@ impl Py { /// ``` pub fn getattr(&self, py: Python<'_>, attr_name: N) -> PyResult where - N: ToPyObject, + N: IntoPy>, { + let attr_name = attr_name.into_py(py); + unsafe { PyObject::from_owned_ptr_or_err( py, - ffi::PyObject_GetAttr(self.as_ptr(), attr_name.to_object(py).as_ptr()), + ffi::PyObject_GetAttr(self.as_ptr(), attr_name.as_ptr()), ) } } @@ -595,17 +597,16 @@ impl Py { /// ``` pub fn setattr(&self, py: Python<'_>, attr_name: N, value: V) -> PyResult<()> where - N: ToPyObject, - V: ToPyObject, + N: IntoPy>, + V: IntoPy>, { + let attr_name = attr_name.into_py(py); + let value = value.into_py(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(), - ), + ffi::PyObject_SetAttr(self.as_ptr(), attr_name.as_ptr(), value.as_ptr()), ) } } @@ -619,16 +620,17 @@ impl Py { args: impl IntoPy>, kwargs: Option<&PyDict>, ) -> PyResult { - let args = args.into_py(py).into_ptr(); + let args = args.into_py(py); let kwargs = kwargs.into_ptr(); - let result = unsafe { - PyObject::from_owned_ptr_or_err(py, ffi::PyObject_Call(self.as_ptr(), args, kwargs)) - }; + unsafe { - ffi::Py_XDECREF(args); + let ret = PyObject::from_owned_ptr_or_err( + py, + ffi::PyObject_Call(self.as_ptr(), args.as_ptr(), kwargs), + ); ffi::Py_XDECREF(kwargs); + ret } - result } /// Calls the object with only positional arguments. @@ -671,17 +673,15 @@ impl Py { N: IntoPy>, A: IntoPy>, { - let name: Py = name.into_py(py); + let callee = self.getattr(py, name)?; let args: Py = args.into_py(py); - - let ptr = self.getattr(py, name)?.into_ptr(); - let args = args.into_ptr(); let kwargs = kwargs.into_ptr(); unsafe { - let result = PyObject::from_owned_ptr_or_err(py, ffi::PyObject_Call(ptr, args, kwargs)); - ffi::Py_DECREF(ptr); - ffi::Py_XDECREF(args); + let result = PyObject::from_owned_ptr_or_err( + py, + ffi::PyObject_Call(callee.as_ptr(), args.as_ptr(), kwargs), + ); ffi::Py_XDECREF(kwargs); result } diff --git a/src/types/any.rs b/src/types/any.rs index 1315b049cea..c6cbf6aed61 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -105,13 +105,9 @@ impl PyAny { N: IntoPy>, { let py = self.py(); - let attr_name = attr_name.into_py(py).into_ptr(); + let attr_name = attr_name.into_py(py); - unsafe { - let ret = ffi::PyObject_HasAttr(self.as_ptr(), attr_name) != 0; - ffi::Py_DECREF(attr_name); - Ok(ret) - } + unsafe { Ok(ffi::PyObject_HasAttr(self.as_ptr(), attr_name.as_ptr()) != 0) } } /// Retrieves an attribute value. @@ -141,11 +137,10 @@ impl PyAny { N: IntoPy>, { let py = self.py(); - let attr_name = attr_name.into_py(py).into_ptr(); + let attr_name = attr_name.into_py(py); unsafe { - let ret = ffi::PyObject_GetAttr(self.as_ptr(), attr_name); - ffi::Py_DECREF(attr_name); + let ret = ffi::PyObject_GetAttr(self.as_ptr(), attr_name.as_ptr()); py.from_owned_ptr_or_err(ret) } } @@ -178,14 +173,11 @@ impl PyAny { V: ToPyObject, { let py = self.py(); - - let attr_name = attr_name.into_py(py).into_ptr(); - let value = value.to_object(py).into_ptr(); + let attr_name = attr_name.into_py(py); + let value = value.to_object(py); unsafe { - let ret = ffi::PyObject_SetAttr(self.as_ptr(), attr_name, value); - ffi::Py_DECREF(attr_name); - ffi::Py_XDECREF(value); + let ret = ffi::PyObject_SetAttr(self.as_ptr(), attr_name.as_ptr(), value.as_ptr()); err::error_on_minusone(py, ret) } } @@ -201,12 +193,10 @@ impl PyAny { N: IntoPy>, { let py = self.py(); - - let attr_name = attr_name.into_py(py).into_ptr(); + let attr_name = attr_name.into_py(py); unsafe { - let ret = ffi::PyObject_DelAttr(self.as_ptr(), attr_name); - ffi::Py_DECREF(attr_name); + let ret = ffi::PyObject_DelAttr(self.as_ptr(), attr_name.as_ptr()); err::error_on_minusone(py, ret) } } @@ -459,17 +449,17 @@ impl PyAny { args: impl IntoPy>, kwargs: Option<&PyDict>, ) -> PyResult<&PyAny> { - let args = args.into_py(self.py()).into_ptr(); + let py = self.py(); + + let args = args.into_py(py); let kwargs = kwargs.into_ptr(); - let result = unsafe { - let return_value = ffi::PyObject_Call(self.as_ptr(), args, kwargs); - self.py().from_owned_ptr_or_err(return_value) - }; + unsafe { - ffi::Py_XDECREF(args); + let return_value = ffi::PyObject_Call(self.as_ptr(), args.as_ptr(), kwargs); + let ret = py.from_owned_ptr_or_err(return_value); ffi::Py_XDECREF(kwargs); + ret } - result } /// Calls the object without arguments. @@ -577,17 +567,14 @@ impl PyAny { A: IntoPy>, { let py = self.py(); - let args: Py = args.into_py(py); - let ptr = self.getattr(name)?.into_ptr(); - let args = args.into_ptr(); + let callee = self.getattr(name)?; + let args: Py = args.into_py(py); let kwargs = kwargs.into_ptr(); unsafe { - let result_ptr = ffi::PyObject_Call(ptr, args, kwargs); + let result_ptr = ffi::PyObject_Call(callee.as_ptr(), args.as_ptr(), kwargs); let result = py.from_owned_ptr_or_err(result_ptr); - ffi::Py_DECREF(ptr); - ffi::Py_XDECREF(args); ffi::Py_XDECREF(kwargs); result } From f1a926f422d0be86b055e02c9b3ffe2227a45163 Mon Sep 17 00:00:00 2001 From: mejrs Date: Fri, 29 Apr 2022 00:57:04 +0200 Subject: [PATCH 6/7] Add some attr tests --- src/instance.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 2 deletions(-) diff --git a/src/instance.rs b/src/instance.rs index 4a2f3a86671..16d9d1a9cdb 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -1001,8 +1001,8 @@ impl PyObject { #[cfg(test)] mod tests { use super::{Py, PyObject}; - use crate::types::PyDict; - use crate::{Python, ToPyObject}; + use crate::types::{PyDict, PyString}; + use crate::{PyAny, PyResult, Python, ToPyObject}; #[test] fn test_call0() { @@ -1052,4 +1052,67 @@ mod tests { assert_eq!(p.get_refcnt(py), cnt); }); } + + #[test] + fn attr() -> PyResult<()> { + use crate::types::PyModule; + + Python::with_gil(|py| { + const CODE: &str = r#" +class A: + pass +a = A() + "#; + let module = PyModule::from_code(py, CODE, "", "")?; + let instance: Py = module.getattr("a")?.into(); + + instance.getattr(py, "foo").unwrap_err(); + + instance.setattr(py, "foo", "bar")?; + + assert!(instance + .getattr(py, "foo")? + .as_ref(py) + .eq(PyString::new(py, "bar"))?); + + instance.getattr(py, "foo")?; + Ok(()) + }) + } + + #[test] + fn pystring_attr() -> PyResult<()> { + use crate::types::PyModule; + + Python::with_gil(|py| { + const CODE: &str = r#" +class A: + pass +a = A() + "#; + let module = PyModule::from_code(py, CODE, "", "")?; + let instance: Py = module.getattr("a")?.into(); + + let foo = crate::intern!(py, "foo"); + let bar = crate::intern!(py, "bar"); + + instance.getattr(py, foo).unwrap_err(); + instance.setattr(py, foo, bar)?; + assert!(instance.getattr(py, foo)?.as_ref(py).eq(bar)?); + Ok(()) + }) + } + + #[test] + fn invalid_attr() -> PyResult<()> { + Python::with_gil(|py| { + let instance: Py = py.eval("object()", None, None)?.into(); + + instance.getattr(py, "foo").unwrap_err(); + + // Cannot assign arbitrary attributes to `object` + instance.setattr(py, "foo", "bar").unwrap_err(); + Ok(()) + }) + } } From cf076051271c78c423c3d6e1e0a82c2baecb215d Mon Sep 17 00:00:00 2001 From: mejrs Date: Sat, 30 Apr 2022 23:33:03 +0200 Subject: [PATCH 7/7] Update migration --- guide/src/migration.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/guide/src/migration.md b/guide/src/migration.md index 3aa50f7f218..bd9f53e5581 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -5,6 +5,35 @@ For a detailed list of all changes, see the [CHANGELOG](changelog.md). ## from 0.16.* to 0.17 + +### Added `impl IntoPy> for &str` + +This may cause inference errors. + +Before: +```rust,compile_fail +# use pyo3::prelude::*; +# +# fn main() { +Python::with_gil(|py| { + // Cannot infer either `Py` or `Py` + let _test = "test".into_py(py); +}); +# } +``` + +After, some type annotations may be necessary: + +```rust +# use pyo3::prelude::*; +# +# fn main() { +Python::with_gil(|py| { + let _test: Py = "test".into_py(py); +}); +# } +``` + ### The `pyproto` feature is now disabled by default In preparation for removing the deprecated `#[pyproto]` attribute macro in a future PyO3 version, it is now gated behind an opt-in feature flag. This also gives a slight saving to compile times for code which does not use the deprecated macro.