pymethods: add support for protocol methods

[davidhewitt]

This is the beginning of an implementation which makes it possible to implement __dunder__ methods directly in #[pymethods] instead of requiring separate protocols.

From experimentation here it looks like it should be possible to support this alongside the existing #[pyproto] traits. This would allow us to deprecate #[pyproto] in 0.15 rather than flat-out remove, giving users a little bit of time to migrate.

Similarly, that enables the implementation for this to come in chunks.

There's a number of things this first PR won't cover:

• Some protocols which need design discussion:
  • The GC protocol - doesn't have a well-specified __dunder__ at the Python level, so we'll need to design one
  • The buffer protocol - same thing for this
  • The sequence protocol - it overlaps messily with the mapping protocol; we might want to do the same as what happens at the Python level, or we might want to make __sequence_getitem__ methods etc to be explicit.
• Suppport for multiple-pymethods feature
• Documentation
• Decprecations

All of the above can be recorded in a tracking issue once this first PR is merged.

This first PR is not ready yet. Progress as follows:

• PyObjectProtocol
• PyDescrProtocol
• PyAsyncProtocol
• PyIterProtocol
• PyMappingProtocol
• PyNumberProtocol

Overall, having solved PyObjectProtocol and PyDescrProtocol I've satisfied myself that I have an approach that can work. There's a lot of janky codegen added to pyo3-macros-backend/src/pymethod.rs. This can definitely be refactored further and made better.

I use dtolnay specialization to merge the __setattr__ and __delattr__ methods into the single slot they share. The same technique should work to handle the "reversible" methods in PyNumberProtocol.

For reviewing - at the moment this code is rather hacky, so might be hard to follow. That said, anyone who's curious is welcome to review and comment at any time! I'll come back to make progress on this whenever I get a chance...

[davidhewitt]

This is now pretty close to mergeable. Most the numerical methods are now supported. There's a few TODOs I've left at the bottom of test_proto_methods.rs

I haven't yet measured how this might affect compile times. I worry a bit that the heavy use of dtolnay specialisation might cause LLVM to crunch on a lot of boilerplate for each pyclass, however I also think that most of it should optimize out. So depending on how smart rustc is, most of that stuff should never reach LLVM. Hopefully we can find ways to solve that, if there is a problem. EDIT: I've refactored a little bit and now a #[pymethod] produces slightly less code than the #[pyproto] implementation did.

I definitely really like how this feels for pyclass usage - I can just start defining #[pymethods] for stuff like __repr__ and they just work. No need to look up protocol traits or anything like that. I'm sure there's a bunch of kinks that we will need to iron out through time.

TBH, given that this doesn't actually break #[pyproto] at all, I'm now thinking that we could even keep both around for a release or two while we're finalizing these #[pymethods], and then once we're confident that this works well, we can set about deprecating #[pyproto].

Something like:

• 0.15: #[pyproto] is still main implementation, #[pymethods] is documented as an experimental alternative
• 0.16:#[pymethods] is documented as the main preferred way, #[pyproto] is still documented as legacy
• 0.17: #[pyproto] is deprecated
• 0.18: #[pyproto] is removed

[davidhewitt]

Ok, this is now as complete as I want it to be to merge a first version. All the traits above are covered.

The TODOs above still apply, additionally there's a couple more:

• tests for __delete__, __anext__, __aiter__, __index__, __int__, __float__, __invert__
• (maybe) better argument extraction errors (if there's an error, the generated code won't say which argument failed, athough that's probably obvious as most these methods take at most one argument).
• __call__ (from discussion below)
• __new__ / __init__ ?

I think we've discussed this concept enough already in #1206, so any reviews here would be about reading the code. If anyone has time to do so: comments, questions, and nits are all welcome. It would be nice to make sure what I've built is maintainable.

I'll merge this next weekend if there's no objections by then; I think this is a great step forward, but there's a lot of follow-up TODOs which we can track once this goes in and gets the ball rolling. I'm sure there's a lot of cleanup and refactoring that could yet still be done in pyo3-macros-backend, however I'm also quite happy with what I've produced.

[mejrs]

I can't really give a technical review of this, because that's a lot of code I'm not familiar with, but I'd just like to say thank you for doing this 👍

I wonder, can we do __new__ and __call__ like this too? I always thought it was weird they needed those #[new] and #[call] attributes (sorry for possibly nerdsniping you into doing more work).

Anyway I plan to try this out and write some things about it in the next few weeks.

[davidhewitt]

😄 thanks - I do this for pleasure though it's always nice to enjoy a shout-out!

I wonder, can we do __new__ and __call__ like this too? I always thought it was weird they needed those #[new] and #[call] attributes (sorry for possibly nerdsniping you into doing more work).

Yes I actually was intending to propose to change __call__ to use this mechanism for sure! It would reduce the amount of special-case code in the macros backend, though it needs a bit of refactoring first because it takes *args and **kwargs at the C level (which the rest of the protos don't).

__new__ - I'm undecided on, because it's quite nice imo to define the Rust new and Python __new__ in one go and I wouldn't want to be writing MyClass::__new__ in normal Rust code. However I can definitely see arguments to support __new__ for consistency, especially for structs where the Rust and Python constructors are different. There's also a related question in that we don't support __init__ at all (#1644), which we definitely could consider. So I would like to do some more thinking before committing to what to do for __new__.

I'll add both to the TODOs further up.

[davidhewitt]

Ok I'm going to proceed to merge this now! If anyone else has outstanding comments, please bring them to the discussion in #1884