From 515c7beac04765f89af53c9fcdc73f2c964a6f5f Mon Sep 17 00:00:00 2001 From: kngwyu Date: Thu, 28 Mar 2019 22:55:41 +0900 Subject: [PATCH 1/6] Allow slf: Py/PyRef/PyRefMut in pymethods --- pyo3-derive-backend/src/method.rs | 43 +++++++++++++++- pyo3-derive-backend/src/pymethod.rs | 51 +++++++++++++++++-- src/instance.rs | 34 ++++++++++++- src/python.rs | 1 - tests/test_nested_iter.rs | 78 +++++++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 8 deletions(-) create mode 100644 tests/test_nested_iter.rs diff --git a/pyo3-derive-backend/src/method.rs b/pyo3-derive-backend/src/method.rs index cc46f2d49a2..bc0bca0c547 100644 --- a/pyo3-derive-backend/src/method.rs +++ b/pyo3-derive-backend/src/method.rs @@ -27,6 +27,35 @@ pub enum FnType { FnCall, FnClass, FnStatic, + PySelf(PySelfType), +} + +/// For fn(slf: &PyRef) support +#[derive(Clone, Copy, PartialEq, Debug)] +pub enum PySelfType { + Py, + PyRef, + PyRefMut, +} + +impl PySelfType { + fn from_args<'a>(args: &[FnArg<'a>]) -> Option { + let arg = args.iter().next()?; + let path = match arg.ty { + syn::Type::Path(p) => p, + _ => return None, + }; + let last_seg = match path.path.segments.last()? { + syn::punctuated::Pair::Punctuated(t, _) => t, + syn::punctuated::Pair::End(t) => t, + }; + match &*last_seg.ident.to_string() { + "Py" => Some(PySelfType::Py), + "PyRef" => Some(PySelfType::PyRef), + "PyRefMut" => Some(PySelfType::PyRefMut), + _ => None, + } + } } #[derive(Clone, PartialEq, Debug)] @@ -51,11 +80,10 @@ impl<'a> FnSpec<'a> { sig: &'a syn::MethodSig, meth_attrs: &'a mut Vec, ) -> syn::Result> { - let (fn_type, fn_attrs) = parse_attributes(meth_attrs)?; + let (mut fn_type, fn_attrs) = parse_attributes(meth_attrs)?; let mut has_self = false; let mut arguments = Vec::new(); - for input in sig.decl.inputs.iter() { match input { syn::FnArg::SelfRef(_) => { @@ -119,6 +147,17 @@ impl<'a> FnSpec<'a> { let ty = get_return_info(&sig.decl.output); + if fn_type == FnType::Fn && !has_self { + if let Some(pyslf) = PySelfType::from_args(&arguments) { + fn_type = FnType::PySelf(pyslf); + arguments.remove(0); + } else { + panic!( + "Static method needs an attribute #[staticmethod] or PyRef/PyRefMut as the 1st arg" + ); + } + } + Ok(FnSpec { tp: fn_type, attrs: fn_attrs, diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index a4688aba921..5a7b6558ad0 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -1,6 +1,6 @@ // Copyright (c) 2017-present PyO3 Project and Contributors -use crate::method::{FnArg, FnSpec, FnType}; +use crate::method::{FnArg, FnSpec, FnType, PySelfType}; use crate::utils; use proc_macro2::{Span, TokenStream}; use quote::quote; @@ -18,6 +18,12 @@ pub fn gen_py_method( match spec.tp { FnType::Fn => impl_py_method_def(name, doc, &spec, &impl_wrap(cls, name, &spec, true)), + FnType::PySelf(pyslf) => impl_py_method_def( + name, + doc, + &spec, + &impl_wrap_pyslf(cls, name, &spec, pyslf, true), + ), FnType::FnNew => impl_py_method_def_new(name, doc, &impl_wrap_new(cls, name, &spec)), FnType::FnInit => impl_py_method_def_init(name, doc, &impl_wrap_init(cls, name, &spec)), FnType::FnCall => impl_py_method_def_call(name, doc, &impl_wrap(cls, name, &spec, false)), @@ -48,7 +54,45 @@ pub fn impl_wrap( noargs: bool, ) -> TokenStream { let body = impl_call(cls, name, &spec); + let slf = quote! { + let _slf = _py.mut_from_borrowed_ptr::<#cls>(_slf); + }; + impl_wrap_common(cls, name, spec, noargs, slf, body) +} + +pub fn impl_wrap_pyslf( + cls: &syn::Type, + name: &syn::Ident, + spec: &FnSpec<'_>, + slftype: PySelfType, + noargs: bool, +) -> TokenStream { + let names = get_arg_names(spec); + let body = quote! { + #cls::#name(_slf, #(#names),*) + }; + let slf = match slftype { + PySelfType::Py => quote! { + let _slf = pyo3::Py::<#cls>::from_borrowed_ptr(_slf); + }, + PySelfType::PyRef => quote! { + let _slf = pyo3::PyRef::<#cls>::from_borrowed_ptr(_py, _slf); + }, + PySelfType::PyRefMut => quote! { + let _slf = pyo3::PyRefMut::<#cls>::from_borrowed_ptr(_py, _slf); + }, + }; + impl_wrap_common(cls, name, spec, noargs, slf, body) +} +fn impl_wrap_common( + cls: &syn::Type, + name: &syn::Ident, + spec: &FnSpec<'_>, + noargs: bool, + slf: TokenStream, + body: TokenStream, +) -> TokenStream { if spec.args.is_empty() && noargs { quote! { unsafe extern "C" fn __wrap( @@ -59,8 +103,7 @@ pub fn impl_wrap( stringify!(#cls), ".", stringify!(#name), "()"); let _pool = pyo3::GILPool::new(); let _py = pyo3::Python::assume_gil_acquired(); - let _slf = _py.mut_from_borrowed_ptr::<#cls>(_slf); - + #slf let _result = { pyo3::derive_utils::IntoPyResult::into_py_result(#body) }; @@ -82,7 +125,7 @@ pub fn impl_wrap( stringify!(#cls), ".", stringify!(#name), "()"); let _pool = pyo3::GILPool::new(); let _py = pyo3::Python::assume_gil_acquired(); - let _slf = _py.mut_from_borrowed_ptr::<#cls>(_slf); + #slf let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); diff --git a/src/instance.rs b/src/instance.rs index 90507955b25..7b0336ccd4f 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -11,7 +11,7 @@ use crate::types::PyAny; use crate::AsPyPointer; use crate::IntoPyPointer; use crate::Python; -use crate::{FromPyObject, IntoPyObject, ToPyObject}; +use crate::{FromPyObject, IntoPyObject, PyTryFrom, ToPyObject}; use std::marker::PhantomData; use std::mem; use std::ops::{Deref, DerefMut}; @@ -72,6 +72,12 @@ impl<'a, T: PyTypeInfo> PyRef<'a, T> { pub(crate) fn from_ref(r: &'a T) -> Self { PyRef(r, PhantomData) } + pub unsafe fn from_owned_ptr(py: Python<'a>, ptr: *mut ffi::PyObject) -> Self { + Self::from_ref(py.from_owned_ptr(ptr)) + } + pub unsafe fn from_borrowed_ptr(py: Python<'a>, ptr: *mut ffi::PyObject) -> Self { + Self::from_ref(py.from_borrowed_ptr(ptr)) + } } impl<'a, T> PyRef<'a, T> @@ -105,6 +111,15 @@ impl<'a, T: PyTypeInfo> Deref for PyRef<'a, T> { } } +impl<'a, T> FromPyObject<'a> for PyRef<'a, T> +where + T: PyTypeInfo, +{ + fn extract(ob: &'a PyAny) -> PyResult> { + T::try_from(ob).map(PyRef::from_ref).map_err(Into::into) + } +} + /// Mutable version of [`PyRef`](struct.PyRef.html). /// # Example /// ``` @@ -135,6 +150,12 @@ impl<'a, T: PyTypeInfo> PyRefMut<'a, T> { pub(crate) fn from_mut(t: &'a mut T) -> Self { PyRefMut(t, PhantomData) } + pub unsafe fn from_owned_ptr(py: Python<'a>, ptr: *mut ffi::PyObject) -> Self { + Self::from_mut(py.mut_from_owned_ptr(ptr)) + } + pub unsafe fn from_borrowed_ptr(py: Python<'a>, ptr: *mut ffi::PyObject) -> Self { + Self::from_mut(py.mut_from_borrowed_ptr(ptr)) + } } impl<'a, T> PyRefMut<'a, T> @@ -174,6 +195,17 @@ impl<'a, T: PyTypeInfo> DerefMut for PyRefMut<'a, T> { } } +impl<'a, T> FromPyObject<'a> for PyRefMut<'a, T> +where + T: PyTypeInfo, +{ + fn extract(ob: &'a PyAny) -> PyResult> { + T::try_from_mut(ob) + .map(PyRefMut::from_mut) + .map_err(Into::into) + } +} + /// Trait implements object reference extraction from python managed pointer. pub trait AsPyRef: Sized { /// Return reference to object. diff --git a/src/python.rs b/src/python.rs index bd57ff3f33f..176f98e5aa9 100644 --- a/src/python.rs +++ b/src/python.rs @@ -240,7 +240,6 @@ impl<'p> Python<'p> { /// Register `ffi::PyObject` pointer in release pool, /// and do unchecked downcast to specific type. - pub unsafe fn from_owned_ptr(self, ptr: *mut ffi::PyObject) -> &'p T where T: PyTypeInfo, diff --git a/tests/test_nested_iter.rs b/tests/test_nested_iter.rs new file mode 100644 index 00000000000..cceda6b0633 --- /dev/null +++ b/tests/test_nested_iter.rs @@ -0,0 +1,78 @@ +//! Rust value -> Python Iterator +//! Inspired by https://github.com/jothan/cordoba, thanks. +use pyo3; +use pyo3::prelude::*; +use pyo3::types::{PyBytes, PyString}; +use pyo3::PyIterProtocol; +use std::collections::HashMap; + +#[macro_use] +mod common; + +/// Assumes it's a file reader or so. +#[pyclass] +struct Reader { + inner: HashMap, +} + +#[pymethods] +impl Reader { + fn get_optional(&self, test: Option) -> PyResult { + Ok(test.unwrap_or(10)) + } + fn get_iter(slf: PyRef, keys: Py) -> PyResult { + Ok(Iter { + reader: slf.into(), + keys: keys, + idx: 0, + }) + } +} + +#[pyclass] +struct Iter { + reader: Py, + keys: Py, + idx: usize, +} + +#[pyproto] +impl PyIterProtocol for Iter { + fn __iter__(slf: PyRefMut) -> PyResult { + let py = unsafe { Python::assume_gil_acquired() }; + Ok(slf.to_object(py)) + } + + fn __next__(mut slf: PyRefMut) -> PyResult> { + let py = unsafe { Python::assume_gil_acquired() }; + match slf.keys.as_ref(py).as_bytes().get(slf.idx) { + Some(&b) => { + let res = slf + .reader + .as_ref(py) + .inner + .get(&b) + .map(|s| PyString::new(py, s).into()); + slf.idx += 1; + Ok(res) + } + None => Ok(None), + } + } +} + +#[test] +fn test_nested_iter() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let reader = [(1, "a"), (2, "b"), (3, "c"), (4, "d"), (5, "e")]; + let reader = Reader { + inner: reader.iter().map(|(k, v)| (*k, v.to_string())).collect(), + } + .into_object(py); + py_assert!( + py, + reader, + "list(reader.get_iter(bytes([3, 5, 2]))) == ['c', 'e', 'b']" + ); +} From b2e01066f02feee7be811467599aea76586e64c8 Mon Sep 17 00:00:00 2001 From: kngwyu Date: Fri, 29 Mar 2019 18:57:05 +0900 Subject: [PATCH 2/6] Introduce FromPyPointer for slf: PyRef/PyRefMut --- pyo3-derive-backend/src/method.rs | 40 ++--------------- pyo3-derive-backend/src/pymethod.rs | 22 +++------- src/conversion.rs | 67 ++++++++++++++++++++++++++--- src/instance.rs | 58 ++++++++++++++----------- src/lib.rs | 4 +- src/python.rs | 64 +++++---------------------- tests/test_nested_iter.rs | 2 +- 7 files changed, 121 insertions(+), 136 deletions(-) diff --git a/pyo3-derive-backend/src/method.rs b/pyo3-derive-backend/src/method.rs index bc0bca0c547..3bea10aff8d 100644 --- a/pyo3-derive-backend/src/method.rs +++ b/pyo3-derive-backend/src/method.rs @@ -27,35 +27,7 @@ pub enum FnType { FnCall, FnClass, FnStatic, - PySelf(PySelfType), -} - -/// For fn(slf: &PyRef) support -#[derive(Clone, Copy, PartialEq, Debug)] -pub enum PySelfType { - Py, - PyRef, - PyRefMut, -} - -impl PySelfType { - fn from_args<'a>(args: &[FnArg<'a>]) -> Option { - let arg = args.iter().next()?; - let path = match arg.ty { - syn::Type::Path(p) => p, - _ => return None, - }; - let last_seg = match path.path.segments.last()? { - syn::punctuated::Pair::Punctuated(t, _) => t, - syn::punctuated::Pair::End(t) => t, - }; - match &*last_seg.ident.to_string() { - "Py" => Some(PySelfType::Py), - "PyRef" => Some(PySelfType::PyRef), - "PyRefMut" => Some(PySelfType::PyRefMut), - _ => None, - } - } + PySelf(syn::Type), } #[derive(Clone, PartialEq, Debug)] @@ -148,14 +120,10 @@ impl<'a> FnSpec<'a> { let ty = get_return_info(&sig.decl.output); if fn_type == FnType::Fn && !has_self { - if let Some(pyslf) = PySelfType::from_args(&arguments) { - fn_type = FnType::PySelf(pyslf); - arguments.remove(0); - } else { - panic!( - "Static method needs an attribute #[staticmethod] or PyRef/PyRefMut as the 1st arg" - ); + if arguments.len() == 0 { + panic!("Static method needs an attribute #[staticmethod]"); } + fn_type = FnType::PySelf(arguments.remove(0).ty.to_owned()); } Ok(FnSpec { diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index 5a7b6558ad0..4cbeaf76d1c 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -1,6 +1,6 @@ // Copyright (c) 2017-present PyO3 Project and Contributors -use crate::method::{FnArg, FnSpec, FnType, PySelfType}; +use crate::method::{FnArg, FnSpec, FnType}; use crate::utils; use proc_macro2::{Span, TokenStream}; use quote::quote; @@ -18,11 +18,11 @@ pub fn gen_py_method( match spec.tp { FnType::Fn => impl_py_method_def(name, doc, &spec, &impl_wrap(cls, name, &spec, true)), - FnType::PySelf(pyslf) => impl_py_method_def( + FnType::PySelf(ref self_ty) => impl_py_method_def( name, doc, &spec, - &impl_wrap_pyslf(cls, name, &spec, pyslf, true), + &impl_wrap_pyslf(cls, name, &spec, self_ty, true), ), FnType::FnNew => impl_py_method_def_new(name, doc, &impl_wrap_new(cls, name, &spec)), FnType::FnInit => impl_py_method_def_init(name, doc, &impl_wrap_init(cls, name, &spec)), @@ -55,7 +55,7 @@ pub fn impl_wrap( ) -> TokenStream { let body = impl_call(cls, name, &spec); let slf = quote! { - let _slf = _py.mut_from_borrowed_ptr::<#cls>(_slf); + let _slf: &mut #cls = pyo3::FromPyPointer::from_borrowed_ptr(_py, _slf); }; impl_wrap_common(cls, name, spec, noargs, slf, body) } @@ -64,23 +64,15 @@ pub fn impl_wrap_pyslf( cls: &syn::Type, name: &syn::Ident, spec: &FnSpec<'_>, - slftype: PySelfType, + self_ty: &syn::Type, noargs: bool, ) -> TokenStream { let names = get_arg_names(spec); let body = quote! { #cls::#name(_slf, #(#names),*) }; - let slf = match slftype { - PySelfType::Py => quote! { - let _slf = pyo3::Py::<#cls>::from_borrowed_ptr(_slf); - }, - PySelfType::PyRef => quote! { - let _slf = pyo3::PyRef::<#cls>::from_borrowed_ptr(_py, _slf); - }, - PySelfType::PyRefMut => quote! { - let _slf = pyo3::PyRefMut::<#cls>::from_borrowed_ptr(_py, _slf); - }, + let slf = quote! { + let _slf: #self_ty = pyo3::FromPyPointer::from_borrowed_ptr(_py, _slf); }; impl_wrap_common(cls, name, spec, noargs, slf, body) } diff --git a/src/conversion.rs b/src/conversion.rs index 72b7442e518..50df2b2b835 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -1,15 +1,13 @@ // Copyright (c) 2017-present PyO3 Project and Contributors //! Conversions between various states of rust and python types and their wrappers. - -use crate::err::{PyDowncastError, PyResult}; -use crate::ffi; +use crate::err::{self, PyDowncastError, PyResult}; use crate::object::PyObject; use crate::type_object::PyTypeInfo; use crate::types::PyAny; use crate::types::PyTuple; -use crate::Py; -use crate::Python; +use crate::{ffi, gil, Py, Python}; +use std::ptr::NonNull; /// This trait represents that, **we can do zero-cost conversion from the object to FFI pointer**. /// @@ -432,6 +430,65 @@ impl FromPy<()> for Py { } } +pub unsafe trait FromPyPointer<'p>: Sized { + unsafe fn from_owned_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option; + unsafe fn from_owned_ptr_or_panic(py: Python<'p>, ptr: *mut ffi::PyObject) -> Self { + match Self::from_owned_ptr_or_opt(py, ptr) { + Some(s) => s, + None => err::panic_after_error(), + } + } + unsafe fn from_owned_ptr(py: Python<'p>, ptr: *mut ffi::PyObject) -> Self { + Self::from_owned_ptr_or_panic(py, ptr) + } + unsafe fn from_owned_ptr_or_err(py: Python<'p>, ptr: *mut ffi::PyObject) -> PyResult { + match Self::from_owned_ptr_or_opt(py, ptr) { + Some(s) => Ok(s), + None => Err(err::PyErr::fetch(py)), + } + } + unsafe fn from_borrowed_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option; + unsafe fn from_borrowed_ptr_or_panic(py: Python<'p>, ptr: *mut ffi::PyObject) -> Self { + match Self::from_borrowed_ptr_or_opt(py, ptr) { + Some(s) => s, + None => err::panic_after_error(), + } + } + unsafe fn from_borrowed_ptr(py: Python<'p>, ptr: *mut ffi::PyObject) -> Self { + Self::from_borrowed_ptr_or_panic(py, ptr) + } + unsafe fn from_borrowed_ptr_or_err(py: Python<'p>, ptr: *mut ffi::PyObject) -> PyResult { + match Self::from_borrowed_ptr_or_opt(py, ptr) { + Some(s) => Ok(s), + None => Err(err::PyErr::fetch(py)), + } + } +} + +unsafe impl<'p, T> FromPyPointer<'p> for &'p T +where + T: PyTypeInfo, +{ + unsafe fn from_owned_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option { + NonNull::new(ptr).map(|p| py.unchecked_downcast(gil::register_owned(py, p))) + } + unsafe fn from_borrowed_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option { + NonNull::new(ptr).map(|p| py.unchecked_downcast(gil::register_borrowed(py, p))) + } +} + +unsafe impl<'p, T> FromPyPointer<'p> for &'p mut T +where + T: PyTypeInfo, +{ + unsafe fn from_owned_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option { + NonNull::new(ptr).map(|p| py.unchecked_mut_downcast(gil::register_owned(py, p))) + } + unsafe fn from_borrowed_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option { + NonNull::new(ptr).map(|p| py.unchecked_mut_downcast(gil::register_borrowed(py, p))) + } +} + #[cfg(test)] mod test { use crate::types::PyList; diff --git a/src/instance.rs b/src/instance.rs index 7b0336ccd4f..2f91b4b16eb 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -8,10 +8,10 @@ use crate::objectprotocol::ObjectProtocol; use crate::type_object::PyTypeCreate; use crate::type_object::{PyTypeInfo, PyTypeObject}; use crate::types::PyAny; -use crate::AsPyPointer; -use crate::IntoPyPointer; -use crate::Python; -use crate::{FromPyObject, IntoPyObject, PyTryFrom, ToPyObject}; +use crate::{ + AsPyPointer, FromPyObject, FromPyPointer, IntoPyObject, IntoPyPointer, PyTryFrom, Python, + ToPyObject, +}; use std::marker::PhantomData; use std::mem; use std::ops::{Deref, DerefMut}; @@ -72,23 +72,16 @@ impl<'a, T: PyTypeInfo> PyRef<'a, T> { pub(crate) fn from_ref(r: &'a T) -> Self { PyRef(r, PhantomData) } - pub unsafe fn from_owned_ptr(py: Python<'a>, ptr: *mut ffi::PyObject) -> Self { - Self::from_ref(py.from_owned_ptr(ptr)) - } - pub unsafe fn from_borrowed_ptr(py: Python<'a>, ptr: *mut ffi::PyObject) -> Self { - Self::from_ref(py.from_borrowed_ptr(ptr)) - } } -impl<'a, T> PyRef<'a, T> +impl<'p, T> PyRef<'p, T> where T: PyTypeInfo + PyTypeObject + PyTypeCreate, { - pub fn new(py: Python, value: T) -> PyResult> { + pub fn new(py: Python<'p>, value: T) -> PyResult> { let obj = T::create(py)?; obj.init(value); - let ref_ = unsafe { py.from_owned_ptr(obj.into_ptr()) }; - Ok(PyRef::from_ref(ref_)) + unsafe { Self::from_owned_ptr_or_err(py, obj.into_ptr()) } } } @@ -120,6 +113,18 @@ where } } +unsafe impl<'p, T> FromPyPointer<'p> for PyRef<'p, T> +where + T: PyTypeInfo, +{ + unsafe fn from_owned_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option { + FromPyPointer::from_owned_ptr_or_opt(py, ptr).map(Self::from_ref) + } + unsafe fn from_borrowed_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option { + FromPyPointer::from_borrowed_ptr_or_opt(py, ptr).map(Self::from_ref) + } +} + /// Mutable version of [`PyRef`](struct.PyRef.html). /// # Example /// ``` @@ -150,23 +155,16 @@ impl<'a, T: PyTypeInfo> PyRefMut<'a, T> { pub(crate) fn from_mut(t: &'a mut T) -> Self { PyRefMut(t, PhantomData) } - pub unsafe fn from_owned_ptr(py: Python<'a>, ptr: *mut ffi::PyObject) -> Self { - Self::from_mut(py.mut_from_owned_ptr(ptr)) - } - pub unsafe fn from_borrowed_ptr(py: Python<'a>, ptr: *mut ffi::PyObject) -> Self { - Self::from_mut(py.mut_from_borrowed_ptr(ptr)) - } } -impl<'a, T> PyRefMut<'a, T> +impl<'p, T> PyRefMut<'p, T> where T: PyTypeInfo + PyTypeObject + PyTypeCreate, { - pub fn new(py: Python, value: T) -> PyResult> { + pub fn new(py: Python<'p>, value: T) -> PyResult> { let obj = T::create(py)?; obj.init(value); - let ref_ = unsafe { py.mut_from_owned_ptr(obj.into_ptr()) }; - Ok(PyRefMut::from_mut(ref_)) + unsafe { Self::from_owned_ptr_or_err(py, obj.into_ptr()) } } } @@ -206,6 +204,18 @@ where } } +unsafe impl<'p, T> FromPyPointer<'p> for PyRefMut<'p, T> +where + T: PyTypeInfo, +{ + unsafe fn from_owned_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option { + FromPyPointer::from_owned_ptr_or_opt(py, ptr).map(Self::from_mut) + } + unsafe fn from_borrowed_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option { + FromPyPointer::from_borrowed_ptr_or_opt(py, ptr).map(Self::from_mut) + } +} + /// Trait implements object reference extraction from python managed pointer. pub trait AsPyRef: Sized { /// Return reference to object. diff --git a/src/lib.rs b/src/lib.rs index 6be9a6cbcae..75f56bd0dd6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -119,8 +119,8 @@ pub use crate::class::*; pub use crate::conversion::{ - AsPyPointer, FromPy, FromPyObject, IntoPy, IntoPyObject, IntoPyPointer, PyTryFrom, PyTryInto, - ToBorrowedObject, ToPyObject, + AsPyPointer, FromPy, FromPyObject, FromPyPointer, IntoPy, IntoPyObject, IntoPyPointer, + PyTryFrom, PyTryInto, ToBorrowedObject, ToPyObject, }; pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyErrValue, PyResult}; pub use crate::gil::{init_once, GILGuard, GILPool}; diff --git a/src/python.rs b/src/python.rs index 176f98e5aa9..5e08092d631 100644 --- a/src/python.rs +++ b/src/python.rs @@ -10,7 +10,7 @@ use crate::object::PyObject; use crate::type_object::{PyTypeInfo, PyTypeObject}; use crate::types::{PyAny, PyDict, PyModule, PyType}; use crate::AsPyPointer; -use crate::{IntoPyPointer, PyTryFrom}; +use crate::{FromPyPointer, IntoPyPointer, PyTryFrom}; use std::ffi::CString; use std::marker::PhantomData; use std::os::raw::c_int; @@ -193,7 +193,7 @@ impl<'p> Python<'p> { } impl<'p> Python<'p> { - unsafe fn unchecked_downcast(self, ob: &PyAny) -> &'p T { + pub(crate) unsafe fn unchecked_downcast(self, ob: &PyAny) -> &'p T { if T::OFFSET == 0 { &*(ob as *const _ as *const T) } else { @@ -203,7 +203,7 @@ impl<'p> Python<'p> { } #[allow(clippy::cast_ref_to_mut)] // FIXME - unsafe fn unchecked_mut_downcast(self, ob: &PyAny) -> &'p mut T { + pub(crate) unsafe fn unchecked_mut_downcast(self, ob: &PyAny) -> &'p mut T { if T::OFFSET == 0 { &mut *(ob as *const _ as *mut T) } else { @@ -244,13 +244,7 @@ impl<'p> Python<'p> { where T: PyTypeInfo, { - match NonNull::new(ptr) { - Some(p) => { - let p = gil::register_owned(self, p); - self.unchecked_downcast(p) - } - None => crate::err::panic_after_error(), - } + FromPyPointer::from_owned_ptr(self, ptr) } /// Register `ffi::PyObject` pointer in release pool, @@ -259,13 +253,7 @@ impl<'p> Python<'p> { where T: PyTypeInfo, { - match NonNull::new(ptr) { - Some(p) => { - let p = gil::register_owned(self, p); - self.unchecked_mut_downcast(p) - } - None => crate::err::panic_after_error(), - } + FromPyPointer::from_owned_ptr(self, ptr) } /// Register owned `ffi::PyObject` pointer in release pool. @@ -275,13 +263,7 @@ impl<'p> Python<'p> { where T: PyTypeInfo, { - match NonNull::new(ptr) { - Some(p) => { - let p = gil::register_owned(self, p); - Ok(self.unchecked_downcast(p)) - } - None => Err(PyErr::fetch(self)), - } + FromPyPointer::from_owned_ptr_or_err(self, ptr) } /// Register owned `ffi::PyObject` pointer in release pool. @@ -291,10 +273,7 @@ impl<'p> Python<'p> { where T: PyTypeInfo, { - NonNull::new(ptr).map(|p| { - let p = gil::register_owned(self, p); - self.unchecked_downcast(p) - }) + FromPyPointer::from_owned_ptr_or_opt(self, ptr) } /// Register borrowed `ffi::PyObject` pointer in release pool. @@ -304,13 +283,7 @@ impl<'p> Python<'p> { where T: PyTypeInfo, { - match NonNull::new(ptr) { - Some(p) => { - let p = gil::register_borrowed(self, p); - self.unchecked_downcast(p) - } - None => crate::err::panic_after_error(), - } + FromPyPointer::from_borrowed_ptr(self, ptr) } /// Register borrowed `ffi::PyObject` pointer in release pool. @@ -320,13 +293,7 @@ impl<'p> Python<'p> { where T: PyTypeInfo, { - match NonNull::new(ptr) { - Some(p) => { - let p = gil::register_borrowed(self, p); - self.unchecked_mut_downcast(p) - } - None => crate::err::panic_after_error(), - } + FromPyPointer::from_borrowed_ptr(self, ptr) } /// Register borrowed `ffi::PyObject` pointer in release pool. @@ -336,13 +303,7 @@ impl<'p> Python<'p> { where T: PyTypeInfo, { - match NonNull::new(ptr) { - Some(p) => { - let p = gil::register_borrowed(self, p); - Ok(self.unchecked_downcast(p)) - } - None => Err(PyErr::fetch(self)), - } + FromPyPointer::from_borrowed_ptr_or_err(self, ptr) } /// Register borrowed `ffi::PyObject` pointer in release pool. @@ -352,10 +313,7 @@ impl<'p> Python<'p> { where T: PyTypeInfo, { - NonNull::new(ptr).map(|p| { - let p = gil::register_borrowed(self, p); - self.unchecked_downcast(p) - }) + FromPyPointer::from_borrowed_ptr_or_opt(self, ptr) } #[doc(hidden)] diff --git a/tests/test_nested_iter.rs b/tests/test_nested_iter.rs index cceda6b0633..1ad82c91b60 100644 --- a/tests/test_nested_iter.rs +++ b/tests/test_nested_iter.rs @@ -20,7 +20,7 @@ impl Reader { fn get_optional(&self, test: Option) -> PyResult { Ok(test.unwrap_or(10)) } - fn get_iter(slf: PyRef, keys: Py) -> PyResult { + fn get_iter(slf: PyRef, keys: Py) -> PyResult { Ok(Iter { reader: slf.into(), keys: keys, From 5a2b021fda5c91bcec8ba2a3aad495a180aaf7ed Mon Sep 17 00:00:00 2001 From: kngwyu Date: Fri, 29 Mar 2019 19:09:18 +0900 Subject: [PATCH 3/6] Remove FromPyObject from PyRef It's invalid --- src/instance.rs | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/src/instance.rs b/src/instance.rs index 2f91b4b16eb..a2cebc4bc25 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -9,8 +9,7 @@ use crate::type_object::PyTypeCreate; use crate::type_object::{PyTypeInfo, PyTypeObject}; use crate::types::PyAny; use crate::{ - AsPyPointer, FromPyObject, FromPyPointer, IntoPyObject, IntoPyPointer, PyTryFrom, Python, - ToPyObject, + AsPyPointer, FromPyObject, FromPyPointer, IntoPyObject, IntoPyPointer, Python, ToPyObject, }; use std::marker::PhantomData; use std::mem; @@ -104,15 +103,6 @@ impl<'a, T: PyTypeInfo> Deref for PyRef<'a, T> { } } -impl<'a, T> FromPyObject<'a> for PyRef<'a, T> -where - T: PyTypeInfo, -{ - fn extract(ob: &'a PyAny) -> PyResult> { - T::try_from(ob).map(PyRef::from_ref).map_err(Into::into) - } -} - unsafe impl<'p, T> FromPyPointer<'p> for PyRef<'p, T> where T: PyTypeInfo, @@ -193,17 +183,6 @@ impl<'a, T: PyTypeInfo> DerefMut for PyRefMut<'a, T> { } } -impl<'a, T> FromPyObject<'a> for PyRefMut<'a, T> -where - T: PyTypeInfo, -{ - fn extract(ob: &'a PyAny) -> PyResult> { - T::try_from_mut(ob) - .map(PyRefMut::from_mut) - .map_err(Into::into) - } -} - unsafe impl<'p, T> FromPyPointer<'p> for PyRefMut<'p, T> where T: PyTypeInfo, From 09bf9bbf4a83a91f5a87e2b2e139525c36de73d5 Mon Sep 17 00:00:00 2001 From: kngwyu Date: Wed, 3 Apr 2019 17:14:03 +0900 Subject: [PATCH 4/6] Allow slf: PyRef by replacing Self --- pyo3-derive-backend/src/method.rs | 32 ++++++++++++++++++++++++++--- pyo3-derive-backend/src/pymethod.rs | 17 +++++++-------- src/conversion.rs | 1 + tests/test_nested_iter.rs | 2 +- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/pyo3-derive-backend/src/method.rs b/pyo3-derive-backend/src/method.rs index 3bea10aff8d..82fb295673c 100644 --- a/pyo3-derive-backend/src/method.rs +++ b/pyo3-derive-backend/src/method.rs @@ -27,7 +27,7 @@ pub enum FnType { FnCall, FnClass, FnStatic, - PySelf(syn::Type), + PySelf(syn::TypePath), } #[derive(Clone, PartialEq, Debug)] @@ -121,9 +121,13 @@ impl<'a> FnSpec<'a> { if fn_type == FnType::Fn && !has_self { if arguments.len() == 0 { - panic!("Static method needs an attribute #[staticmethod]"); + panic!("Static method needs #[staticmethod] attribute"); } - fn_type = FnType::PySelf(arguments.remove(0).ty.to_owned()); + let tp = match arguments.remove(0).ty { + syn::Type::Path(p) => replace_self(p), + _ => panic!("Invalid type as self"), + }; + fn_type = FnType::PySelf(tp); } Ok(FnSpec { @@ -387,3 +391,25 @@ fn parse_attributes(attrs: &mut Vec) -> syn::Result<(FnType, Vec None => Ok((FnType::Fn, spec)), } } + +// Replace A with A<_> +fn replace_self(path: &syn::TypePath) -> syn::TypePath { + fn infer(span: proc_macro2::Span) -> syn::GenericArgument { + syn::GenericArgument::Type(syn::Type::Infer(syn::TypeInfer { + underscore_token: syn::token::Underscore { spans: [span] }, + })) + } + let mut res = path.to_owned(); + for seg in &mut res.path.segments { + if let syn::PathArguments::AngleBracketed(ref mut g) = seg.arguments { + for arg in &mut g.args { + if let syn::GenericArgument::Type(syn::Type::Path(p)) = arg { + if p.path.segments.len() == 1 && p.path.segments[0].ident == "Self" { + *arg = infer(p.path.segments[0].ident.span()); + } + } + } + } + } + res +} diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index 4cbeaf76d1c..d68cc39dec9 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -1,5 +1,4 @@ // Copyright (c) 2017-present PyO3 Project and Contributors - use crate::method::{FnArg, FnSpec, FnType}; use crate::utils; use proc_macro2::{Span, TokenStream}; @@ -54,9 +53,7 @@ pub fn impl_wrap( noargs: bool, ) -> TokenStream { let body = impl_call(cls, name, &spec); - let slf = quote! { - let _slf: &mut #cls = pyo3::FromPyPointer::from_borrowed_ptr(_py, _slf); - }; + let slf = impl_self("e! { &mut #cls }); impl_wrap_common(cls, name, spec, noargs, slf, body) } @@ -64,16 +61,14 @@ pub fn impl_wrap_pyslf( cls: &syn::Type, name: &syn::Ident, spec: &FnSpec<'_>, - self_ty: &syn::Type, + self_ty: &syn::TypePath, noargs: bool, ) -> TokenStream { let names = get_arg_names(spec); let body = quote! { #cls::#name(_slf, #(#names),*) }; - let slf = quote! { - let _slf: #self_ty = pyo3::FromPyPointer::from_borrowed_ptr(_py, _slf); - }; + let slf = impl_self(self_ty); impl_wrap_common(cls, name, spec, noargs, slf, body) } @@ -381,6 +376,12 @@ fn impl_call(_cls: &syn::Type, fname: &syn::Ident, spec: &FnSpec<'_>) -> TokenSt quote! { _slf.#fname(#(#names),*) } } +fn impl_self(self_ty: &T) -> TokenStream { + quote! { + let _slf: #self_ty = pyo3::FromPyPointer::from_borrowed_ptr(_py, _slf); + } +} + /// Converts a bool to "true" or "false" fn bool_to_ident(condition: bool) -> syn::Ident { if condition { diff --git a/src/conversion.rs b/src/conversion.rs index 50df2b2b835..21c80669bd5 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -430,6 +430,7 @@ impl FromPy<()> for Py { } } +/// Raw level conversion between `*mut ffi::PyObject` and PyO3 types. pub unsafe trait FromPyPointer<'p>: Sized { unsafe fn from_owned_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option; unsafe fn from_owned_ptr_or_panic(py: Python<'p>, ptr: *mut ffi::PyObject) -> Self { diff --git a/tests/test_nested_iter.rs b/tests/test_nested_iter.rs index 1ad82c91b60..cceda6b0633 100644 --- a/tests/test_nested_iter.rs +++ b/tests/test_nested_iter.rs @@ -20,7 +20,7 @@ impl Reader { fn get_optional(&self, test: Option) -> PyResult { Ok(test.unwrap_or(10)) } - fn get_iter(slf: PyRef, keys: Py) -> PyResult { + fn get_iter(slf: PyRef, keys: Py) -> PyResult { Ok(Iter { reader: slf.into(), keys: keys, From a7736dd51b7acc4cc173f07cd897f08a2ee34d34 Mon Sep 17 00:00:00 2001 From: kngwyu Date: Wed, 3 Apr 2019 18:01:28 +0900 Subject: [PATCH 5/6] Add test of slf: PyRefMut<_> --- tests/{test_nested_iter.rs => test_pyself.rs} | 45 ++++++++++++++++--- 1 file changed, 38 insertions(+), 7 deletions(-) rename tests/{test_nested_iter.rs => test_pyself.rs} (62%) diff --git a/tests/test_nested_iter.rs b/tests/test_pyself.rs similarity index 62% rename from tests/test_nested_iter.rs rename to tests/test_pyself.rs index cceda6b0633..ee2006817fc 100644 --- a/tests/test_nested_iter.rs +++ b/tests/test_pyself.rs @@ -1,5 +1,4 @@ -//! Rust value -> Python Iterator -//! Inspired by https://github.com/jothan/cordoba, thanks. +//! Test slf: PyRef/PyMutRef(especially, slf.into::) works use pyo3; use pyo3::prelude::*; use pyo3::types::{PyBytes, PyString}; @@ -10,20 +9,32 @@ use std::collections::HashMap; mod common; /// Assumes it's a file reader or so. +/// Inspired by https://github.com/jothan/cordoba, thanks. #[pyclass] +#[derive(Clone)] struct Reader { inner: HashMap, } #[pymethods] impl Reader { - fn get_optional(&self, test: Option) -> PyResult { - Ok(test.unwrap_or(10)) - } fn get_iter(slf: PyRef, keys: Py) -> PyResult { Ok(Iter { reader: slf.into(), - keys: keys, + keys, + idx: 0, + }) + } + fn get_iter_and_reset( + mut slf: PyRefMut, + keys: Py, + py: Python, + ) -> PyResult { + let reader = Py::new(py, slf.clone())?; + slf.inner.clear(); + Ok(Iter { + reader, + keys, idx: 0, }) } @@ -42,7 +53,6 @@ impl PyIterProtocol for Iter { let py = unsafe { Python::assume_gil_acquired() }; Ok(slf.to_object(py)) } - fn __next__(mut slf: PyRefMut) -> PyResult> { let py = unsafe { Python::assume_gil_acquired() }; match slf.keys.as_ref(py).as_bytes().get(slf.idx) { @@ -76,3 +86,24 @@ fn test_nested_iter() { "list(reader.get_iter(bytes([3, 5, 2]))) == ['c', 'e', 'b']" ); } + +#[test] +fn test_nested_iter_reset() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let reader = [(1, "a"), (2, "b"), (3, "c"), (4, "d"), (5, "e")]; + let reader = PyRef::new( + py, + Reader { + inner: reader.iter().map(|(k, v)| (*k, v.to_string())).collect(), + }, + ) + .unwrap(); + let obj = reader.into_object(py); + py_assert!( + py, + obj, + "list(obj.get_iter_and_reset(bytes([3, 5, 2]))) == ['c', 'e', 'b']" + ); + assert!(reader.inner.is_empty()); +} From c7d6c48e68fe540136d192458e0715fead35a769 Mon Sep 17 00:00:00 2001 From: kngwyu Date: Wed, 17 Apr 2019 19:04:53 +0900 Subject: [PATCH 6/6] Add changelog entry about `slf: PyRef` --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6353de98a54..a5fcea0d505 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added * Have `PyModule` generate an index of its members (`__all__` list). + * Allow `slf: PyRef` for pyclass(#419) ### Changed