search

rust3ds / ctru-rs

Rust wrapper for libctru
https://rust3ds.github.io/ctru-rs/
Other
117 stars 17 forks source link

Expose static inline functions to bindings #123

Closed DeltaF1 closed 11 months ago

DeltaF1 commented 1 year ago

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

https://github.com/rust-lang/rust-bindgen/pull/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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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.