Page MenuHomePhabricator

Add new driver mode for dumping compiler options
ClosedPublic

Authored by aaron.ballman on Apr 19 2018, 12:20 PM.

Details

Reviewers
rsmith
dlj
Summary

There are situations where an out-of-tree user may need to know information about what compiler options are used and what features/extensions may be available for a given invocation of the compiler. For instance, a tool may want to interrogate Clang so that it can emulate settings specific to the compilation without having to reverse engineer the Clang command line options or keep a custom mapping of language modes to features or extensions available in that mode.

This patch adds the ability to dump compiler option-related information to a JSON file. Specifically, it dumps the language options, codegen options, diagnostic options, and features/extensions lists -- however, this output could be extended to other information should it be useful (like target options, available attributes, etc). In order to support features and extensions, I moved them into a .def file so that we could build the various lists we care about from them without a significant increase in maintenance burden.

I selected JSON as the output because the data being output is serialized (as opposed to something human-writable like a config file), it's what our tool already groks, and it was simple to output to. If/when we get a JSON support library, I expect this code to be converted over to use that.

Our expectation is that the list of features and extensions will only ever be added to (because these are user-facing and removing one could break code), but that the other options may be added to, renamed, or removed between major releases (as opposed to point releases).

Diff Detail

Event Timeline

aaron.ballman created this revision.Apr 19 2018, 12:20 PM

The list of features/extensions seems okay; it's information which is already available through a stable interface (specifically, clang -E). JSON also seems fine as a representation.

Including the language/codegen/etc. options as-is doesn't make sense; we can, and often do, change the internal representation of the option structs. If the information is useful, we should provide a stable interface.

The list of features/extensions seems okay; it's information which is already available through a stable interface (specifically, clang -E). JSON also seems fine as a representation.

Including the language/codegen/etc. options as-is doesn't make sense; we can, and often do, change the internal representation of the option structs. If the information is useful, we should provide a stable interface.

For our needs, the features/extensions is the most important information. I'd be happy to split this patch into two parts: one for the features/extensions dump and another for extended information if the compiler options part remains contentious.

Our tool already has to parse the Clang command line to get much of this information, and that effectively is the stable interface to compiler options. However, every once in a while, some piece of configuration information is calculated from the command line and getting Clang's exact answer is useful. If any of those options we care about wind up being changed, there's a good chance we may need to change something on our end anyway, so breaking us is actually useful. Given the benefits of having implementation freedom for the options interfaces, I'd prefer to keep them unstable for this feature.

If any of those options we care about wind up being changed, there's a good chance we may need to change something on our end anyway, so breaking us is actually useful.

I'm not sure I follow. The language options have specific consequences you could check some other way, so they're "stable" in the sense that the same flags will produce the same result (e.g. LangOpts.FastMath represents whether FAST_MATH is defined). So it should be possible to provide the data somehow; I'm more concerned about taking names and enum values from LangOptions.def itself.

If any of those options we care about wind up being changed, there's a good chance we may need to change something on our end anyway, so breaking us is actually useful.

I'm not sure I follow. The language options have specific consequences you could check some other way, so they're "stable" in the sense that the same flags will produce the same result (e.g. LangOpts.FastMath represents whether FAST_MATH is defined). So it should be possible to provide the data somehow; I'm more concerned about taking names and enum values from LangOptions.def itself.

When a name or enum value from LangOptions.def changes, that generally signals some behavior may have changed (we don't often rename things just for the sake of renaming them, but that seems a less likely scenario) and we might want to react to that in our product. That's why the list of names from LangOptions.def directly is somewhat of a benefit (but it is not a design requirement).

How do you envision surfacing a list of language options that isn't tied to values from LangOptions.def itself? Would it still be tied in to the syntax in LangOptions.def (and others) so that there's not a substantial increase in maintenance burden when modifying options?

Updated to remove the contentious parts, though I still hope to add those back in.

lebedev.ri added inline comments.
lib/Frontend/FrontendActions.cpp
779

This is likely to do an allocation for each feature.
Maybe consider llvm::SmallString<64>

781

Similarly, you may want to explicitly use llvm::Twine here

791

same

aaron.ballman marked 3 inline comments as done.

Updated based on review feedback.

lib/Frontend/FrontendActions.cpp
779

64 bytes seemed a bit tight, so I went with 128 instead. Likely still a bit too small, but shouldn't be too bad.

lebedev.ri added inline comments.May 13 2018, 2:11 PM
lib/Frontend/FrontendActions.cpp
779

FixedString / FixedVector would be nice to have here.

hfinkel added inline comments.
test/Frontend/compiler-options-dump.cpp
3

You don't need -ffast-math here I presume.

15

cxx_range_for is both a feature and an extension?

aaron.ballman marked 3 inline comments as done.May 28 2018, 4:51 AM
aaron.ballman added inline comments.
test/Frontend/compiler-options-dump.cpp
3

Correct, I'll drop from the commit (or in the next patch).

15

Correct. The way we implement has_feature is to return true only if the -std line supports the feature, and has_extension will return true if __has_feature returns true or if the extension is enabled.

rsmith added a reviewer: dlj.May 29 2018, 5:11 PM

+@dlj, who had some more ambitious plans for expressing our configuration as JSON.

The part you have here seems fine. I share Eli's concerns about exposing our LangOptions with any kind of implied stability guarantee.

rsmith accepted this revision.May 29 2018, 5:13 PM
This revision is now accepted and ready to land.May 29 2018, 5:13 PM
aaron.ballman closed this revision.May 31 2018, 7:01 AM
aaron.ballman marked 4 inline comments as done.

Committed in r333653.

test/Frontend/compiler-options-dump.cpp
3

I dropped this from the commit.