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

Generate BindgenOptions and the Builder methods using macros #2473

Merged
merged 14 commits into from Apr 3, 2023

Conversation

pvdrz
Copy link
Contributor

@pvdrz pvdrz commented Mar 29, 2023

TODO:

  • Document the options! macro.
  • Try to allow optional commas between "fields" to avoid weird macro expansion errors.
  • Decide to introduce a trait to convert fields automatically to CLI args instead of having to pass a closure for every field.
  • Check the documentation of each field and method.
  • Try to refactor the "Regular expressions are supported" docstring from all the options using a RegexSet.

pvdrz added 10 commits March 29, 2023 17:04
This is done so the definition, default value and `Builder` methods for
each field of `BindgenOptions` are kept in the same region of code.

Before this change, adding (or modifying) a new option for `bindgen`
required:
- Updating the fields of the `BindgenOptions` type.
- Updating the `Default` implementation for `BindgenOptions`.
- Updating one or several `Builder` methods with proper documentation
  explaining the default value of the option.
- Updating the `Builder::command_line_flags` method.

Each one of these steps was done in a different place inside
`bindgen/lib.rs`.

With this change, all these 4 steps are done in the same place. This
should make less likely to have bugs.

Clearly using macros implies properly documenting how to use such macros
and makes adding new options a bit harder because most (all?) editors
are not able to format code and suggest completions inside macros.

This change also moves all the code related to setting `BindgenOptions`
to the new `bindgen/options.rs` file.
All the options that support regular expressions had the following
sentence in their documentation:

> Regular expressions are supported

This comment was factored out to the macro that documents options based
on `RegexSet`s.
This is done to avoid hard to detect macro parsing issues due to missing
commas.
This trait eases the conversion of `BindgenOptions` fields into CLI
args.

This commit also changes the `options` macro so the flag can be passed
to `as_args` instead of a closure.
@pvdrz pvdrz marked this pull request as ready for review March 30, 2023 23:19
@pvdrz pvdrz merged commit 040149b into rust-lang:main Apr 3, 2023
24 checks passed
@pvdrz pvdrz deleted the centralize-options branch April 3, 2023 15:51
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.

None yet

1 participant