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

Update bindgen #292

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

hannesweisbach
Copy link

@hannesweisbach hannesweisbach commented Feb 16, 2025

Updating for a solution for issue rust-lang/rust-bindgen#2312 of rust-bindgen.

I'm in the process to generate bindings for softdevice 332, when I encountered the error

+ for s in s332
+ ./nrf-softdevice-gen/target/release/nrf-softdevice-gen ./softdevice/s332/headers ./nrf-softdevice-s332/src/bindings.rs
thread 'main' panicked at /home/hannesweisbach/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.55.1/src/ir/context.rs:838:9:
"ble_gap_evt_timeout_t_union_(unnamed_at__/tmp/ble_gap_h_1196_3)" is not a valid Ident
stack backtrace:

This SO post https://stackoverflow.com/questions/76443280/rust-bindgen-causes-a-is-not-a-valid-ident-error-on-build pointed me to rust-lang/rust-bindgen#2312.

The version bump solves the error above.

Updating to get solution for rust-lang/rust-bindgen#2312.

Signed-off-by: Hannes Weisbach <[email protected]>
@hannesweisbach
Copy link
Author

Edit: changed from 0.71.1 down to 0.70.0, because 0.71.1 would generate stray "unsafe" keywords. Not sure what's up. 0.70.0 seems to work though.

@alexmoon
Copy link
Contributor

It looks like bindings generated with the new version of bindgen are not backwards-compatible. If you want to update bindgen, please run gen.sh so that the repository remains consistent and reproducible and make sure that all tests pass (by running ci.sh). Ideally you can get it to work by just adjusting the bindgen configuration.

@hannesweisbach
Copy link
Author

hannesweisbach commented Feb 17, 2025

Thanks for your feedback!

So not quite as straight-forward as I hoped. I want to mention that I get these types of warnings when I run gen.sh:

+ ./nrf-softdevice-gen/target/release/nrf-softdevice-gen ./softdevice/s140/headers ./nrf-softdevice-s140/src/bindings.rs
Warning: can't set `imports_granularity = Module`, unstable features are only available in nightly channel.
Warning: can't set `group_imports = StdExternalCrate`, unstable features are only available in nightly channel.
Warning: can't set `imports_granularity = Module`, unstable features are only available in nightly channel.
Warning: can't set `group_imports = StdExternalCrate`, unstable features are only available in nightly channel.
+ rustfmt ./nrf-softdevice-s140/src/bindings.rs
Warning: can't set `imports_granularity = Module`, unstable features are only available in nightly channel.
Warning: can't set `group_imports = StdExternalCrate`, unstable features are only available in nightly channel.
+ cd nrf-softdevice-s140
+ cargo build --target thumbv7em-none-eabihf

Do you know if they are expected/benign?

The new generated bindings do not have as nice formatted docs as the old ones. Instead of multiple lines there is just one #[doc]macro with embedded newlines. There is also an issue open rust-lang/rust-bindgen#3073 regarding this issue. As a stop-gap there is a set of rustfmt options to re-format the comments in multiline but with /// instead of #[doc]. Would that be acceptable?

The new bindgen version introduced _bitfield_align_1-members in structs which required to be initialized manually, so I added initializers in nrf-softdevice/ and examples/. ./ci.sh executes now without errors. I'm still investigating whether there is a setting for bindgen to control generation of these members.

@alexmoon
Copy link
Contributor

Do you know if they are expected/benign?

Yes, those warnings are fine. We use some rustfmt options that are only on nightly.

The new generated bindings do not have as nice formatted docs as the old ones. Instead of multiple lines there is just one #[doc]macro with embedded newlines. There is also an issue open rust-lang/rust-bindgen#3073 regarding this issue. As a stop-gap there is a set of rustfmt options to re-format the comments in multiline but with /// instead of #[doc]. Would that be acceptable?

#[doc] macros with embedded newlines are exactly identical to multiple #[doc] macros as far as rustdoc is concerned, so that's fine.

The new bindgen version introduced _bitfield_align_1-members in structs which required to be initialized manually, so I added initializers in nrf-softdevice/ and examples/. ./ci.sh executes now without errors. I'm still investigating whether there is a setting for bindgen to control generation of these members.

This is more of a problem, because it breaks existing code. In most cases it's also useless because the bitfield is byte aligned anyway. Let me know what you find.

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 this pull request may close these issues.

2 participants