Skip to content
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

pymethods: prevent methods sharing the same name #2399

merged 1 commit into from
May 24, 2022


Copy link

This PR tweaks the #[pymethods] code generation so that the generated functions have meaningful names and are members of the associated class.

In general, the structure of generated code has changed from

unsafe extern "C" fn __wrap(...) { ... }


impl SomeClass {
    unsafe extern "C" fn __pymethod_foo__(...) { ... }

There are a few nice benefits that this brings:

  • As the generated function names are based on their names exposed to Python, and they share the struct's namespace, it's not possible to have multiple pymethods with the same #[pyo3(name = "foo")].
  • We have a hack to replace_self to remove Self from generated code; we don't need this now the generated code is associated with the class.
  • Stack traces will be nicer in panic messages etc, because the frames will have meaningful names rather than __wrap (that was actually what motivated me to try this today as I was struggling to debug WIP: new function signature #2302; this idea's been in the back of my mind for a while)

Copy link

mejrs commented May 24, 2022

Thanks, this should make reading cargo expand output a bit nicer too :)

Copy link
Member Author

Also true! While working on this I also realised that (with some extra work) it might be possible to rearrange the generated code so that we just add all these generated methods in a single impl SomeClass block, which might reduce the visual complexity of the generated output further.

@davidhewitt davidhewitt merged commit eafbbc5 into PyO3:main May 24, 2022
@davidhewitt davidhewitt deleted the avoid-duplicate-pymethods branch May 24, 2022 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

2 participants