This is an archive of the discontinued LLVM Phabricator instance.

[clang] [Driver] Support multiple configuration files
ClosedPublic

Authored by mgorny on Sep 20 2022, 2:25 AM.

Details

Summary

Support specifying multiple configuration files via multiple --config
options. When multiple files are specified, the options from subsequent
files are appended to the options from the initial file.

While at it, remove the incorrect assertion about CfgFileName being
non-empty. It can be empty if --config "" is passed, and it makes
sense to report it as non-existing file rather than crash.

Diff Detail

Event Timeline

mgorny created this revision.Sep 20 2022, 2:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 20 2022, 2:25 AM
Herald added a subscriber: StephenFan. · View Herald Transcript
mgorny requested review of this revision.Sep 20 2022, 2:25 AM

What about the case, when executable file has a target prefix (so implicit config file may be used) and the option --config is specified in command line? Now this case becomes possible and we should either reject it (because explicit config files have precedence) or support it (because configuration is built up of a few files). I would be inclined to the first variant but there could be use cases for the other. In any case it worth to be documented.

clang/lib/Driver/Driver.cpp
989–990

Maybe rename it to loadConfigFiles?

What about the case, when executable file has a target prefix (so implicit config file may be used) and the option --config is specified in command line? Now this case becomes possible and we should either reject it (because explicit config files have precedence)

Rejecting sounds like a bad idea. We should certainly allow --config to be used, independently of whether prefixed executable is to be used and whether the vendor supplies default configs.

or support it (because configuration is built up of a few files). I would be inclined to the first variant but there could be use cases for the other. In any case it worth to be documented.

Yeah, it could make sense to use default configs unconditionally now. In the end, we have --no-default-config to override that. I'll update the patch to do that.

clang/lib/Driver/Driver.cpp
989–990

Will do.

mgorny marked an inline comment as done.Sep 21 2022, 12:28 AM
mgorny updated this revision to Diff 461803.Sep 21 2022, 12:31 AM

Combining --config and default configuration files is now explicitly allowed.

Adjust the code to load default configs before explicitly specified files. Update tests to cover combining --config with defaults. Update the documentation.

For the record, I'm working on support for loading defaults from multiple files in a separate patch.

sepavloff added inline comments.Sep 21 2022, 2:43 AM
clang/lib/Driver/Driver.cpp
1061

Does it mean that executable without prefix won't load implicit config file? For example, execution of clang won't trigger loading clang.cfg or default.cfg?

mgorny added inline comments.
clang/lib/Driver/Driver.cpp
1061

Yes, this patch doesn't change how defaults are loaded. This is tackled separately in D134337.

sepavloff accepted this revision.Sep 21 2022, 4:05 AM

LGTM.

Thanks!

This revision is now accepted and ready to land.Sep 21 2022, 4:05 AM
This revision was landed with ongoing or failed builds.Sep 21 2022, 4:14 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2022, 4:14 AM

Thanks a lot!