Skip to content

Comments

Manual C header#14430

Closed
Cryoris wants to merge 13 commits intoQiskit:mainfrom
Cryoris:c-header
Closed

Manual C header#14430
Cryoris wants to merge 13 commits intoQiskit:mainfrom
Cryoris:c-header

Conversation

@Cryoris
Copy link
Collaborator

@Cryoris Cryoris commented May 22, 2025

Summary

This PR implements the headers for the C API manually and removes the dependency on cbindgen.
This needs rebasing on top of #14340.

Details

The header structure here is proposed as

cext/crates/
   include/
       qiskit.h  // main header that we include in C programs. Can also hold metadata, like the version.
       qiskit/  // dependencies
           circuit.h
           observable.h
           <... and all other headers...>

Why remove cbindgen?

cbindgen is a tool we currently use to automatically generate the header for the C FFI for Rust, off the .rs files that expose Rust structs, enums and functions. It's been super useful in starting the C API, but we're already hitting some limitations and expecting some more serious limitations in the future. The two main points IMO are:

  • cbindgen treats all objects as part of a global namespace: this means that trying to expose an object some_namespace::Thing to C only works if there is no other object called Thing in the parsed Rust API. This is a fundamental limitation in cbindgen, which they also mention in their docs.
  • the project is maintained on a rather infrequent basis. This doesn't generally need to be an issue, but there are a series of small problems/missing features we have that I think could be solved rather easily in cbindgen. But that requires active PR and issue reviews.

Smaller problems we can work around (but that get a bit annoying and sum up):

  • cbindgen pulls the Rust docs of objects we expose as opaque pointers. For example, we expose SparseObservable as typedef struct SparseObservable SparseObservable to C, which causes the header to include the Rust docstring. We currently solve this by manually writing the Sphinx docs for these objects. This also affects enums we directly expose, such as BitTerm.
  • We need to manually define object renames, due to missing flexibility in cbindgens export control.
  • We require some custom headers anyways (e.g. complex number definitions) and end up with a mix of generated headers and tracked headers, which we need to orchestrate.

Why keep cbindgen?

  • If we write the header manually, we need to include the docs there. This either leads to doc duplication or less extensive docs in the Rust code that exposes the C FFI.
  • So far we've been able to work around all issues. The smaller issues listed above could be resolved with PRs to cbindgen (as we're trying with Allow excluding prefixes for objects mozilla/cbindgen#1064). The global namespace could be a dealbreaker in the longer run, though.

Todos

@Cryoris Cryoris requested a review from a team as a code owner May 22, 2025 09:35
@Cryoris Cryoris added the C API Related to the C API label May 22, 2025
@qiskit-bot
Copy link
Collaborator

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

@jakelishman
Copy link
Member

Shall we close this PR? For the time being at least, we don't seem to be heading in this direction, and the PR's just going to get super stale if left open.

@jakelishman jakelishman added this to the 2.3.0 milestone Oct 8, 2025
@github-project-automation github-project-automation bot moved this to Ready in Qiskit 2.3 Oct 8, 2025
@jakelishman jakelishman moved this from Ready to In review in Qiskit 2.3 Oct 8, 2025
@jakelishman jakelishman removed this from Qiskit 2.3 Oct 8, 2025
@jakelishman jakelishman removed this from the 2.3.0 milestone Oct 8, 2025
@jakelishman
Copy link
Member

Since Julien didn't reply, I'll just close it (since I assume that's the correct state) and see if he re-opens it to indicate I was wrong.

@jakelishman jakelishman closed this Oct 8, 2025
@Cryoris
Copy link
Collaborator Author

Cryoris commented Oct 8, 2025

Whoops sorry, this fell off my radar. I would expect that sooner or later we'll want to do this for the reasons listed in the description, but I'm also fine keeping it for now since we were able to circumnavigate problems so far.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants