Skip to content

Clean up BindingGenerator interface#2881

Merged
messense merged 2 commits intoPyO3:mainfrom
e-nomem:binding-cleanup
Dec 3, 2025
Merged

Clean up BindingGenerator interface#2881
messense merged 2 commits intoPyO3:mainfrom
e-nomem:binding-cleanup

Conversation

@e-nomem
Copy link
Copy Markdown
Contributor

@e-nomem e-nomem commented Dec 1, 2025

The last PR about the BindingGenerator trait! (at least for now 😅 )

Now that all the bindings have been migrated, it seems fairly clear that the temp dir and python interpreter only matter for some bindings and shouldn't be a part of the trait interface itself. This PR hoists those parameters into the generator struct if necessary and adjusts the code to match.

It also creates a BindingType enum for PyO3 so that we can error out early if the python interpreter is required but missing, rather than panicking later on.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the BindingGenerator trait interface to remove temp_dir and interpreter parameters from the trait method signature. These dependencies are now managed within the generator structs themselves, improving encapsulation and simplifying the trait interface. The changes also introduce a BindingType enum for PyO3 to enable early validation of interpreter requirements.

Key Changes

  • Removed temp_dir and interpreter parameters from the BindingGenerator::generate_bindings() trait method
  • Added lifetime parameters to Pyo3BindingGenerator and CffiBindingGenerator to store interpreter references
  • Introduced BindingType enum in PyO3 generator to distinguish between abi3 and non-abi3 builds with early error checking
  • Updated all generator constructors to accept and validate their required dependencies upfront

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/binding_generator/mod.rs Removed temp_dir and interpreter parameters from the BindingGenerator trait and generate_binding function signature
src/binding_generator/pyo3_binding.rs Added lifetime parameter, introduced BindingType enum, moved tempdir into struct, added early interpreter validation in constructor
src/binding_generator/cffi_binding.rs Added lifetime parameter, moved interpreter and tempdir into struct, updated constructor to require interpreter, removed redundant tempdir.close() call
src/binding_generator/uniffi_binding.rs Removed unused temp_dir and interpreter parameters from generate_bindings method
src/binding_generator/bin_binding.rs Removed unused temp_dir and interpreter parameters from generate_bindings method
src/build_context.rs Updated all binding generator instantiations to use new constructors with proper error handling, simplified generate_binding calls

Comment thread src/binding_generator/pyo3_binding.rs Outdated
@messense messense merged commit 347ebcc into PyO3:main Dec 3, 2025
45 checks passed
@e-nomem e-nomem deleted the binding-cleanup branch December 3, 2025 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants