PyO3 / PyO3/pyo3

FFI module cleanup

Open
#1,289 12 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Good First Issue needs-implementer refactoring
Dominant language
Rust
Stars
16.2k
Forks
1k
Avg merge
2d 6h
Merged PRs (30d)
66

Description

The modules in src/ffi have become a bit out-of-date, and also quite disorganised so it's kinda hard to figure out what's missing.

Since Python 3.9 the cpython include files have been split so that (roughly) the limited api is in cpython/Include/ and the unlimited api is in cpython/Include/cpython/. I propose for simplicity in maintaining the pyo3::ffi module we should reorganise code to match that structure.

I suggest that we do the following things:

  • reorganise ffi/mod.rs to be alphabetical and have // skipped foo.h comments for each header present in cpython master which is not present in our bindings at the moment.
  • add ffi/cpython/mod.rs to contain the unlimited api headers, and do the same as with ffi/mod.rs.
  • for each file in ffi/, reorganise the definitions in it appropriately into ffi and ffi/cpython parts which match the cpython master definitions. This should include:
    • Put the existing definitions in the same order as upstream cpython.
    • Either:
      • Add // skipped foo for each definition foo we don't currently have in our definitions.
      • Add new definition for each we don't currently have in our definitions.

The idea with each // skipped foo being in alphabetical order is that it's then hopefully easier for us to compare against upstream in the future.

With the new cpython folder for the unlimited api, it might be reduce a lot of #[cfg(Py_LIMITED_API)] chatter as we'll only need that once on the mod cpython definition.

Also, it could be interesting to not have pub use self::cpython::* in ffi/mod.rs, so that users are forced to use unlimited api symbols as ffi::cpython::foo. But that's probably quite breaking, so I can go either way on it.

Perhaps in general the above guidelines of how to organise our ffi modules should go in CONTRIBUTING.md ?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/ffi/mod.rs and src/ffi/cpython/mod.rs, then compare each file under src/ffi with the corresponding CPython master headers. Reorganize definitions into ffi and ffi/cpython in upstream order, adding the requested skipped comments or missing definitions; the work is done when the modules consistently match the upstream split.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, rust
Domain
backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.