Skip to content

Support typedefs and function pointers in bindgen - #16847

Open
Cryoris wants to merge 3 commits into
Qiskit:mainfrom
Cryoris:ctypes-more-types
Open

Support typedefs and function pointers in bindgen#16847
Cryoris wants to merge 3 commits into
Qiskit:mainfrom
Cryoris:ctypes-more-types

Conversation

@Cryoris

@Cryoris Cryoris commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

In the recent passmanager in C infrastructure I stumbled upon exposing typedefs and function pointers from Rust to C. This PR adds support to the bindgen crate. There's no explicit tests that I've found but I tested this locally -- with something that is not included in this PR though.

We might end up not needing this for the C passmanager, and I'm fine merging something like this at a later stage. But now that I've had this locally and it's working I wanted to open the PR anyways since it seems very useful to have!

AI/LLM disclosure

  • No part of this submission is LLM generated.
  • Some written text was generated by:
  • Some submitted code was generated by: Claude Sonnet 5

Claude added the bit on function pointers, I've tested this locally and went over the changes.

@Cryoris
Cryoris requested a review from a team as a code owner August 27, 2026 12:37
@Cryoris
Cryoris requested a review from alexanderivrii August 27, 2026 12:37
@Cryoris Cryoris added Changelog: None Do not include in the GitHub Release changelog. C API Related to the C API labels Aug 27, 2026
@qiskit-bot

Copy link
Copy Markdown
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

pub fn r#type(
mut ty: &ir::Type,
mut override_fn: impl FnMut(&str) -> Option<Primitive>,
override_fn: &mut dyn FnMut(&str) -> Option<Primitive>,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This change is because we're defining the function pointer recursively by calling into r#type -- with the previous version cargo complained about an infinite recursion.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we should need dyn here, but I'm not certain:

  • we should be able to re-write it to be iterative instead; we're already doing that to handle pointers, which would otherwise be naturally recursive.
  • I think we can rewrite it as &mut impl FnMut(&str) -> Option<Primitive>, and then rely on mutable-reference reborrows to keep the type equal. The initial calls of this function change to &mut |name| { ... } (so you manually reference the immediate-value lambda), but the recursive calls stay as-is.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wouldn't really bother with the first bullet point, it's more just a comment that the function already has a natural recursive description, but I wrote it iteratively.

@Cryoris
Cryoris requested review from jakelishman and removed request for alexanderivrii August 27, 2026 12:38

@jakelishman jakelishman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not an actual review: let's hold off on actually merging this til we've positively decided that we'll actually use it - there's an open question on #16839 about whether we should use (effectively) the same API as #16549, which ends up erasing the function-pointer type from the type system, so doesn't use this handling.

@mtreinish

Copy link
Copy Markdown
Member

FWIW, supporting function pointers in bindgen is something I wanted for #16810 too. I had another branch locally that potentially hit the need for it too.

@jakelishman

Copy link
Copy Markdown
Member

I have questions about the use of them in that PR too, but yeah, if we use them, we can use this PR to add them no trouble.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C API Related to the C API Changelog: None Do not include in the GitHub Release changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants