Skip to content

Fix rust_bindgen compile flag filtering#3363

Merged
UebelAndre merged 2 commits intobazelbuild:mainfrom
bmclarnon:bindgen
Mar 18, 2025
Merged

Fix rust_bindgen compile flag filtering#3363
UebelAndre merged 2 commits intobazelbuild:mainfrom
bmclarnon:bindgen

Conversation

@bmclarnon
Copy link
Copy Markdown
Contributor

All flags were treated as accepting a parameter, causing any argument following a parameterless flag (e.g. -no-standard-includes) to be passed through. This causes bindgen failures if the next argument isn't supported by clang.

-nostd* flags has also been expanded using
https://clang.llvm.org/docs/ClangCommandLineReference.html to make it clearer that these flags are parameterless. Since these flags resulted in a prefix match, not a full match, they wouldn't set open_arg = True and therefore wouldn't cause issues, but this behavior is unnecessarily subtle now that there are two lists.

Fixes #3359

All flags were treated as accepting a parameter, causing any argument
following a parameterless flag (e.g. `-no-standard-includes`) to be
passed through. This causes bindgen failures if the next argument isn't
supported by clang.

`-nostd*` flags has also been expanded using
https://clang.llvm.org/docs/ClangCommandLineReference.html to make it
clearer that these flags are parameterless. Since these flags resulted
in a prefix match, not a full match, they wouldn't set `open_arg = True`
and therefore wouldn't cause issues, but this behavior is unnecessarily
subtle now that there are two lists.

Fixes #3359
Copy link
Copy Markdown
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@UebelAndre UebelAndre enabled auto-merge March 18, 2025 16:42
@UebelAndre UebelAndre added this pull request to the merge queue Mar 18, 2025
@UebelAndre UebelAndre removed this pull request from the merge queue due to a manual request Mar 18, 2025
@UebelAndre
Copy link
Copy Markdown
Collaborator

@bmclarnon looks like buildifier needs to run.

@bmclarnon
Copy link
Copy Markdown
Contributor Author

@UebelAndre Oops, sorry I missed that! And thanks for the really quick review!

@UebelAndre UebelAndre added this pull request to the merge queue Mar 18, 2025
Merged via the queue into bazelbuild:main with commit 633e56d Mar 18, 2025
2 checks passed
@bmclarnon bmclarnon deleted the bindgen branch March 19, 2025 17:08
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.

rust_bindgen incorrectly filters non-clang args

2 participants