diff --git a/guide/src/class.md b/guide/src/class.md index a11cea740f3..3dcf312f7a5 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -1131,7 +1131,7 @@ impl<'a, 'py> pyo3::impl_::extract_argument::PyFunctionArgument<'a, 'py> for &'a type Holder = ::std::option::Option>; #[inline] - fn extract(obj: &'py pyo3::PyAny, holder: &'a mut Self::Holder) -> pyo3::PyResult { + fn extract(obj: pyo3::impl_::extract_argument::PyArg<'py>, holder: &'a mut Self::Holder) -> pyo3::PyResult { pyo3::impl_::extract_argument::extract_pyclass_ref(obj, holder) } } @@ -1141,7 +1141,7 @@ impl<'a, 'py> pyo3::impl_::extract_argument::PyFunctionArgument<'a, 'py> for &'a type Holder = ::std::option::Option>; #[inline] - fn extract(obj: &'py pyo3::PyAny, holder: &'a mut Self::Holder) -> pyo3::PyResult { + fn extract(obj: pyo3::impl_::extract_argument::PyArg<'py>, holder: &'a mut Self::Holder) -> pyo3::PyResult { pyo3::impl_::extract_argument::extract_pyclass_ref_mut(obj, holder) } } diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 7050be23d5c..6fd77cdd095 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -191,7 +191,7 @@ impl SelfType { }); error_mode.handle_error(quote_spanned! { *span => _pyo3::impl_::extract_argument::#method::<#cls>( - #py.from_borrowed_ptr::<_pyo3::PyAny>(#slf), + _pyo3::impl_::extract_argument::PyArg::from_gil_ref(#py.from_borrowed_ptr::<_pyo3::PyAny>(#slf)), &mut #holder, ) }) diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs index 1ef31867406..0fed72c2b81 100644 --- a/pyo3-macros-backend/src/params.rs +++ b/pyo3-macros-backend/src/params.rs @@ -44,8 +44,8 @@ pub fn impl_arg_params( .collect::>()?; return Ok(( quote! { - let _args = py.from_borrowed_ptr::<_pyo3::types::PyTuple>(_args); - let _kwargs: ::std::option::Option<&_pyo3::types::PyDict> = py.from_borrowed_ptr_or_opt(_kwargs); + let _args = _pyo3::impl_::extract_argument::PyArg::from_ptr(py, _args); + let _kwargs = _pyo3::impl_::extract_argument::PyArg::from_ptr_or_opt(py, _kwargs); }, arg_convert, )); @@ -132,7 +132,7 @@ pub fn impl_arg_params( keyword_only_parameters: &[#(#keyword_only_parameters),*], }; let mut #args_array = [::std::option::Option::None; #num_params]; - let (_args, _kwargs) = #extract_expression; + let (_args, _kwargs) = &#extract_expression; }, param_conversion, )) @@ -180,7 +180,8 @@ fn impl_arg_param( let holder = push_holder(); return Ok(quote_arg_span! { _pyo3::impl_::extract_argument::extract_argument( - _args, + #[allow(clippy::useless_conversion)] + ::std::convert::From::from(_args), &mut #holder, #name_str )? @@ -193,7 +194,8 @@ fn impl_arg_param( let holder = push_holder(); return Ok(quote_arg_span! { _pyo3::impl_::extract_argument::extract_optional_argument( - _kwargs.map(::std::convert::AsRef::as_ref), + #[allow(clippy::useless_conversion, clippy::redundant_closure)] + _kwargs.as_ref().map(|kwargs| ::std::convert::From::from(kwargs)), &mut #holder, #name_str, || ::std::option::Option::None diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index f60dbe5f4d4..e1144f1a731 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -835,7 +835,7 @@ impl<'a> PyClassImplsBuilder<'a> { type Holder = ::std::option::Option<_pyo3::PyRef<'py, #cls>>; #[inline] - fn extract(obj: &'py _pyo3::PyAny, holder: &'a mut Self::Holder) -> _pyo3::PyResult { + fn extract(obj: _pyo3::impl_::extract_argument::PyArg<'py>, holder: &'a mut Self::Holder) -> _pyo3::PyResult { _pyo3::impl_::extract_argument::extract_pyclass_ref(obj, holder) } } @@ -847,7 +847,7 @@ impl<'a> PyClassImplsBuilder<'a> { type Holder = ::std::option::Option<_pyo3::PyRef<'py, #cls>>; #[inline] - fn extract(obj: &'py _pyo3::PyAny, holder: &'a mut Self::Holder) -> _pyo3::PyResult { + fn extract(obj: _pyo3::impl_::extract_argument::PyArg<'py>, holder: &'a mut Self::Holder) -> _pyo3::PyResult { _pyo3::impl_::extract_argument::extract_pyclass_ref(obj, holder) } } @@ -857,7 +857,7 @@ impl<'a> PyClassImplsBuilder<'a> { type Holder = ::std::option::Option<_pyo3::PyRefMut<'py, #cls>>; #[inline] - fn extract(obj: &'py _pyo3::PyAny, holder: &'a mut Self::Holder) -> _pyo3::PyResult { + fn extract(obj: _pyo3::impl_::extract_argument::PyArg<'py>, holder: &'a mut Self::Holder) -> _pyo3::PyResult { _pyo3::impl_::extract_argument::extract_pyclass_ref_mut(obj, holder) } } diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 8a97c712cac..26c6ca8067e 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -996,7 +996,7 @@ fn extract_object( }); extract_error_mode.handle_error(quote! { _pyo3::impl_::extract_argument::extract_argument( - #source, + _pyo3::impl_::extract_argument::PyArg::from_gil_ref(#source), &mut #holder, #name ) diff --git a/pyo3-macros-backend/src/quotes.rs b/pyo3-macros-backend/src/quotes.rs index 239036ef3ca..b7a96aa10be 100644 --- a/pyo3-macros-backend/src/quotes.rs +++ b/pyo3-macros-backend/src/quotes.rs @@ -16,6 +16,7 @@ pub(crate) fn ok_wrap(obj: TokenStream) -> TokenStream { pub(crate) fn map_result_into_ptr(result: TokenStream) -> TokenStream { quote! { - _pyo3::impl_::wrap::map_result_into_ptr(py, #result) + let result = _pyo3::impl_::wrap::map_result_into_ptr(py, #result); + result } } diff --git a/pytests/src/pyfunctions.rs b/pytests/src/pyfunctions.rs index 1eef970430e..a92733c2898 100644 --- a/pytests/src/pyfunctions.rs +++ b/pytests/src/pyfunctions.rs @@ -4,62 +4,66 @@ use pyo3::types::{PyDict, PyTuple}; #[pyfunction(signature = ())] fn none() {} +type Any<'py> = Bound<'py, PyAny>; +type Dict<'py> = Bound<'py, PyDict>; +type Tuple<'py> = Bound<'py, PyTuple>; + #[pyfunction(signature = (a, b = None, *, c = None))] -fn simple<'a>( - a: &'a PyAny, - b: Option<&'a PyAny>, - c: Option<&'a PyAny>, -) -> (&'a PyAny, Option<&'a PyAny>, Option<&'a PyAny>) { +fn simple<'py>( + a: Any<'py>, + b: Option>, + c: Option>, +) -> (Any<'py>, Option>, Option>) { (a, b, c) } #[pyfunction(signature = (a, b = None, *args, c = None))] -fn simple_args<'a>( - a: &'a PyAny, - b: Option<&'a PyAny>, - args: &'a PyTuple, - c: Option<&'a PyAny>, -) -> (&'a PyAny, Option<&'a PyAny>, &'a PyTuple, Option<&'a PyAny>) { +fn simple_args<'py>( + a: Any<'py>, + b: Option>, + args: Tuple<'py>, + c: Option>, +) -> (Any<'py>, Option>, Tuple<'py>, Option>) { (a, b, args, c) } #[pyfunction(signature = (a, b = None, c = None, **kwargs))] -fn simple_kwargs<'a>( - a: &'a PyAny, - b: Option<&'a PyAny>, - c: Option<&'a PyAny>, - kwargs: Option<&'a PyDict>, +fn simple_kwargs<'py>( + a: Any<'py>, + b: Option>, + c: Option>, + kwargs: Option>, ) -> ( - &'a PyAny, - Option<&'a PyAny>, - Option<&'a PyAny>, - Option<&'a PyDict>, + Any<'py>, + Option>, + Option>, + Option>, ) { (a, b, c, kwargs) } #[pyfunction(signature = (a, b = None, *args, c = None, **kwargs))] -fn simple_args_kwargs<'a>( - a: &'a PyAny, - b: Option<&'a PyAny>, - args: &'a PyTuple, - c: Option<&'a PyAny>, - kwargs: Option<&'a PyDict>, +fn simple_args_kwargs<'py>( + a: Any<'py>, + b: Option>, + args: Tuple<'py>, + c: Option>, + kwargs: Option>, ) -> ( - &'a PyAny, - Option<&'a PyAny>, - &'a PyTuple, - Option<&'a PyAny>, - Option<&'a PyDict>, + Any<'py>, + Option>, + Tuple<'py>, + Option>, + Option>, ) { (a, b, args, c, kwargs) } #[pyfunction(signature = (*args, **kwargs))] -fn args_kwargs<'a>( - args: &'a PyTuple, - kwargs: Option<&'a PyDict>, -) -> (&'a PyTuple, Option<&'a PyDict>) { +fn args_kwargs<'py>( + args: Tuple<'py>, + kwargs: Option>, +) -> (Tuple<'py>, Option>) { (args, kwargs) } diff --git a/src/err/mod.rs b/src/err/mod.rs index 06ac3ea66c3..d2d233c43f3 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -809,7 +809,7 @@ impl PyErrArguments for PyDowncastErrorArguments { } /// Convert `PyDowncastError` to Python `TypeError`. -impl<'a> std::convert::From> for PyErr { +impl std::convert::From> for PyErr { fn from(err: PyDowncastError<'_>) -> PyErr { let args = PyDowncastErrorArguments { from: err.from.get_type().into(), @@ -820,9 +820,9 @@ impl<'a> std::convert::From> for PyErr { } } -impl<'a> std::error::Error for PyDowncastError<'a> {} +impl std::error::Error for PyDowncastError<'_> {} -impl<'a> std::fmt::Display for PyDowncastError<'a> { +impl std::fmt::Display for PyDowncastError<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { display_downcast_error(f, &self.from.as_borrowed(), &self.to) } diff --git a/src/impl_/extract_argument.rs b/src/impl_/extract_argument.rs index 71aea0c8446..1872e995203 100644 --- a/src/impl_/extract_argument.rs +++ b/src/impl_/extract_argument.rs @@ -2,9 +2,59 @@ use crate::{ exceptions::PyTypeError, ffi, pyclass::boolean_struct::False, - types::{PyDict, PyString, PyTuple}, - FromPyObject, PyAny, PyClass, PyErr, PyRef, PyRefMut, PyResult, Python, + types::{ + any::PyAnyMethods, dict::PyDictMethods, string::PyStringMethods, tuple::PyTupleMethods, + PyDict, PyString, PyTuple, + }, + Borrowed, Bound, FromPyObject, PyAny, PyClass, PyErr, PyNativeType, PyRef, PyRefMut, PyResult, + PyTypeCheck, Python, }; +use crate::{PyObject, ToPyObject}; + +/// Function argument extraction borrows input arguments. +/// +/// TODO maybe change this to be pub type instead of a wrapper when Borrowed is public +#[derive(Copy, Clone)] +#[repr(transparent)] +pub struct PyArg<'py>(Borrowed<'py, 'py, PyAny>); + +impl<'py> PyArg<'py> { + pub unsafe fn from_ptr_or_opt(py: Python<'py>, ptr: *mut ffi::PyObject) -> Option { + Borrowed::from_ptr_or_opt(py, ptr).map(Self) + } + + pub unsafe fn from_ptr(py: Python<'py>, ptr: *mut ffi::PyObject) -> Self { + Self(Borrowed::from_ptr(py, ptr)) + } + + pub fn from_gil_ref(instance: &'py PyAny) -> Self { + Self(instance.as_borrowed()) + } + + pub fn as_gil_ref(self) -> &'py PyAny { + self.0.into_gil_ref() + } +} + +// These two `From` implementations help make various macro code pieces compile +impl<'py, T> From<&'py Bound<'py, T>> for PyArg<'py> { + fn from(other: &'py Bound<'py, T>) -> Self { + Self(Borrowed::from(other.as_any())) + } +} + +impl<'py> From<&'_ Self> for PyArg<'py> { + fn from(other: &'_ Self) -> Self { + Self(other.0) + } +} + +impl ToPyObject for PyArg<'_> { + #[inline] + fn to_object(&self, py: Python<'_>) -> PyObject { + self.0.to_object(py) + } +} /// A trait which is used to help PyO3 macros extract function arguments. /// @@ -16,7 +66,7 @@ use crate::{ /// There exists a trivial blanket implementation for `T: FromPyObject` with `Holder = ()`. pub trait PyFunctionArgument<'a, 'py>: Sized + 'a { type Holder: FunctionArgumentHolder; - fn extract(obj: &'py PyAny, holder: &'a mut Self::Holder) -> PyResult; + fn extract(obj: PyArg<'py>, holder: &'a mut Self::Holder) -> PyResult; } impl<'a, 'py, T> PyFunctionArgument<'a, 'py> for T @@ -26,8 +76,20 @@ where type Holder = (); #[inline] - fn extract(obj: &'py PyAny, _: &'a mut ()) -> PyResult { - obj.extract() + fn extract(obj: PyArg<'py>, _: &'a mut ()) -> PyResult { + obj.0.into_gil_ref().extract() + } +} + +impl<'a, 'py, T> PyFunctionArgument<'a, 'py> for &'a Bound<'py, T> +where + T: PyTypeCheck, +{ + type Holder = Option>; + + #[inline] + fn extract(obj: PyArg<'py>, holder: &'a mut Option>) -> PyResult { + Ok(&*holder.insert(obj.0.into_gil_ref().extract()?)) } } @@ -47,24 +109,24 @@ impl FunctionArgumentHolder for Option { #[inline] pub fn extract_pyclass_ref<'a, 'py: 'a, T: PyClass>( - obj: &'py PyAny, + obj: PyArg<'py>, holder: &'a mut Option>, ) -> PyResult<&'a T> { - Ok(&*holder.insert(obj.extract()?)) + Ok(&*holder.insert(obj.0.into_gil_ref().extract()?)) } #[inline] pub fn extract_pyclass_ref_mut<'a, 'py: 'a, T: PyClass>( - obj: &'py PyAny, + obj: PyArg<'py>, holder: &'a mut Option>, ) -> PyResult<&'a mut T> { - Ok(&mut *holder.insert(obj.extract()?)) + Ok(&mut *holder.insert(obj.0.into_gil_ref().extract()?)) } /// The standard implementation of how PyO3 extracts a `#[pyfunction]` or `#[pymethod]` function argument. #[doc(hidden)] pub fn extract_argument<'a, 'py, T>( - obj: &'py PyAny, + obj: PyArg<'py>, holder: &'a mut T::Holder, arg_name: &str, ) -> PyResult @@ -73,7 +135,7 @@ where { match PyFunctionArgument::extract(obj, holder) { Ok(value) => Ok(value), - Err(e) => Err(argument_extraction_error(obj.py(), arg_name, e)), + Err(e) => Err(argument_extraction_error(obj.0.py(), arg_name, e)), } } @@ -81,7 +143,7 @@ where /// does not implement `PyFunctionArgument` for `T: PyClass`. #[doc(hidden)] pub fn extract_optional_argument<'a, 'py, T>( - obj: Option<&'py PyAny>, + obj: Option>, holder: &'a mut T::Holder, arg_name: &str, default: fn() -> Option, @@ -91,7 +153,7 @@ where { match obj { Some(obj) => { - if obj.is_none() { + if obj.0.is_none() { // Explicit `None` will result in None being used as the function argument Ok(None) } else { @@ -105,7 +167,7 @@ where /// Alternative to [`extract_argument`] used when the argument has a default value provided by an annotation. #[doc(hidden)] pub fn extract_argument_with_default<'a, 'py, T>( - obj: Option<&'py PyAny>, + obj: Option>, holder: &'a mut T::Holder, arg_name: &str, default: fn() -> T, @@ -122,20 +184,20 @@ where /// Alternative to [`extract_argument`] used when the argument has a `#[pyo3(from_py_with)]` annotation. #[doc(hidden)] pub fn from_py_with<'py, T>( - obj: &'py PyAny, + obj: PyArg<'py>, arg_name: &str, extractor: fn(&'py PyAny) -> PyResult, ) -> PyResult { - match extractor(obj) { + match extractor(obj.0.into_gil_ref()) { Ok(value) => Ok(value), - Err(e) => Err(argument_extraction_error(obj.py(), arg_name, e)), + Err(e) => Err(argument_extraction_error(obj.0.py(), arg_name, e)), } } /// Alternative to [`extract_argument`] used when the argument has a `#[pyo3(from_py_with)]` annotation and also a default value. #[doc(hidden)] pub fn from_py_with_with_default<'py, T>( - obj: Option<&'py PyAny>, + obj: Option>, arg_name: &str, extractor: fn(&'py PyAny) -> PyResult, default: fn() -> T, @@ -170,7 +232,7 @@ pub fn argument_extraction_error(py: Python<'_>, arg_name: &str, error: PyErr) - /// `argument` must not be `None` #[doc(hidden)] #[inline] -pub unsafe fn unwrap_required_argument(argument: Option<&PyAny>) -> &PyAny { +pub unsafe fn unwrap_required_argument(argument: Option>) -> PyArg<'_> { match argument { Some(value) => value, #[cfg(debug_assertions)] @@ -217,7 +279,7 @@ impl FunctionDescription { args: *const *mut ffi::PyObject, nargs: ffi::Py_ssize_t, kwnames: *mut ffi::PyObject, - output: &mut [Option<&'py PyAny>], + output: &mut [Option>], ) -> PyResult<(V::Varargs, K::Varkeywords)> where V: VarargsHandler<'py>, @@ -234,8 +296,8 @@ impl FunctionDescription { ); // Handle positional arguments - // Safety: Option<&PyAny> has the same memory layout as `*mut ffi::PyObject` - let args: *const Option<&PyAny> = args.cast(); + // Safety: Option has the same memory layout as `*mut ffi::PyObject` + let args: *const Option> = args.cast(); let positional_args_provided = nargs as usize; let remaining_positional_args = if args.is_null() { debug_assert_eq!(positional_args_provided, 0); @@ -255,13 +317,21 @@ impl FunctionDescription { // Handle keyword arguments let mut varkeywords = K::Varkeywords::default(); - if let Some(kwnames) = py.from_borrowed_ptr_or_opt::(kwnames) { + + // Safety: kwnames is known to be a pointer to a tuple, or null + let kwnames: &'py Option> = std::mem::transmute(&kwnames); + if let Some(kwnames) = kwnames.as_ref() { // Safety: &PyAny has the same memory layout as `*mut ffi::PyObject` - let kwargs = - ::std::slice::from_raw_parts((args as *const &PyAny).offset(nargs), kwnames.len()); + let kwargs = ::std::slice::from_raw_parts( + (args as *const PyArg<'py>).offset(nargs), + kwnames.len(), + ); self.handle_kwargs::( - kwnames.iter().zip(kwargs.iter().copied()), + kwnames + .iter_borrowed() + .map(PyArg) + .zip(kwargs.iter().copied()), &mut varkeywords, num_positional_parameters, output, @@ -290,17 +360,20 @@ impl FunctionDescription { /// - `kwargs` must be a pointer to a PyDict, or NULL. pub unsafe fn extract_arguments_tuple_dict<'py, V, K>( &self, - py: Python<'py>, + _py: Python<'py>, args: *mut ffi::PyObject, kwargs: *mut ffi::PyObject, - output: &mut [Option<&'py PyAny>], + output: &mut [Option>], ) -> PyResult<(V::Varargs, K::Varkeywords)> where V: VarargsHandler<'py>, K: VarkeywordsHandler<'py>, { - let args = py.from_borrowed_ptr::(args); - let kwargs: ::std::option::Option<&PyDict> = py.from_borrowed_ptr_or_opt(kwargs); + // Safety: Bound has the same layout as a raw pointer, and reference is known to be + // borrowed for 'py. + let args: &'py Bound<'py, PyTuple> = std::mem::transmute(&args); + let kwargs: &'py Option> = std::mem::transmute(&kwargs); + let kwargs = kwargs.as_ref(); let num_positional_parameters = self.positional_parameter_names.len(); @@ -312,8 +385,12 @@ impl FunctionDescription { ); // Copy positional arguments into output - for (i, arg) in args.iter().take(num_positional_parameters).enumerate() { - output[i] = Some(arg); + for (i, arg) in args + .iter_borrowed() + .take(num_positional_parameters) + .enumerate() + { + output[i] = Some(PyArg(arg)); } // If any arguments remain, push them to varargs (if possible) or error @@ -322,7 +399,12 @@ impl FunctionDescription { // Handle keyword arguments let mut varkeywords = K::Varkeywords::default(); if let Some(kwargs) = kwargs { - self.handle_kwargs::(kwargs, &mut varkeywords, num_positional_parameters, output)? + self.handle_kwargs::( + kwargs.iter_borrowed().map(|(k, v)| (PyArg(k), PyArg(v))), + &mut varkeywords, + num_positional_parameters, + output, + )? } // Once all inputs have been processed, check that all required arguments have been provided. @@ -339,11 +421,11 @@ impl FunctionDescription { kwargs: I, varkeywords: &mut K::Varkeywords, num_positional_parameters: usize, - output: &mut [Option<&'py PyAny>], + output: &mut [Option>], ) -> PyResult<()> where K: VarkeywordsHandler<'py>, - I: IntoIterator, + I: IntoIterator, PyArg<'py>)>, { debug_assert_eq!( num_positional_parameters, @@ -359,7 +441,7 @@ impl FunctionDescription { // If it isn't, then it will be handled below as a varkeyword (which may raise an // error if this function doesn't accept **kwargs). Rust source is always UTF-8 // and so all argument names in `#[pyfunction]` signature must be UTF-8. - if let Ok(kwarg_name) = kwarg_name_py.downcast::()?.to_str() { + if let Ok(kwarg_name) = kwarg_name_py.0.downcast::()?.to_str() { // Try to place parameter in keyword only parameters if let Some(i) = self.find_keyword_parameter_in_keyword_only(kwarg_name) { if output[i + num_positional_parameters] @@ -378,7 +460,7 @@ impl FunctionDescription { // kwarg to conflict with a postional-only argument - the value // will go into **kwargs anyway. if K::handle_varkeyword(varkeywords, kwarg_name_py, value, self).is_err() { - positional_only_keyword_arguments.push(kwarg_name); + positional_only_keyword_arguments.push(kwarg_name_py); } } else if output[i].replace(value).is_some() { return Err(self.multiple_values_for_argument(kwarg_name)); @@ -417,7 +499,7 @@ impl FunctionDescription { #[inline] fn ensure_no_missing_required_positional_arguments( &self, - output: &[Option<&PyAny>], + output: &[Option>], positional_args_provided: usize, ) -> PyResult<()> { if positional_args_provided < self.required_positional_parameters { @@ -433,7 +515,7 @@ impl FunctionDescription { #[inline] fn ensure_no_missing_required_keyword_arguments( &self, - output: &[Option<&PyAny>], + output: &[Option>], ) -> PyResult<()> { let keyword_output = &output[self.positional_parameter_names.len()..]; for (param, out) in self.keyword_only_parameters.iter().zip(keyword_output) { @@ -478,21 +560,26 @@ impl FunctionDescription { } #[cold] - fn unexpected_keyword_argument(&self, argument: &PyAny) -> PyErr { + fn unexpected_keyword_argument(&self, argument: PyArg<'_>) -> PyErr { PyTypeError::new_err(format!( "{} got an unexpected keyword argument '{}'", self.full_name(), - argument + argument.0.as_any() )) } #[cold] - fn positional_only_keyword_arguments(&self, parameter_names: &[&str]) -> PyErr { + fn positional_only_keyword_arguments(&self, parameter_names: &[PyArg<'_>]) -> PyErr { let mut msg = format!( "{} got some positional-only arguments passed as keyword arguments: ", self.full_name() ); - push_parameter_list(&mut msg, parameter_names); + push_parameter_list( + &mut msg, + parameter_names + .iter() + .map(|a| a.0.downcast::().unwrap().to_str().unwrap()), + ); PyTypeError::new_err(msg) } @@ -510,12 +597,12 @@ impl FunctionDescription { argument_type, arguments, ); - push_parameter_list(&mut msg, parameter_names); + push_parameter_list(&mut msg, parameter_names.iter().copied()); PyTypeError::new_err(msg) } #[cold] - fn missing_required_keyword_arguments(&self, keyword_outputs: &[Option<&PyAny>]) -> PyErr { + fn missing_required_keyword_arguments(&self, keyword_outputs: &[Option>]) -> PyErr { debug_assert_eq!(self.keyword_only_parameters.len(), keyword_outputs.len()); let missing_keyword_only_arguments: Vec<_> = self @@ -536,7 +623,7 @@ impl FunctionDescription { } #[cold] - fn missing_required_positional_arguments(&self, output: &[Option<&PyAny>]) -> PyErr { + fn missing_required_positional_arguments(&self, output: &[Option>]) -> PyErr { let missing_positional_arguments: Vec<_> = self .positional_parameter_names .iter() @@ -556,14 +643,14 @@ pub trait VarargsHandler<'py> { /// Called by `FunctionDescription::extract_arguments_fastcall` with any additional arguments. fn handle_varargs_fastcall( py: Python<'py>, - varargs: &[Option<&PyAny>], + varargs: &[Option>], function_description: &FunctionDescription, ) -> PyResult; /// Called by `FunctionDescription::extract_arguments_tuple_dict` with the original tuple. /// /// Additional arguments are those in the tuple slice starting from `function_description.positional_parameter_names.len()`. fn handle_varargs_tuple( - args: &'py PyTuple, + args: &Bound<'py, PyTuple>, function_description: &FunctionDescription, ) -> PyResult; } @@ -577,7 +664,7 @@ impl<'py> VarargsHandler<'py> for NoVarargs { #[inline] fn handle_varargs_fastcall( _py: Python<'py>, - varargs: &[Option<&PyAny>], + varargs: &[Option>], function_description: &FunctionDescription, ) -> PyResult { let extra_arguments = varargs.len(); @@ -591,7 +678,7 @@ impl<'py> VarargsHandler<'py> for NoVarargs { #[inline] fn handle_varargs_tuple( - args: &'py PyTuple, + args: &Bound<'py, PyTuple>, function_description: &FunctionDescription, ) -> PyResult { let positional_parameter_count = function_description.positional_parameter_names.len(); @@ -608,19 +695,19 @@ impl<'py> VarargsHandler<'py> for NoVarargs { pub struct TupleVarargs; impl<'py> VarargsHandler<'py> for TupleVarargs { - type Varargs = &'py PyTuple; + type Varargs = Bound<'py, PyTuple>; #[inline] fn handle_varargs_fastcall( py: Python<'py>, - varargs: &[Option<&PyAny>], + varargs: &[Option>], _function_description: &FunctionDescription, ) -> PyResult { - Ok(PyTuple::new_bound(py, varargs).into_gil_ref()) + Ok(PyTuple::new_bound(py, varargs)) } #[inline] fn handle_varargs_tuple( - args: &'py PyTuple, + args: &Bound<'py, PyTuple>, function_description: &FunctionDescription, ) -> PyResult { let positional_parameters = function_description.positional_parameter_names.len(); @@ -633,8 +720,8 @@ pub trait VarkeywordsHandler<'py> { type Varkeywords: Default; fn handle_varkeyword( varkeywords: &mut Self::Varkeywords, - name: &'py PyAny, - value: &'py PyAny, + name: PyArg<'py>, + value: PyArg<'py>, function_description: &FunctionDescription, ) -> PyResult<()>; } @@ -647,8 +734,8 @@ impl<'py> VarkeywordsHandler<'py> for NoVarkeywords { #[inline] fn handle_varkeyword( _varkeywords: &mut Self::Varkeywords, - name: &'py PyAny, - _value: &'py PyAny, + name: PyArg<'py>, + _value: PyArg<'py>, function_description: &FunctionDescription, ) -> PyResult<()> { Err(function_description.unexpected_keyword_argument(name)) @@ -659,28 +746,32 @@ impl<'py> VarkeywordsHandler<'py> for NoVarkeywords { pub struct DictVarkeywords; impl<'py> VarkeywordsHandler<'py> for DictVarkeywords { - type Varkeywords = Option<&'py PyDict>; + type Varkeywords = Option>; #[inline] fn handle_varkeyword( varkeywords: &mut Self::Varkeywords, - name: &'py PyAny, - value: &'py PyAny, + name: PyArg<'py>, + value: PyArg<'py>, _function_description: &FunctionDescription, ) -> PyResult<()> { varkeywords - .get_or_insert_with(|| PyDict::new_bound(name.py()).into_gil_ref()) + .get_or_insert_with(|| PyDict::new_bound(name.0.py())) .set_item(name, value) } } -fn push_parameter_list(msg: &mut String, parameter_names: &[&str]) { - for (i, parameter) in parameter_names.iter().enumerate() { +fn push_parameter_list<'a>( + msg: &mut String, + parameter_names: impl ExactSizeIterator, +) { + let len = parameter_names.len(); + for (i, parameter) in parameter_names.enumerate() { if i != 0 { - if parameter_names.len() > 2 { + if len > 2 { msg.push(','); } - if i == parameter_names.len() - 1 { + if i == len - 1 { msg.push_str(" and ") } else { msg.push(' ') @@ -797,35 +888,35 @@ mod tests { #[test] fn push_parameter_list_empty() { let mut s = String::new(); - push_parameter_list(&mut s, &[]); + push_parameter_list(&mut s, [].into_iter()); assert_eq!(&s, ""); } #[test] fn push_parameter_list_one() { let mut s = String::new(); - push_parameter_list(&mut s, &["a"]); + push_parameter_list(&mut s, ["a"].into_iter()); assert_eq!(&s, "'a'"); } #[test] fn push_parameter_list_two() { let mut s = String::new(); - push_parameter_list(&mut s, &["a", "b"]); + push_parameter_list(&mut s, ["a", "b"].into_iter()); assert_eq!(&s, "'a' and 'b'"); } #[test] fn push_parameter_list_three() { let mut s = String::new(); - push_parameter_list(&mut s, &["a", "b", "c"]); + push_parameter_list(&mut s, ["a", "b", "c"].into_iter()); assert_eq!(&s, "'a', 'b', and 'c'"); } #[test] fn push_parameter_list_four() { let mut s = String::new(); - push_parameter_list(&mut s, &["a", "b", "c", "d"]); + push_parameter_list(&mut s, ["a", "b", "c", "d"].into_iter()); assert_eq!(&s, "'a', 'b', 'c', and 'd'"); } } diff --git a/src/instance.rs b/src/instance.rs index 8c3ac59ed3f..bc4717d4356 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -309,7 +309,8 @@ impl<'a, 'py> Borrowed<'a, 'py, PyAny> { /// This is similar to `std::slice::from_raw_parts`, the lifetime `'a` is completely defined by /// the caller and it's the caller's responsibility to ensure that the reference this is /// derived from is valid for the lifetime `'a`. - pub(crate) unsafe fn from_ptr_unchecked(py: Python<'py>, ptr: *mut ffi::PyObject) -> Self { + #[doc(hidden)] // Used in macro code. + pub unsafe fn from_ptr_unchecked(py: Python<'py>, ptr: *mut ffi::PyObject) -> Self { Self(NonNull::new_unchecked(ptr), PhantomData, py) } } @@ -1305,6 +1306,22 @@ impl IntoPy for &Bound<'_, T> { } } +impl ToPyObject for Borrowed<'_, '_, T> { + /// Converts `Py` instance -> PyObject. + fn to_object(&self, py: Python<'_>) -> PyObject { + unsafe { PyObject::from_borrowed_ptr(py, self.as_ptr()) } + } +} + +impl IntoPy for Borrowed<'_, '_, T> { + /// Converts a `Py` instance to `PyObject`. + /// Consumes `self` without calling `Py_DECREF()`. + #[inline] + fn into_py(self, py: Python<'_>) -> PyObject { + self.to_object(py) + } +} + unsafe impl crate::AsPyPointer for Py { /// Gets the underlying FFI pointer, returns a borrowed pointer. #[inline] @@ -1422,7 +1439,7 @@ where impl<'a, T> FromPyObject<'a> for Bound<'a, T> where - T: PyTypeInfo, + T: PyTypeCheck, { /// Extracts `Self` from the source `PyObject`. fn extract(ob: &'a PyAny) -> PyResult { diff --git a/src/types/dict.rs b/src/types/dict.rs index c6faea70dce..f2749d73865 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -524,6 +524,17 @@ impl<'py> PyDictMethods<'py> for Bound<'py, PyDict> { } } +impl<'py> Bound<'py, PyDict> { + /// Iterates over the contents of this dictionary without incrementing reference counts. + ///`` + /// # Safety + /// The corresponding key in the dictionary cannot be mutated for the duration + /// of the borrow. + pub(crate) unsafe fn iter_borrowed<'a>(&'a self) -> BorrowedDictIter<'a, 'py> { + BorrowedDictIter::new(self) + } +} + fn dict_len(dict: &Bound<'_, PyDict>) -> Py_ssize_t { #[cfg(any(not(Py_3_8), PyPy, Py_LIMITED_API))] unsafe { @@ -662,6 +673,104 @@ impl<'py> IntoIterator for Bound<'py, PyDict> { } } +mod borrowed_iter { + use super::*; + + /// Variant of the above which is used to iterate the items of the dictionary + /// without incrementing reference counts. This is only safe if it's known + /// that the dictionary will not be modified during iteration. + pub struct BorrowedDictIter<'a, 'py> { + dict: &'a Bound<'py, PyDict>, + ppos: ffi::Py_ssize_t, + di_used: ffi::Py_ssize_t, + len: ffi::Py_ssize_t, + } + + impl<'a, 'py> Iterator for BorrowedDictIter<'a, 'py> { + type Item = (Borrowed<'a, 'py, PyAny>, Borrowed<'a, 'py, PyAny>); + + #[inline] + fn next(&mut self) -> Option { + let ma_used = dict_len(self.dict); + + // These checks are similar to what CPython does. + // + // If the dimension of the dict changes e.g. key-value pairs are removed + // or added during iteration, this will panic next time when `next` is called + if self.di_used != ma_used { + self.di_used = -1; + panic!("dictionary changed size during iteration"); + }; + + // If the dict is changed in such a way that the length remains constant + // then this will panic at the end of iteration - similar to this: + // + // d = {"a":1, "b":2, "c": 3} + // + // for k, v in d.items(): + // d[f"{k}_"] = 4 + // del d[k] + // print(k) + // + if self.len == -1 { + self.di_used = -1; + panic!("dictionary keys changed during iteration"); + }; + + let ret = unsafe { self.next_unchecked() }; + if ret.is_some() { + self.len -= 1 + } + ret + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + let len = self.len(); + (len, Some(len)) + } + } + + impl ExactSizeIterator for BorrowedDictIter<'_, '_> { + fn len(&self) -> usize { + self.len as usize + } + } + + impl<'a, 'py> BorrowedDictIter<'a, 'py> { + pub(super) fn new(dict: &'a Bound<'py, PyDict>) -> Self { + let len = dict_len(dict); + BorrowedDictIter { + dict, + ppos: 0, + di_used: len, + len, + } + } + + /// Advances the iterator without checking for concurrent modification. + /// + /// See [`PyDict_Next`](https://docs.python.org/3/c-api/dict.html#c.PyDict_Next) + /// for more information. + unsafe fn next_unchecked( + &mut self, + ) -> Option<(Borrowed<'a, 'py, PyAny>, Borrowed<'a, 'py, PyAny>)> { + let mut key: *mut ffi::PyObject = std::ptr::null_mut(); + let mut value: *mut ffi::PyObject = std::ptr::null_mut(); + + if ffi::PyDict_Next(self.dict.as_ptr(), &mut self.ppos, &mut key, &mut value) != 0 { + let py = self.dict.py(); + // PyDict_Next returns borrowed values; for safety must make them owned (see #890) + Some((key.assume_borrowed(py), value.assume_borrowed(py))) + } else { + None + } + } + } +} + +pub(crate) use borrowed_iter::BorrowedDictIter; + /// Conversion trait that allows a sequence of tuples to be converted into `PyDict` /// Primary use case for this trait is `call` and `call_method` methods as keywords argument. pub trait IntoPyDict { diff --git a/tests/ui/invalid_argument_attributes.stderr b/tests/ui/invalid_argument_attributes.stderr index ef8b59a8691..c122dd25f8c 100644 --- a/tests/ui/invalid_argument_attributes.stderr +++ b/tests/ui/invalid_argument_attributes.stderr @@ -93,6 +93,7 @@ error[E0277]: the trait bound `CancelHandle: Clone` is not satisfied | ^^^^ the trait `Clone` is not implemented for `CancelHandle` | = help: the following other types implement trait `PyFunctionArgument<'a, 'py>`: + &'a pyo3::Bound<'py, T> &'a pyo3::coroutine::Coroutine &'a mut pyo3::coroutine::Coroutine = note: required for `CancelHandle` to implement `FromPyObject<'_>` diff --git a/tests/ui/invalid_result_conversion.stderr b/tests/ui/invalid_result_conversion.stderr index b3e65517e36..d2417a86336 100644 --- a/tests/ui/invalid_result_conversion.stderr +++ b/tests/ui/invalid_result_conversion.stderr @@ -8,7 +8,7 @@ error[E0277]: the trait bound `PyErr: From` is not satisfied > > > - >> + >> >> >> >