compile: Add minimal bindgen allowlist_function() support
#20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
By default `bindgen` emits bindings for every item in the provided header, which are typically platform- and architecture-specific defines from `stdint.h` and `stdbool.h`. This is noisy and unnecessary as the function signatures typically don't use any of these. By allowing the user to set a custom list of function names, bindgen only emits bindings for said functions and any type(s) they (recursively) reference. Unfortunately `bindgen::Builder` cannot be cloned and is moved by every call, which doesn't allow us to call `.generate()` on it multiple times (and neatly add the `header()` without updating the underlying struct). In contrast, `Config::compile()` here borrows `self` and requires the builder struct to be reusable across possibly multiple calls on the same `Config`. For this reason a minimal struct has been added that currently only supports `allowlist_function()`.
Twinklebear
approved these changes
Jun 29, 2022
There was a problem hiding this comment.
Thanks @MarijnS95 , this looks great!
It'd be a bit nicer if the whole bindgen options could be cloned since then we wouldn't need to do any work to add support for specifying other ones, but this will work well to get around that limitation. We can add other bindgen options as needed for folks through this same route
I think CI should pass after I merge this to master since then it'll pick up your commit removing aligned_alloc
MarijnS95
added a commit
to Traverse-Research/ispc-downsampler
that referenced
this pull request
Jun 29, 2022
`ispc_compile` can now be configured to omit irrelevant bindings from the bindings file and leave only the function(s) in place that are relevant to interface with the ISPC kernel (Twinklebear/ispc-rs#20). This makes the file less busy/noisy, and it won't change when regenerating across OS-es anymore.
MarijnS95
added a commit
to Traverse-Research/ispc-downsampler
that referenced
this pull request
Jun 30, 2022
`ispc_compile` can now be configured to omit irrelevant bindings from the bindings file and leave only the function(s) in place that are relevant to interface with the ISPC kernel (Twinklebear/ispc-rs#20). This makes the file less busy/noisy, and it won't change when regenerating across OS-es anymore.
MarijnS95
added a commit
to Traverse-Research/ispc-downsampler
that referenced
this pull request
Jun 30, 2022
`ispc_compile` can now be configured to omit irrelevant bindings from the bindings file and leave only the function(s) in place that are relevant to interface with the ISPC kernel (Twinklebear/ispc-rs#20). This makes the file less busy/noisy, and it won't change when regenerating across OS-es anymore.
MarijnS95
added a commit
to Traverse-Research/ispc-downsampler
that referenced
this pull request
Jun 30, 2022
`ispc_compile` can now be configured to omit irrelevant bindings from the bindings file and leave only the function(s) in place that are relevant to interface with the ISPC kernel (Twinklebear/ispc-rs#20). This makes the file less busy/noisy, and it won't change when regenerating across OS-es anymore.
MarijnS95
added a commit
to Traverse-Research/intel-tex-rs-2
that referenced
this pull request
Jun 30, 2022
`ispc_compile` can now be configured to omit irrelevant bindings from the bindings file and leave only the function(s) in place that are relevant to interface with the ISPC kernel (Twinklebear/ispc-rs#20). This makes the file less busy/noisy, and it won't change when regenerating across OS-es anymore.
MarijnS95
added a commit
to Traverse-Research/ispc-downsampler
that referenced
this pull request
Jun 30, 2022
`ispc_compile` can now be configured to omit irrelevant bindings from the bindings file and leave only the function(s) in place that are relevant to interface with the ISPC kernel (Twinklebear/ispc-rs#20). This makes the file less busy/noisy, and it won't change when regenerating across OS-es anymore.
MarijnS95
added a commit
to Traverse-Research/intel-tex-rs-2
that referenced
this pull request
Jun 30, 2022
* Upgrade to `ispc 1.1.0` with cleaned up bindings file `ispc_compile` can now be configured to omit irrelevant bindings from the bindings file and leave only the function(s) in place that are relevant to interface with the ISPC kernel (Twinklebear/ispc-rs#20). This makes the file less busy/noisy, and it won't change when regenerating across OS-es anymore. * Edition 2018 cleanup
|
rust-lang/rust-bindgen#2302 looks like this just got implemented, so we might improve the situation soon when a newer |
MarijnS95
added a commit
to MarijnS95/ispc-rs
that referenced
this pull request
Oct 20, 2022
As announced earlier in Twinklebear#20 when introducing `BindgenOptions`, upstream `bindgen` didn't yet allow cloning nor copying their `Builder` struct, limiting it to generate a single binding per `Builder` object. This doesn't fit with ispc-rs's `Config` struct which explicitly allows multiple libraries to be compiled using (partially) the same base configuration (barring any side-effects from `fn compile(&mut self, lib)` updating various fields depending on `lib`). This issue has since been addressed upstream, and now allows us to move and clone the `bindgen::Builder` around instead of implementing a very limited subset of config fields and manually reconstructing the builder every time.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By default
bindgenemits bindings for every item in the provided header, which are typically platform- and architecture-specific defines fromstdint.handstdbool.h. This is noisy and unnecessary as the function signatures typically don't use any of these.By allowing the user to set a custom list of function names, bindgen only emits bindings for said functions and any type(s) they (recursively) reference.
Unfortunately
bindgen::Buildercannot be cloned and is moved by every call, which doesn't allow us to call.generate()on it multiple times (and neatly add theheader()without updating the underlying struct). In contrast,Config::compile()here borrowsselfand requires the builder struct to be reusable across possibly multiple calls on the sameConfig. For this reason a minimal struct has been added that currently only supportsallowlist_function().