From 6f75fc8eb792dba0c09724afdb98f30c015f810d Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Sat, 28 Aug 2021 16:40:30 +0200 Subject: [PATCH 1/2] more macro hygiene cleanup: test #[pymethods] and more arg parsing and protos --- pyo3-macros-backend/src/defs.rs | 20 +++++----- pyo3-macros-backend/src/deprecations.rs | 2 +- pyo3-macros-backend/src/method.rs | 7 ++-- pyo3-macros-backend/src/params.rs | 24 ++++++++---- pyo3-macros-backend/src/proto_method.rs | 2 +- pyo3-macros-backend/src/pyimpl.rs | 26 ++++++------- pyo3-macros-backend/src/pymethod.rs | 12 +++--- pyo3-macros-backend/src/pyproto.rs | 10 ++--- tests/test_proc_macro_hygiene.rs | 50 +++++++++++++++++++++++++ 9 files changed, 106 insertions(+), 47 deletions(-) diff --git a/pyo3-macros-backend/src/defs.rs b/pyo3-macros-backend/src/defs.rs index d64cd7bd..0c92906d 100644 --- a/pyo3-macros-backend/src/defs.rs +++ b/pyo3-macros-backend/src/defs.rs @@ -143,7 +143,7 @@ impl SlotDef { pub const OBJECT: Proto = Proto { name: "Object", - module: "pyo3::class::basic", + module: "::pyo3::class::basic", methods: &[ MethodProto::new("__getattr__", "PyObjectGetAttrProtocol") .args(&["Name"]) @@ -189,7 +189,7 @@ pub const OBJECT: Proto = Proto { pub const ASYNC: Proto = Proto { name: "Async", - module: "pyo3::class::pyasync", + module: "::pyo3::class::pyasync", methods: &[ MethodProto::new("__await__", "PyAsyncAwaitProtocol").args(&["Receiver"]), MethodProto::new("__aiter__", "PyAsyncAiterProtocol").args(&["Receiver"]), @@ -212,7 +212,7 @@ pub const ASYNC: Proto = Proto { pub const BUFFER: Proto = Proto { name: "Buffer", - module: "pyo3::class::buffer", + module: "::pyo3::class::buffer", methods: &[ MethodProto::new("bf_getbuffer", "PyBufferGetBufferProtocol").has_self(), MethodProto::new("bf_releasebuffer", "PyBufferReleaseBufferProtocol").has_self(), @@ -230,7 +230,7 @@ pub const BUFFER: Proto = Proto { pub const CONTEXT: Proto = Proto { name: "Context", - module: "pyo3::class::context", + module: "::pyo3::class::context", methods: &[ MethodProto::new("__enter__", "PyContextEnterProtocol").has_self(), MethodProto::new("__exit__", "PyContextExitProtocol") @@ -246,7 +246,7 @@ pub const CONTEXT: Proto = Proto { pub const GC: Proto = Proto { name: "GC", - module: "pyo3::class::gc", + module: "::pyo3::class::gc", methods: &[ MethodProto::new("__traverse__", "PyGCTraverseProtocol") .has_self() @@ -264,7 +264,7 @@ pub const GC: Proto = Proto { pub const DESCR: Proto = Proto { name: "Descr", - module: "pyo3::class::descr", + module: "::pyo3::class::descr", methods: &[ MethodProto::new("__get__", "PyDescrGetProtocol").args(&["Receiver", "Inst", "Owner"]), MethodProto::new("__set__", "PyDescrSetProtocol").args(&["Receiver", "Inst", "Value"]), @@ -287,7 +287,7 @@ pub const DESCR: Proto = Proto { pub const ITER: Proto = Proto { name: "Iter", - module: "pyo3::class::iter", + module: "::pyo3::class::iter", py_methods: &[], methods: &[ MethodProto::new("__iter__", "PyIterIterProtocol").args(&["Receiver"]), @@ -301,7 +301,7 @@ pub const ITER: Proto = Proto { pub const MAPPING: Proto = Proto { name: "Mapping", - module: "pyo3::class::mapping", + module: "::pyo3::class::mapping", methods: &[ MethodProto::new("__len__", "PyMappingLenProtocol").has_self(), MethodProto::new("__getitem__", "PyMappingGetItemProtocol") @@ -334,7 +334,7 @@ pub const MAPPING: Proto = Proto { pub const SEQ: Proto = Proto { name: "Sequence", - module: "pyo3::class::sequence", + module: "::pyo3::class::sequence", methods: &[ MethodProto::new("__len__", "PySequenceLenProtocol").has_self(), MethodProto::new("__getitem__", "PySequenceGetItemProtocol") @@ -391,7 +391,7 @@ pub const SEQ: Proto = Proto { pub const NUM: Proto = Proto { name: "Number", - module: "pyo3::class::number", + module: "::pyo3::class::number", methods: &[ MethodProto::new("__add__", "PyNumberAddProtocol").args(&["Left", "Right"]), MethodProto::new("__sub__", "PyNumberSubProtocol").args(&["Left", "Right"]), diff --git a/pyo3-macros-backend/src/deprecations.rs b/pyo3-macros-backend/src/deprecations.rs index 7acf769c..0c4e28b9 100644 --- a/pyo3-macros-backend/src/deprecations.rs +++ b/pyo3-macros-backend/src/deprecations.rs @@ -39,7 +39,7 @@ impl ToTokens for Deprecations { let ident = deprecation.ident(*span); quote_spanned!( *span => - let _ = pyo3::impl_::deprecations::#ident; + let _ = ::pyo3::impl_::deprecations::#ident; ) .to_tokens(tokens) } diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 7e27e92e..51582015 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -484,11 +484,10 @@ impl<'a> FnSpec<'a> { #deprecations ::pyo3::callback::handle_panic(|#py| { #self_conversion - use ::std::option::Option; - let _kwnames: Option<&::pyo3::types::PyTuple> = #py.from_borrowed_ptr_or_opt(_kwnames); + let _kwnames: ::std::option::Option<&::pyo3::types::PyTuple> = #py.from_borrowed_ptr_or_opt(_kwnames); // Safety: &PyAny has the same memory layout as `*mut ffi::PyObject` let _args = _args as *const &::pyo3::PyAny; - let _kwargs = if let Option::Some(kwnames) = _kwnames { + let _kwargs = if let ::std::option::Option::Some(kwnames) = _kwnames { ::std::slice::from_raw_parts(_args.offset(_nargs), kwnames.len()) } else { &[] @@ -537,7 +536,7 @@ impl<'a> FnSpec<'a> { let result = #arg_convert_and_rust_call; let initializer: ::pyo3::PyClassInitializer::<#cls> = result.convert(#py)?; let cell = initializer.create_cell_from_subtype(#py, subtype)?; - Ok(cell as *mut ::pyo3::ffi::PyObject) + ::std::result::Result::Ok(cell as *mut ::pyo3::ffi::PyObject) }) } } diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs index 0d64f183..45259b77 100644 --- a/pyo3-macros-backend/src/params.rs +++ b/pyo3-macros-backend/src/params.rs @@ -91,7 +91,7 @@ pub fn impl_arg_params( if kwonly { keyword_only_parameters.push(quote! { - pyo3::derive_utils::KeywordOnlyParameterDescription { + ::pyo3::derive_utils::KeywordOnlyParameterDescription { name: #name, required: #required, } @@ -123,7 +123,7 @@ pub fn impl_arg_params( let (accept_args, accept_kwargs) = accept_args_kwargs(&spec.attrs); let cls_name = if let Some(cls) = self_ { - quote! { ::std::option::Option::Some(<#cls as pyo3::type_object::PyTypeInfo>::NAME) } + quote! { ::std::option::Option::Some(<#cls as ::pyo3::type_object::PyTypeInfo>::NAME) } } else { quote! { ::std::option::Option::None } }; @@ -236,12 +236,22 @@ fn impl_arg_param( let arg_value_or_default = match (spec.default_value(name), arg.optional.is_some()) { (Some(default), true) if default.to_string() != "None" => { - quote_arg_span! { #arg_value.map_or_else(|| Ok(Some(#default)), |_obj| #extract)? } + quote_arg_span! { + #arg_value.map_or_else(|| ::std::result::Result::Ok(::std::option::Option::Some(#default)), + |_obj| #extract)? + } } (Some(default), _) => { - quote_arg_span! { #arg_value.map_or_else(|| Ok(#default), |_obj| #extract)? } + quote_arg_span! { + #arg_value.map_or_else(|| ::std::result::Result::Ok(#default), |_obj| #extract)? + } + } + (None, true) => { + quote_arg_span! { + #arg_value.map_or(::std::result::Result::Ok(::std::option::Option::None), + |_obj| #extract)? + } } - (None, true) => quote_arg_span! { #arg_value.map_or(Ok(None), |_obj| #extract)? }, (None, false) => { quote_arg_span! { { @@ -257,7 +267,7 @@ fn impl_arg_param( let (target_ty, borrow_tmp) = if arg.optional.is_some() { // Get Option<&T> from Option> ( - quote_arg_span! { Option<<#tref as pyo3::derive_utils::ExtractExt<'_>>::Target> }, + quote_arg_span! { ::std::option::Option<<#tref as ::pyo3::derive_utils::ExtractExt<'_>>::Target> }, if mut_.is_some() { quote_arg_span! { _tmp.as_deref_mut() } } else { @@ -267,7 +277,7 @@ fn impl_arg_param( } else { // Get &T from PyRef ( - quote_arg_span! { <#tref as pyo3::derive_utils::ExtractExt<'_>>::Target }, + quote_arg_span! { <#tref as ::pyo3::derive_utils::ExtractExt<'_>>::Target }, quote_arg_span! { &#mut_ *_tmp }, ) }; diff --git a/pyo3-macros-backend/src/proto_method.rs b/pyo3-macros-backend/src/proto_method.rs index 8d8569b0..3c14843f 100644 --- a/pyo3-macros-backend/src/proto_method.rs +++ b/pyo3-macros-backend/src/proto_method.rs @@ -95,7 +95,7 @@ pub(crate) fn impl_method_proto( }; Ok(quote! { - impl<'p> ::#module::#proto<'p> for #cls { + impl<'p> #module::#proto<'p> for #cls { #(#impl_types)* #res_type_def } diff --git a/pyo3-macros-backend/src/pyimpl.rs b/pyo3-macros-backend/src/pyimpl.rs index 6b5c7b7f..61d20267 100644 --- a/pyo3-macros-backend/src/pyimpl.rs +++ b/pyo3-macros-backend/src/pyimpl.rs @@ -94,13 +94,13 @@ fn gen_py_const(cls: &syn::Type, spec: &ConstSpec) -> TokenStream { let deprecations = &spec.attributes.deprecations; let python_name = &spec.null_terminated_python_name(); quote! { - pyo3::class::PyMethodDefType::ClassAttribute({ - pyo3::class::PyClassAttributeDef::new( + ::pyo3::class::PyMethodDefType::ClassAttribute({ + ::pyo3::class::PyClassAttributeDef::new( #python_name, - pyo3::class::methods::PyClassAttributeFactory({ - fn __wrap(py: pyo3::Python<'_>) -> pyo3::PyObject { + ::pyo3::class::methods::PyClassAttributeFactory({ + fn __wrap(py: ::pyo3::Python<'_>) -> ::pyo3::PyObject { #deprecations - pyo3::IntoPy::into_py(#cls::#member, py) + ::pyo3::IntoPy::into_py(#cls::#member, py) } __wrap }) @@ -111,11 +111,11 @@ fn gen_py_const(cls: &syn::Type, spec: &ConstSpec) -> TokenStream { fn impl_py_methods(ty: &syn::Type, methods: Vec) -> TokenStream { quote! { - impl pyo3::class::impl_::PyMethods<#ty> - for pyo3::class::impl_::PyClassImplCollector<#ty> + impl ::pyo3::class::impl_::PyMethods<#ty> + for ::pyo3::class::impl_::PyClassImplCollector<#ty> { - fn py_methods(self) -> &'static [pyo3::class::methods::PyMethodDefType] { - static METHODS: &[pyo3::class::methods::PyMethodDefType] = &[#(#methods),*]; + fn py_methods(self) -> &'static [::pyo3::class::methods::PyMethodDefType] { + static METHODS: &[::pyo3::class::methods::PyMethodDefType] = &[#(#methods),*]; METHODS } } @@ -128,10 +128,10 @@ fn submit_methods_inventory(ty: &syn::Type, methods: Vec) -> TokenS } quote! { - pyo3::inventory::submit! { - #![crate = pyo3] { - type Inventory = <#ty as pyo3::class::impl_::HasMethodsInventory>::Methods; - ::new(vec![#(#methods),*]) + ::pyo3::inventory::submit! { + #![crate = ::pyo3] { + type Inventory = <#ty as ::pyo3::class::impl_::HasMethodsInventory>::Methods; + ::new(::std::vec![#(#methods),*]) } } } diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 0991be14..f217f32c 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -36,12 +36,12 @@ pub fn gen_py_method( FnType::FnClass => GeneratedPyMethod::Method(impl_py_method_def( cls, &spec, - Some(quote!(pyo3::ffi::METH_CLASS)), + Some(quote!(::pyo3::ffi::METH_CLASS)), )?), FnType::FnStatic => GeneratedPyMethod::Method(impl_py_method_def( cls, &spec, - Some(quote!(pyo3::ffi::METH_STATIC)), + Some(quote!(::pyo3::ffi::METH_STATIC)), )?), // special prototypes FnType::FnNew => GeneratedPyMethod::New(impl_py_method_def_new(cls, &spec)?), @@ -111,8 +111,8 @@ fn impl_py_method_def_new(cls: &syn::Type, spec: &FnSpec) -> Result let wrapper = spec.get_wrapper_function(&wrapper_ident, Some(cls))?; Ok(quote! { impl ::pyo3::class::impl_::PyClassNewImpl<#cls> for ::pyo3::class::impl_::PyClassImplCollector<#cls> { - fn new_impl(self) -> Option { - Some({ + fn new_impl(self) -> ::std::option::Option<::pyo3::ffi::newfunc> { + ::std::option::Option::Some({ #wrapper #wrapper_ident }) @@ -126,8 +126,8 @@ fn impl_py_method_def_call(cls: &syn::Type, spec: &FnSpec) -> Result for ::pyo3::class::impl_::PyClassImplCollector<#cls> { - fn call_impl(self) -> Option { - Some({ + fn call_impl(self) -> ::std::option::Option<::pyo3::ffi::PyCFunctionWithKeywords> { + ::std::option::Option::Some({ #wrapper #wrapper_ident }) diff --git a/pyo3-macros-backend/src/pyproto.rs b/pyo3-macros-backend/src/pyproto.rs index a3943d6a..23e5d8a5 100644 --- a/pyo3-macros-backend/src/pyproto.rs +++ b/pyo3-macros-backend/src/pyproto.rs @@ -144,13 +144,13 @@ fn impl_proto_methods( { fn buffer_procs( self - ) -> Option<&'static ::pyo3::class::impl_::PyBufferProcs> { + ) -> ::std::option::Option<&'static ::pyo3::class::impl_::PyBufferProcs> { static PROCS: ::pyo3::class::impl_::PyBufferProcs = ::pyo3::class::impl_::PyBufferProcs { - bf_getbuffer: Some(pyo3::class::buffer::getbuffer::<#ty>), - bf_releasebuffer: Some(pyo3::class::buffer::releasebuffer::<#ty>), + bf_getbuffer: ::std::option::Option::Some(::pyo3::class::buffer::getbuffer::<#ty>), + bf_releasebuffer: ::std::option::Option::Some(::pyo3::class::buffer::releasebuffer::<#ty>), }; - Some(&PROCS) + ::std::option::Option::Some(&PROCS) } } }); @@ -164,7 +164,7 @@ fn impl_proto_methods( quote! {{ ::pyo3::ffi::PyType_Slot { slot: ::pyo3::ffi::#slot, - pfunc: ::#module::#slot_impl::<#ty> as _ + pfunc: #module::#slot_impl::<#ty> as _ } }} }) diff --git a/tests/test_proc_macro_hygiene.rs b/tests/test_proc_macro_hygiene.rs index ac54e557..355186c6 100644 --- a/tests/test_proc_macro_hygiene.rs +++ b/tests/test_proc_macro_hygiene.rs @@ -26,6 +26,43 @@ pub struct Bar { c: ::std::option::Option<::pyo3::Py>, } +#[::pyo3::proc_macro::pymethods] +impl Bar { + #[args(x = "1", "*", _z = "2")] + fn test(&self, _y: &Bar, _z: i32) {} + #[staticmethod] + fn staticmethod() {} + #[classmethod] + fn clsmethod(_: &::pyo3::types::PyType) {} + #[call] + #[args(args = "*", kwds = "**")] + fn __call__( + &self, + _args: &::pyo3::types::PyTuple, + _kwds: ::std::option::Option<&::pyo3::types::PyDict>, + ) -> ::pyo3::PyResult { + ::std::unimplemented!() + } + #[new] + fn new(a: u8) -> Self { + Bar { + a, + b: Foo, + c: ::std::option::Option::None, + } + } + #[getter] + fn get(&self) -> i32 { + 0 + } + #[setter] + fn set(&self, _v: i32) {} + #[classattr] + fn class_attr() -> i32 { + 0 + } +} + #[::pyo3::proc_macro::pyproto] impl ::pyo3::class::gc::PyGCProtocol for Bar { fn __traverse__( @@ -43,6 +80,19 @@ impl ::pyo3::class::gc::PyGCProtocol for Bar { } } +#[cfg(not(Py_LIMITED_API))] +#[::pyo3::proc_macro::pyproto] +impl ::pyo3::class::PyBufferProtocol for Bar { + fn bf_getbuffer( + _s: ::pyo3::PyRefMut, + _v: *mut ::pyo3::ffi::Py_buffer, + _f: ::std::os::raw::c_int, + ) -> ::pyo3::PyResult<()> { + ::std::unimplemented!() + } + fn bf_releasebuffer(_s: ::pyo3::PyRefMut, _v: *mut ::pyo3::ffi::Py_buffer) {} +} + #[::pyo3::proc_macro::pyfunction] fn do_something(x: i32) -> ::pyo3::PyResult { ::std::result::Result::Ok(x) From 381eb9c5018204c0297ffb42bf67be42ddc17ada Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Sun, 29 Aug 2021 08:30:05 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: mejrs <59372212+mejrs@users.noreply.github.com> --- tests/test_proc_macro_hygiene.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_proc_macro_hygiene.rs b/tests/test_proc_macro_hygiene.rs index 355186c6..7d1766e1 100644 --- a/tests/test_proc_macro_hygiene.rs +++ b/tests/test_proc_macro_hygiene.rs @@ -41,7 +41,7 @@ impl Bar { _args: &::pyo3::types::PyTuple, _kwds: ::std::option::Option<&::pyo3::types::PyDict>, ) -> ::pyo3::PyResult { - ::std::unimplemented!() + ::std::panic!("unimplemented isn't hygienic before 1.50") } #[new] fn new(a: u8) -> Self { @@ -88,7 +88,7 @@ impl ::pyo3::class::PyBufferProtocol for Bar { _v: *mut ::pyo3::ffi::Py_buffer, _f: ::std::os::raw::c_int, ) -> ::pyo3::PyResult<()> { - ::std::unimplemented!() + ::std::panic!("unimplemented isn't hygienic before 1.50") } fn bf_releasebuffer(_s: ::pyo3::PyRefMut, _v: *mut ::pyo3::ffi::Py_buffer) {} }