feat!: change `InstructionHandler` to be a trait and require it everywhere in Rust
This PR removes the InstructionHandler type that is defined in terms of boxed closures, and replaces it with an InstructionHandler trait. This has the potential to provide signficant performance improvements (and I think it's probably cleaner regardless).
This PR also removes every Rust method that could use InstructionHandler but doesn't, requiring a deliberate use of an instruction handler. However, because we don't expose instruction handlers to Python, this is not true for our Python bindings. Ideally, this should not be a change for Python users, but I would appreciate close review of this part.
PR Preview Action v1.6.2 :---: |
:rocket: View preview athttps://rigetti.github.io/quil-rs/pr-preview/pr-484/
|
Built to branch quil-py-docs at 2025-11-07 21:08 UTC.
Preview will be ready when the GitHub Pages deployment is complete.
This has the potential to provide signficant performance improvements (and I think it's probably cleaner regardless).
Have you verified any potential improvements with benchmarks?
I marked this approved, but just noticed the CI checks failed due to an MSRV issue. Can you fix that up before merging?
I marked this approved, but just noticed the CI checks failed due to an MSRV issue. Can you fix that up before merging?
@asaites This is a false positive – see rust-lang/rust-clippy#15792. We're not the only ones running into this with PyO3, either. There's not really a good fix except for ignoring the warning entirely, and I'm not sure that we want to do that.