This is an archive of the discontinued LLVM Phabricator instance.

Keep configuration file search directories in ExpansionContext. NFC
ClosedPublic

Authored by sepavloff on Oct 7 2022, 4:17 AM.

Details

Summary

Class ExpansionContext encapsulates options for search and expansion of
response files, including configuration files. With this change the
directories which are searched for configuration files are also stored
in ExpansionContext.

Diff Detail

Event Timeline

sepavloff created this revision.Oct 7 2022, 4:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2022, 4:17 AM
sepavloff requested review of this revision.Oct 7 2022, 4:17 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 7 2022, 4:17 AM
mgorny added inline comments.Oct 14 2022, 10:43 AM
clang/lib/Driver/Driver.cpp
1020

I don't really understand the purpose of this change, or how it's related to moving search dirs.

mgorny added inline comments.Oct 14 2022, 10:46 AM
clang/lib/Driver/Driver.cpp
1020

Oh, it's because of ExpCtx.findConfigFile() API.

mgorny added inline comments.Oct 16 2022, 11:48 AM
llvm/include/llvm/Support/CommandLine.h
2197

I'm sorry if I'm missing something obvious but is there a specific reason you're using std::string here when pretty much all prior call sites were using SmallString?

sepavloff updated this revision to Diff 468137.Oct 17 2022, 1:52 AM

Change parameter type of findConfigFile, rebased

sepavloff added inline comments.Oct 17 2022, 1:54 AM
llvm/include/llvm/Support/CommandLine.h
2197

Changed type of this parameter to base of SmallString.

mgorny accepted this revision.Oct 17 2022, 5:36 AM

I haven't tested it but eyeball-review, it looks neat. Thanks!

This revision is now accepted and ready to land.Oct 17 2022, 5:36 AM

I am not familiar with ExpansionContext. Is there any behavior difference which can be demonstrated by a test?

I am not familiar with ExpansionContext. Is there any behavior difference which can be demonstrated by a test?

This is an NFC test, it does not change behavior. However moving these properties into ExpansionContext provides access to the search directories in the functions expandResponseFile(s) and allows more natural implementation of the functionality of D133325.

This revision was landed with ongoing or failed builds.Oct 19 2022, 3:20 AM
This revision was automatically updated to reflect the committed changes.