Skip to content

Use PyMethodsImpl instead of *ProtocolImpl::methods #917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- The `GILGuard` returned from `Python::acquire_gil` will now only assume responsiblity for freeing owned references on drop if no other `GILPool` or `GILGuard` exists. This ensures that multiple calls to the safe api `Python::acquire_gil` cannot lead to dangling references. [#893](https://github.com/PyO3/pyo3/pull/893)
- The trait `ObjectProtocol` has been removed, and all the methods from the trait have been moved to `PyAny`. [#911](https://github.com/PyO3/pyo3/pull/911)
- The exception to this is `ObjectProtocol::None`, which has simply been removed. Use `Python::None` instead.
- No `#![feature(specialization)]` in user code. [#917](https://github.com/PyO3/pyo3/pull/917)

### Removed
- `PyMethodsProtocol` is now renamed to `PyMethodsImpl` and hidden. [#889](https://github.com/PyO3/pyo3/pull/889)
- `num-traits` is no longer a dependency. [#895](https://github.com/PyO3/pyo3/pull/895)
- `ObjectProtocol`. [#911](https://github.com/PyO3/pyo3/pull/911)
- All `*ProtocolImpl` traits. [#917](https://github.com/PyO3/pyo3/pull/917)

### Fixed
- `__radd__` and other `__r*__` methods now correctly work with operators. [#839](https://github.com/PyO3/pyo3/pull/839)
Expand Down
143 changes: 72 additions & 71 deletions guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,76 +17,7 @@ struct MyClass {
The above example generates implementations for [`PyTypeInfo`], [`PyTypeObject`],
and [`PyClass`] for `MyClass`.

Specifically, the following implementation is generated:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this code block to the last of the page.
It's too long for a user who doesn't have a strong interest in the implementation detail.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!


```rust
use pyo3::prelude::*;

/// Class for demonstration
struct MyClass {
num: i32,
debug: bool,
}

impl pyo3::pyclass::PyClassAlloc for MyClass {}

unsafe impl pyo3::PyTypeInfo for MyClass {
type Type = MyClass;
type BaseType = PyAny;
type BaseLayout = pyo3::pycell::PyCellBase<PyAny>;
type Layout = PyCell<Self>;
type Initializer = PyClassInitializer<Self>;
type AsRefTarget = PyCell<Self>;

const NAME: &'static str = "MyClass";
const MODULE: Option<&'static str> = None;
const DESCRIPTION: &'static str = "Class for demonstration";
const FLAGS: usize = 0;

#[inline]
fn type_object() -> &'static pyo3::ffi::PyTypeObject {
use pyo3::type_object::LazyStaticType;
static TYPE_OBJECT: LazyStaticType = LazyStaticType::new();
TYPE_OBJECT.get_or_init::<Self>()
}
}

impl pyo3::pyclass::PyClass for MyClass {
type Dict = pyo3::pyclass_slots::PyClassDummySlot;
type WeakRef = pyo3::pyclass_slots::PyClassDummySlot;
type BaseNativeType = PyAny;
}

impl pyo3::IntoPy<PyObject> for MyClass {
fn into_py(self, py: pyo3::Python) -> pyo3::PyObject {
pyo3::IntoPy::into_py(pyo3::Py::new(py, self).unwrap(), py)
}
}

pub struct Pyo3MethodsInventoryForMyClass {
methods: &'static [pyo3::class::PyMethodDefType],
}

impl pyo3::class::methods::PyMethodsInventory for Pyo3MethodsInventoryForMyClass {
fn new(methods: &'static [pyo3::class::PyMethodDefType]) -> Self {
Self { methods }
}

fn get_methods(&self) -> &'static [pyo3::class::PyMethodDefType] {
self.methods
}
}

impl pyo3::class::methods::PyMethodsImpl for MyClass {
type Methods = Pyo3MethodsInventoryForMyClass;
}

pyo3::inventory::collect!(Pyo3MethodsInventoryForMyClass);
# let gil = Python::acquire_gil();
# let py = gil.python();
# let cls = py.get_type::<MyClass>();
# pyo3::py_run!(py, cls, "assert cls.__name__ == 'MyClass'")
```
If you curious what `#[pyclass]` generates, see [How methods are implemented](#how-methods-are-implemented) section.

## Adding the class to a module

Expand Down Expand Up @@ -944,7 +875,77 @@ pyclass dependent on whether there is an impl block, we'd need to implement the
`#[pyclass]` and override the implementation in `#[pymethods]`, which is to the best of my knowledge
only possible with the specialization feature, which can't be used on stable.

To escape this we use [inventory](https://github.com/dtolnay/inventory), which allows us to collect `impl`s from arbitrary source code by exploiting some binary trick. See [inventory: how it works](https://github.com/dtolnay/inventory#how-it-works) and `pyo3_derive_backend::py_class::impl_inventory` for more details.
To escape this we use [inventory](https://github.com/dtolnay/inventory),
which allows us to collect `impl`s from arbitrary source code by exploiting some binary trick.
See [inventory: how it works](https://github.com/dtolnay/inventory#how-it-works) and `pyo3_derive_backend::py_class` for more details.

Specifically, the following implementation is generated:

```rust
use pyo3::prelude::*;

/// Class for demonstration
struct MyClass {
num: i32,
debug: bool,
}

impl pyo3::pyclass::PyClassAlloc for MyClass {}

unsafe impl pyo3::PyTypeInfo for MyClass {
type Type = MyClass;
type BaseType = PyAny;
type BaseLayout = pyo3::pycell::PyCellBase<PyAny>;
type Layout = PyCell<Self>;
type Initializer = PyClassInitializer<Self>;
type AsRefTarget = PyCell<Self>;

const NAME: &'static str = "MyClass";
const MODULE: Option<&'static str> = None;
const DESCRIPTION: &'static str = "Class for demonstration";
const FLAGS: usize = 0;

#[inline]
fn type_object() -> &'static pyo3::ffi::PyTypeObject {
use pyo3::type_object::LazyStaticType;
static TYPE_OBJECT: LazyStaticType = LazyStaticType::new();
TYPE_OBJECT.get_or_init::<Self>()
}
}

impl pyo3::pyclass::PyClass for MyClass {
type Dict = pyo3::pyclass_slots::PyClassDummySlot;
type WeakRef = pyo3::pyclass_slots::PyClassDummySlot;
type BaseNativeType = PyAny;
}

impl pyo3::IntoPy<PyObject> for MyClass {
fn into_py(self, py: pyo3::Python) -> pyo3::PyObject {
pyo3::IntoPy::into_py(pyo3::Py::new(py, self).unwrap(), py)
}
}

pub struct Pyo3MethodsInventoryForMyClass {
methods: &'static [pyo3::class::PyMethodDefType],
}
impl pyo3::class::methods::PyMethodsInventory for Pyo3MethodsInventoryForMyClass {
fn new(methods: &'static [pyo3::class::PyMethodDefType]) -> Self {
Self { methods }
}
fn get(&self) -> &'static [pyo3::class::PyMethodDefType] {
self.methods
}
}
impl pyo3::class::methods::PyMethodsImpl for MyClass {
type Methods = Pyo3MethodsInventoryForMyClass;
}
pyo3::inventory::collect!(Pyo3MethodsInventoryForMyClass);
# let gil = Python::acquire_gil();
# let py = gil.python();
# let cls = py.get_type::<MyClass>();
# pyo3::py_run!(py, cls, "assert cls.__name__ == 'MyClass'")
```


[`GILGuard`]: https://docs.rs/pyo3/latest/pyo3/struct.GILGuard.html
[`PyGCProtocol`]: https://docs.rs/pyo3/latest/pyo3/class/gc/trait.PyGCProtocol.html
Expand Down
20 changes: 7 additions & 13 deletions pyo3-derive-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,9 @@ fn parse_descriptors(item: &mut syn::Field) -> syn::Result<Vec<FnType>> {
Ok(descs)
}

/// The orphan rule disallows using a generic inventory struct, so we create the whole boilerplate
/// once per class
fn impl_inventory(cls: &syn::Ident) -> TokenStream {
// Try to build a unique type that gives a hint about it's function when
// it comes up in error messages
/// To allow multiple #[pymethods]/#[pyproto] block, we define inventory types.
fn impl_methods_inventory(cls: &syn::Ident) -> TokenStream {
// Try to build a unique type for better error messages
let name = format!("Pyo3MethodsInventoryFor{}", cls);
let inventory_cls = syn::Ident::new(&name, Span::call_site());

Expand All @@ -221,15 +219,11 @@ fn impl_inventory(cls: &syn::Ident) -> TokenStream {
pub struct #inventory_cls {
methods: &'static [pyo3::class::PyMethodDefType],
}

impl pyo3::class::methods::PyMethodsInventory for #inventory_cls {
fn new(methods: &'static [pyo3::class::PyMethodDefType]) -> Self {
Self {
methods
}
Self { methods }
}

fn get_methods(&self) -> &'static [pyo3::class::PyMethodDefType] {
fn get(&self) -> &'static [pyo3::class::PyMethodDefType] {
self.methods
}
}
Expand Down Expand Up @@ -345,7 +339,7 @@ fn impl_class(
quote! {}
};

let inventory_impl = impl_inventory(&cls);
let impl_inventory = impl_methods_inventory(&cls);

let base = &attr.base;
let flags = &attr.flags;
Expand Down Expand Up @@ -418,7 +412,7 @@ fn impl_class(

#into_pyobject

#inventory_impl
#impl_inventory

#extra

Expand Down
59 changes: 30 additions & 29 deletions pyo3-derive-backend/src/pyproto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub fn build_py_proto(ast: &mut syn::ItemImpl) -> syn::Result<TokenStream> {
));
};

let tokens = impl_proto_impl(&ast.self_ty, &mut ast.items, proto);
let tokens = impl_proto_impl(&ast.self_ty, &mut ast.items, proto)?;

// attach lifetime
let mut seg = path.segments.pop().unwrap().into_value();
Expand All @@ -57,53 +57,54 @@ fn impl_proto_impl(
ty: &syn::Type,
impls: &mut Vec<syn::ImplItem>,
proto: &defs::Proto,
) -> TokenStream {
let mut tokens = TokenStream::new();
) -> syn::Result<TokenStream> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

let mut trait_impls = TokenStream::new();
let mut py_methods = Vec::new();

for iimpl in impls.iter_mut() {
if let syn::ImplItem::Method(ref mut met) = iimpl {
if let Some(m) = proto.get_proto(&met.sig.ident) {
impl_method_proto(ty, &mut met.sig, m).to_tokens(&mut tokens);
impl_method_proto(ty, &mut met.sig, m).to_tokens(&mut trait_impls);
}
if let Some(m) = proto.get_method(&met.sig.ident) {
let name = &met.sig.ident;
let proto: syn::Path = syn::parse_str(m.proto).unwrap();

let fn_spec = match FnSpec::parse(&met.sig, &mut met.attrs, false) {
Ok(fn_spec) => fn_spec,
Err(err) => return err.to_compile_error(),
};
let meth = pymethod::impl_proto_wrap(ty, &fn_spec);
let fn_spec = FnSpec::parse(&met.sig, &mut met.attrs, false)?;
let method = pymethod::impl_proto_wrap(ty, &fn_spec);
let coexist = if m.can_coexist {
// We need METH_COEXIST here to prevent __add__ from overriding __radd__
quote!(pyo3::ffi::METH_COEXIST)
} else {
quote!(0)
};
// TODO(kngwyu): doc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

py_methods.push(quote! {
impl #proto for #ty
{
#[inline]
fn #name() -> Option<pyo3::class::methods::PyMethodDef> {
#meth

Some(pyo3::class::PyMethodDef {
ml_name: stringify!(#name),
ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap),
// We need METH_COEXIST here to prevent __add__ from overriding __radd__
ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS | #coexist,
ml_doc: ""
})
pyo3::class::PyMethodDefType::Method({
#method
pyo3::class::PyMethodDef {
ml_name: stringify!(#name),
ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap),
ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS | #coexist,
ml_doc: ""
}
}
})
});
}
}
}

quote! {
#tokens

#(#py_methods)*
if py_methods.is_empty() {
return Ok(quote! { #trait_impls });
}
let inventory_submission = quote! {
pyo3::inventory::submit! {
#![crate = pyo3] {
type ProtoInventory = <#ty as pyo3::class::methods::PyMethodsImpl>::Methods;
<ProtoInventory as pyo3::class::methods::PyMethodsInventory>::new(&[#(#py_methods),*])
}
}
};
Ok(quote! {
#trait_impls
#inventory_submission
})
}
Loading