Split BindgenOptions

[pvdrz]

This PR splits all the fields inside BindgenOptions struct into two new structs:

• inputs: BindgenInputs: Which holds all the command line arguments and implements Clone.
• state: BindgenState: Which holds the remaining fields of BindgenOptions and does not implement Clone.

This change would allow bindgen to preserve its initial configuration and run again if required (which would help solve #1090).

It would also ease a transition to a newer version of clap (#1873) and is somewhat related to #2132.

Additionally BindgenOptions::inputs is exposed immutably, meaning that it is not possible to accidentally change the user inputs.

cc @emilio @amanjeev

Blocked by: #2271

[bors-servo]

☔ The latest upstream changes (presumably 8b29355) made this pull request unmergeable. Please resolve the merge conflicts.

[bors-servo]

☔ The latest upstream changes (presumably 61636e9) made this pull request unmergeable. Please resolve the merge conflicts.

[bors-servo]

☔ The latest upstream changes (presumably 0d805d7) made this pull request unmergeable. Please resolve the merge conflicts.

[bors-servo]

☔ The latest upstream changes (presumably 86f059f) made this pull request unmergeable. Please resolve the merge conflicts.

[emilio]

So this is a pretty massive change, and I'm not sure it's warranted.

From a quick look, the things in BindgenOptions that can't be cloned are input_unsaved_files and the ParseCallbacks.

input_unsaved_files seems rather trivial to get rid of (just a matter of generating the UnsavedFile list in generate rather than the caller unless I'm missing something).

The ParseCallbacks seem like similarly could be passed separately from BindgenOptions as an extra argument to generate etc, unless I'm missing something.

That leaves the weird mutations that we do on the clang flags and so. Those don't seem impossible to remove (we could make the options immutable I think). But also, not sure we'd need to. As long as we clone the options before calling generate(), we can call generate() again with the same set of options and it should work.

That is to say, I think this can be done in a much less intrusive way, wdyt?

[pvdrz]

I agree that it would be possible to preserve the user inputs by just cloning BindgenOptions before calling generate. But at the same time, I believe this change makes clearer when a value was set by the user with some intent vs when bindgen modified one value due to its internal logic.

At the same time, keeping all the CLI inputs in a single type makes easier moving to things like clap 3 with its derive macro. Which would ease adding new flags and keeping the documentation and names consistent as they would be a bunch of field attributes instead of being on a different file.

I'm not opposed to just cloning things before they are changed. I just think this makes the code a bit more organized while only adding the extra burden of thinking if something is part of the inputs or part of the state to decide where to find it. At the end most of the changes done to the uses of BindgenOptions were reduced to doing

+ foo.options()
- foo.inputs()

[bors-servo]

☔ The latest upstream changes (presumably #2278) made this pull request unmergeable. Please resolve the merge conflicts.

[pvdrz]

Closed in favor of #2285

[divagant-martian]

Sad to this closed, seemed like a good code quality improvement