Improvements to property macros

.travis.yml

@@ -20,7 +20,7 @@ matrix:
       python: "3.7"
       # Keep this synced up with build.rs and ensure that the nightly version does have clippy available
       # https://static.rust-lang.org/dist/YYYY-MM-DD/clippy-nightly-x86_64-unknown-linux-gnu.tar.gz exists
-      env: TRAVIS_RUST_VERSION=nightly-2019-07-19
+      env: TRAVIS_RUST_VERSION=nightly-2020-01-21
     - name: PyPy3.5 7.0 # Tested via anaconda PyPy (since travis's PyPy version is too old)
       python: "3.7"
       env: FEATURES="pypy" PATH="$PATH:/opt/anaconda/envs/pypy3/bin"

CHANGELOG.md

@@ -15,19 +15,22 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 * The blanket implementations for `FromPyObject` for `&T` and `&mut T` are no longer specializable. Implement `PyTryFrom` for your type to control the behavior of `FromPyObject::extract()` for your types. [#713](https://github.com/PyO3/pyo3/pull/713)
 * The implementation for `IntoPy<U> for T` where `U: FromPy<T>` is no longer specializable. Control the behavior of this via the implementation of `FromPy`. [#713](https://github.com/PyO3/pyo3/pull/713)
 * Use `parking_lot::Mutex` instead of `spin::Mutex`. [#734](https://github.com/PyO3/pyo3/pull/734)
+* Bumped minimum Rust version to `1.42.0-nightly 2020-01-21`. [#760](https://github.com/PyO3/pyo3/pull/760)
 
 ### Added
 
 * `PyClass`, `PyClassShell`, `PyObjectLayout`, `PyClassInitializer`. [#683](https://github.com/PyO3/pyo3/pull/683)
 * Implemented `IntoIterator` for `PySet` and `PyFrozenSet`. [#716](https://github.com/PyO3/pyo3/pull/716)
 * `FromPyObject` is now automatically implemented for `T: Clone` pyclasses. [#730](https://github.com/PyO3/pyo3/pull/730)
 * `#[pyo3(get)]` and `#[pyo3(set)]` will now use the Rust doc-comment from the field for the Python property. [#755](https://github.com/PyO3/pyo3/pull/755)
+* `#[setter]` functions may now take an argument of `Pyo3::Python`. [#760](https://github.com/PyO3/pyo3/pull/760)
 
 ### Fixed
 
 * Fixed unsoundness of subclassing. [#683](https://github.com/PyO3/pyo3/pull/683).
 * Clear error indicator when the exception is handled on the Rust side. [#719](https://github.com/PyO3/pyo3/pull/719)
 * Usage of raw identifiers with `#[pyo3(set)]`. [#745](https://github.com/PyO3/pyo3/pull/745)
+* Usage of `PyObject` with `#[pyo3(get)]`. [#760](https://github.com/PyO3/pyo3/pull/760)
 
 ### Removed
 

README.md

@@ -16,7 +16,7 @@ A comparison with rust-cpython can be found [in the guide](https://pyo3.rs/maste
 
 ## Usage
 
-PyO3 supports Python 3.5 and up. The minimum required Rust version is 1.37.0-nightly 2019-07-19.
+PyO3 supports Python 3.5 and up. The minimum required Rust version is 1.42.0-nightly 2020-01-21.
 
 If you have never used nightly Rust, the official guide has
 [a great section](https://doc.rust-lang.org/book/appendix-07-nightly-rust.html#rustup-and-the-role-of-rust-nightly)

build.rs

@@ -19,8 +19,8 @@ use version_check::{Channel, Date, Version};
 /// Specifies the minimum nightly version needed to compile pyo3.
 /// Keep this synced up with the travis ci config,
 /// But note that this is the rustc version which can be lower than the nightly version
-const MIN_DATE: &'static str = "2019-07-18";
-const MIN_VERSION: &'static str = "1.37.0-nightly";
+const MIN_DATE: &'static str = "2020-01-20";
+const MIN_VERSION: &'static str = "1.42.0-nightly";
 //const PYTHON_INTERPRETER: &'static str = "python3";
 
 lazy_static! {

pyo3-derive-backend/src/pyclass.rs

@@ -1,7 +1,9 @@
 // Copyright (c) 2017-present PyO3 Project and Contributors
 
-use crate::method::{FnArg, FnSpec, FnType};
-use crate::pymethod::{impl_py_getter_def, impl_py_setter_def, impl_wrap_getter, impl_wrap_setter};
+use crate::method::FnType;
+use crate::pymethod::{
+    impl_py_getter_def, impl_py_setter_def, impl_wrap_getter, impl_wrap_setter, PropertyType,
+};
 use crate::utils;
 use proc_macro2::{Span, TokenStream};
 use quote::quote;
@@ -421,90 +423,26 @@ fn impl_descriptors(
     cls: &syn::Type,
     descriptors: Vec<(syn::Field, Vec<FnType>)>,
 ) -> syn::Result<TokenStream> {
-    let methods: Vec<TokenStream> = descriptors
-        .iter()
-        .flat_map(|&(ref field, ref fns)| {
-            fns.iter()
-                .map(|desc| {
-                    let name = field.ident.as_ref().unwrap();
-                    let field_ty = &field.ty;
-                    match *desc {
-                        FnType::Getter => {
-                            quote! {
-                                impl #cls {
-                                    fn #name(&self) -> pyo3::PyResult<#field_ty> {
-                                        Ok(self.#name.clone())
-                                    }
-                                }
-                            }
-                        }
-                        FnType::Setter => {
-                            let setter_name =
-                                syn::Ident::new(&format!("set_{}", name.unraw()), Span::call_site());
-                            quote! {
-                                impl #cls {
-                                    fn #setter_name(&mut self, value: #field_ty) -> pyo3::PyResult<()> {
-                                        self.#name = value;
-                                        Ok(())
-                                    }
-                                }
-                            }
-                        }
-                        _ => unreachable!(),
-                    }
-                })
-                .collect::<Vec<TokenStream>>()
-        })
-        .collect();
-
     let py_methods: Vec<TokenStream> = descriptors
         .iter()
         .flat_map(|&(ref field, ref fns)| {
             fns.iter()
                 .map(|desc| {
-                    let name = field.ident.as_ref().unwrap();
-
+                    let name = field.ident.as_ref().unwrap().unraw();
                     let doc = utils::get_doc(&field.attrs, None, true)
                         .unwrap_or_else(|_| syn::LitStr::new(&name.to_string(), name.span()));
 
-                    let field_ty = &field.ty;
                     match *desc {
-                        FnType::Getter => {
-                            let spec = FnSpec {
-                                tp: FnType::Getter,
-                                name: &name,
-                                python_name: name.unraw(),
-                                attrs: Vec::new(),
-                                args: Vec::new(),
-                                output: parse_quote!(PyResult<#field_ty>),
-                                doc,
-                            };
-                            Ok(impl_py_getter_def(&spec, &impl_wrap_getter(&cls, &spec)?))
-                        }
-                        FnType::Setter => {
-                            let setter_name = syn::Ident::new(
-                                &format!("set_{}", name.unraw()),
-                                Span::call_site(),
-                            );
-                            let spec = FnSpec {
-                                tp: FnType::Setter,
-                                name: &setter_name,
-                                python_name: name.unraw(),
-                                attrs: Vec::new(),
-                                args: vec![FnArg {
-                                    name: &name,
-                                    mutability: &None,
-                                    by_ref: &None,
-                                    ty: field_ty,
-                                    optional: None,
-                                    py: true,
-                                    reference: false,
-                                }],
-                                output: parse_quote!(PyResult<()>),
-                                doc,
-                            };
-                            Ok(impl_py_setter_def(&spec, &impl_wrap_setter(&cls, &spec)?))
-                        }
+                        FnType::Getter => Ok(impl_py_getter_def(
+                            &name,
+                            &doc,
+                            &impl_wrap_getter(&cls, PropertyType::Descriptor(&field))?,
+                        )),
+                        FnType::Setter => Ok(impl_py_setter_def(
+                            &name,
+                            &doc,
+                            &impl_wrap_setter(&cls, PropertyType::Descriptor(&field))?,
+                        )),
                         _ => unreachable!(),
                     }
                 })
@@ -513,7 +451,6 @@ fn impl_descriptors(
         .collect::<syn::Result<_>>()?;
 
     Ok(quote! {
-        #(#methods)*
 
         pyo3::inventory::submit! {
             #![crate = pyo3] {

pyo3-derive-backend/src/pymethod.rs

@@ -3,6 +3,12 @@ use crate::method::{FnArg, FnSpec, FnType};
 use crate::utils;
 use proc_macro2::{Span, TokenStream};
 use quote::quote;
+use syn::ext::IdentExt;
+
+pub enum PropertyType<'a> {
+    Descriptor(&'a syn::Field),
+    Function(&'a FnSpec<'a>),
+}
 
 pub fn gen_py_method(
     cls: &syn::Type,
@@ -21,8 +27,16 @@ pub fn gen_py_method(
         FnType::FnCall => impl_py_method_def_call(&spec, &impl_wrap(cls, &spec, false)),
         FnType::FnClass => impl_py_method_def_class(&spec, &impl_wrap_class(cls, &spec)),
         FnType::FnStatic => impl_py_method_def_static(&spec, &impl_wrap_static(cls, &spec)),
-        FnType::Getter => impl_py_getter_def(&spec, &impl_wrap_getter(cls, &spec)?),
-        FnType::Setter => impl_py_setter_def(&spec, &impl_wrap_setter(cls, &spec)?),
+        FnType::Getter => impl_py_getter_def(
+            &spec.python_name,
+            &spec.doc,
+            &impl_wrap_getter(cls, PropertyType::Function(&spec))?,
+        ),
+        FnType::Setter => impl_py_setter_def(
+            &spec.python_name,
+            &spec.doc,
+            &impl_wrap_setter(cls, PropertyType::Function(&spec))?,
+        ),
     })
 }
 
@@ -244,28 +258,48 @@ pub fn impl_wrap_static(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
     }
 }
 
-/// Generate functiona wrapper (PyCFunction, PyCFunctionWithKeywords)
-pub(crate) fn impl_wrap_getter(cls: &syn::Type, spec: &FnSpec) -> syn::Result<TokenStream> {
-    let takes_py = match &*spec.args {
-        [] => false,
-        [arg] if utils::if_type_is_python(arg.ty) => true,
-        _ => {
-            return Err(syn::Error::new_spanned(
-                spec.args[0].ty,
-                "Getter function can only have one argument of type pyo3::Python!",
-            ));
-        }
-    };
+fn impl_call_getter(spec: &FnSpec) -> syn::Result<TokenStream> {
+    let (py_arg, args) = split_off_python_arg(&spec.args);
+    if !args.is_empty() {
+        return Err(syn::Error::new_spanned(
+            args[0].ty,
+            "Getter function can only have one argument of type pyo3::Python",
+        ));
+    }
 
     let name = &spec.name;
-    let python_name = &spec.python_name;
-
-    let fncall = if takes_py {
+    let fncall = if py_arg.is_some() {
         quote! { _slf.#name(_py) }
     } else {
         quote! { _slf.#name() }
     };
 
+    Ok(fncall)
+}
+
+/// Generate functiona wrapper (PyCFunction, PyCFunctionWithKeywords)
+pub(crate) fn impl_wrap_getter(
+    cls: &syn::Type,
+    property_type: PropertyType,
+) -> syn::Result<TokenStream> {
+    let python_name;
+    let getter_impl;
+
+    match property_type {
+        PropertyType::Descriptor(field) => {
+            let name = field.ident.as_ref().unwrap();
+            python_name = name.unraw();
+            getter_impl = quote!({
+                use pyo3::derive_utils::GetPropertyValue;
+                (&_slf.#name).get_property_value(_py)
+            });
+        }
+        PropertyType::Function(spec) => {
+            python_name = spec.python_name.clone();
+            getter_impl = impl_call_getter(&spec)?;
+        }
+    };
+
     Ok(quote! {
         unsafe extern "C" fn __wrap(
             _slf: *mut pyo3::ffi::PyObject, _: *mut ::std::os::raw::c_void) -> *mut pyo3::ffi::PyObject
@@ -276,7 +310,7 @@ pub(crate) fn impl_wrap_getter(cls: &syn::Type, spec: &FnSpec) -> syn::Result<To
             let _pool = pyo3::GILPool::new(_py);
             let _slf = _py.mut_from_borrowed_ptr::<#cls>(_slf);
 
-            let result = pyo3::derive_utils::IntoPyResult::into_py_result(#fncall);
+            let result = pyo3::derive_utils::IntoPyResult::into_py_result(#getter_impl);
 
             match result {
                 Ok(val) => {
@@ -291,24 +325,48 @@ pub(crate) fn impl_wrap_getter(cls: &syn::Type, spec: &FnSpec) -> syn::Result<To
     })
 }
 
-/// Generate functiona wrapper (PyCFunction, PyCFunctionWithKeywords)
-pub(crate) fn impl_wrap_setter(cls: &syn::Type, spec: &FnSpec<'_>) -> syn::Result<TokenStream> {
+fn impl_call_setter(spec: &FnSpec) -> syn::Result<TokenStream> {
+    let (py_arg, args) = split_off_python_arg(&spec.args);
+
+    if args.is_empty() {
+        return Err(syn::Error::new_spanned(
+            &spec.name,
+            "Setter function expected to have one argument",
+        ));
+    } else if args.len() > 1 {
+        return Err(syn::Error::new_spanned(
+            &args[1].ty,
+            "Setter function can have at most two arguments: one of pyo3::Python, and one other",
+        ));
+    }
+
     let name = &spec.name;
-    let python_name = &spec.python_name;
+    let fncall = if py_arg.is_some() {
+        quote!(pyo3::derive_utils::IntoPyResult::into_py_result(_slf.#name(_py, _val)))
+    } else {
+        quote!(pyo3::derive_utils::IntoPyResult::into_py_result(_slf.#name(_val)))
+    };
 
-    let val_ty = match &*spec.args {
-        [] => {
-            return Err(syn::Error::new_spanned(
-                &spec.name,
-                "Not enough arguments for setter {}::{}",
-            ))
+    Ok(fncall)
+}
+
+/// Generate functiona wrapper (PyCFunction, PyCFunctionWithKeywords)
+pub(crate) fn impl_wrap_setter(
+    cls: &syn::Type,
+    property_type: PropertyType,
+) -> syn::Result<TokenStream> {
+    let python_name;
+    let setter_impl;
+
+    match property_type {
+        PropertyType::Descriptor(field) => {
+            let name = field.ident.as_ref().unwrap();
+            python_name = name.unraw();
+            setter_impl = quote!({ _slf.#name = _val; Ok(()) });
         }
-        [arg] => &arg.ty,
-        _ => {
-            return Err(syn::Error::new_spanned(
-                spec.args[0].ty,
-                "Setter function must have exactly one argument",
-            ))
+        PropertyType::Function(spec) => {
+            python_name = spec.python_name.clone();
+            setter_impl = impl_call_setter(&spec)?;
         }
     };
 
@@ -324,9 +382,9 @@ pub(crate) fn impl_wrap_setter(cls: &syn::Type, spec: &FnSpec<'_>) -> syn::Resul
             let _slf = _py.mut_from_borrowed_ptr::<#cls>(_slf);
             let _value = _py.from_borrowed_ptr(_value);
 
-            let _result = match <#val_ty as pyo3::FromPyObject>::extract(_value) {
+            let _result = match pyo3::FromPyObject::extract(_value) {
                 Ok(_val) => {
-                    pyo3::derive_utils::IntoPyResult::into_py_result(_slf.#name(_val))
+                    #setter_impl
                 }
                 Err(e) => Err(e)
             };
@@ -617,10 +675,11 @@ pub fn impl_py_method_def_call(spec: &FnSpec, wrapper: &TokenStream) -> TokenStr
     }
 }
 
-pub(crate) fn impl_py_setter_def(spec: &FnSpec, wrapper: &TokenStream) -> TokenStream {
-    let python_name = &&spec.python_name;
-    let doc = &spec.doc;
-
+pub(crate) fn impl_py_setter_def(
+    python_name: &syn::Ident,
+    doc: &syn::LitStr,
+    wrapper: &TokenStream,
+) -> TokenStream {
     quote! {
         pyo3::class::PyMethodDefType::Setter({
             #wrapper
@@ -634,10 +693,11 @@ pub(crate) fn impl_py_setter_def(spec: &FnSpec, wrapper: &TokenStream) -> TokenS
     }
 }
 
-pub(crate) fn impl_py_getter_def(spec: &FnSpec, wrapper: &TokenStream) -> TokenStream {
-    let python_name = &&spec.python_name;
-    let doc = &spec.doc;
-
+pub(crate) fn impl_py_getter_def(
+    python_name: &syn::Ident,
+    doc: &syn::LitStr,
+    wrapper: &TokenStream,
+) -> TokenStream {
     quote! {
         pyo3::class::PyMethodDefType::Getter({
             #wrapper
@@ -650,3 +710,11 @@ pub(crate) fn impl_py_getter_def(spec: &FnSpec, wrapper: &TokenStream) -> TokenS
         })
     }
 }
+
+/// Split an argument of pyo3::Python from the front of the arg list, if present
+fn split_off_python_arg<'a>(args: &'a [FnArg<'a>]) -> (Option<&FnArg>, &[FnArg]) {
+    match args {
+        [py, rest @ ..] if utils::if_type_is_python(&py.ty) => (Some(py), rest),
+        rest => (None, rest),
+    }
+}