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

Discussion: how to deal with XED's static inline functions #10

Closed
agodnic opened this issue Jul 2, 2019 · 3 comments
Closed

Discussion: how to deal with XED's static inline functions #10

agodnic opened this issue Jul 2, 2019 · 3 comments
Labels
discussion Open discussion

Comments

@agodnic
Copy link
Member

agodnic commented Jul 2, 2019

Bindgen is currently unable to deal with functions declared as static inline (see #1344 and #1090).
XED has way too many functions like this.

The current workaround being used in this repository is to manually run c2rust to transpile the whole functions.

Disadvantages of current approach

  1. Updating the XED git submodule to a newer version requires manually running c2rust.
  2. (I'm not sure about this one) The transpiled code may be architecture-dependant in some way, since the function's code is generated for a fixed processor architecture and potentially run on a different architecture.

Possible solutions

  1. Change build.rs so that c2rust is run in conjunction with bindgen.
  2. Implement the feature we need in bindgen.
@agodnic agodnic added the discussion Open discussion label Jul 2, 2019
@Phantomical
Copy link
Member

Phantomical commented Jul 2, 2019

The major problem with running c2rust automatically here is that the output of c2rust includes a lot of stuff that's also generated by bindgen. Specifically, it has external function declarations, struct definitions, enum definitions, anything that's not an inline method from the headers. In order for us to be able to use it automatically we'd have to figure out how to only generate the parts we want. I don't know if that is possible with c2rust.

I think this leaves these options

  1. Investigate whether running c2rust in build.rs is possible the way we want it to
  2. Implement the features in bindgen
  3. Stay with the status quo and add some checks to ensure that the c2rust bindings don't get out of date (either something like a release checklist or unit tests)

@bepzi
Copy link

bepzi commented Aug 23, 2019

Just another perspective chiming in: I've got the same issue with my port of LV2, and I've found that c2rust isn't enough. If there's code doesn't get used anywhere, it won't be transpiled (#105). So in my case, c2rust would only work as expected if I ran it on a C program that directly or indirectly brings in every single type and function in the API, which I can't (or don't want to) do.

It seems like this project has a lot of actual C code (as opposed to just headers) and tests, so c2rust might work just fine, but I believe the better long-term solution would be to implement what we need in rust-bindgen.

@agodnic
Copy link
Member Author

agodnic commented Oct 28, 2019

Worked around in #24
This should enable us to update XED versions more often.

@agodnic agodnic closed this as completed Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Open discussion
Projects
None yet
Development

No branches or pull requests

3 participants