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

Expose static inline functions to bindings #123

Closed
DeltaF1 opened this issue May 15, 2023 · 3 comments · Fixed by #133
Closed

Expose static inline functions to bindings #123

DeltaF1 opened this issue May 15, 2023 · 3 comments · Fixed by #133

Comments

@DeltaF1
Copy link
Contributor

DeltaF1 commented May 15, 2023

libctru has some functions that are static inline (I ran into getThreadCommandBuffer specifically) and so are not exposed by bindgen by default.

rust-lang/rust-bindgen#2335 adds a feature to generate bindings for such functions. I got as far as adding these arguments to bindgen.sh:

    --experimental \
    --wrap-static-fns \

which adds the missing functions to bindings.rs, and generates a C file with wrapper functions in /tmp/bindgen/extern.c. I don't understand build processes enough to proceed from this point, but extern.c needs to be compiled and then linked with the library. Here's a link discussing how to do this and also about how to enable LTO so that ctru-rs can benefit from inlining those functions. I believe this also depends upon #110 fixing the __getreent issue.

@Meziu
Copy link
Member

Meziu commented May 16, 2023

@ian-h-chamberlain @AzureMarker Do you think we should add experimental options to bindgen? It would better cover libctru with the bindings, but we are already full of experimental features here and there.

I'm asking because I intend on merging #110, so I could add this in if you think it's fine.

@AzureMarker
Copy link
Member

If we do add it, it should be a separate PR. I do like having increased coverage of the libctru static functions, but since we already converted the common ones I'm not sure that we need to do this refactor now. Feel free to go ahead and open a PR for it though if someone gets it working.

@ian-h-chamberlain
Copy link
Member

I'd echo the same, the new bindgen feature seems nice but a separate PR would probably be good just to make it easier to review, since some of the manually-ported functions would have to get removed.

At first I was thinking the requirement to build an extra C source file would be kind of troublesome, but I suppose it should always work since we already have a build dependency on the devkitPro toolchain.

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 a pull request may close this issue.

4 participants