sepavloff created this revision.
sepavloff added reviewers: rjmccall, kadircet, jackoalan, thakis, rnk.
Herald added a subscriber: hiraditya.
Herald added a project: All.
sepavloff requested review of this revision.
Herald added subscribers: llvm-commits, MaskRay.
Herald added projects: clang, LLVM.
Functions that implement expansion of response and config files depend
on many options, which are passes as arguments. Extending the expansion
requires new options, it in turn causes changing calls in various places
making them even more bulky.
This change introduces a class ExpansionContext, which represents set of
options that control the expansion. Its methods implements expansion of
responce files including config files. It makes extending the expansion
easier.
No functional changes.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D132379
Files:
clang/lib/Driver/Driver.cpp
clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
clang/tools/driver/driver.cpp
llvm/include/llvm/Support/CommandLine.h
llvm/lib/Support/CommandLine.cpp
llvm/unittests/Support/CommandLineTest.cpp
Index: llvm/unittests/Support/CommandLineTest.cpp
===================================================================
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -870,9 +870,9 @@
// Expand response files.
llvm::BumpPtrAllocator A;
llvm::StringSaver Saver(A);
- ASSERT_TRUE(llvm::cl::ExpandResponseFiles(
- Saver, llvm::cl::TokenizeGNUCommandLine, Argv, false, true, false,
- /*CurrentDir=*/StringRef(TestRoot), FS));
+ llvm::cl::ExpansionContext ECtx(Saver, llvm::cl::TokenizeGNUCommandLine, &FS);
+ ECtx.setCurrentDir(TestRoot).setRelativeNames(true);
+ ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
EXPECT_THAT(Argv, testing::Pointwise(
StringEquality(),
{"test/test", "-flag_1", "-option_1", "-option_2",
@@ -933,9 +933,9 @@
#else
cl::TokenizerCallback Tokenizer = cl::TokenizeGNUCommandLine;
#endif
- ASSERT_FALSE(
- cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, false,
- /*CurrentDir=*/llvm::StringRef(TestRoot), FS));
+ llvm::cl::ExpansionContext ECtx(Saver, Tokenizer, &FS);
+ ECtx.setCurrentDir(TestRoot);
+ ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
EXPECT_THAT(Argv,
testing::Pointwise(StringEquality(),
@@ -972,9 +972,9 @@
BumpPtrAllocator A;
StringSaver Saver(A);
- ASSERT_FALSE(cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv,
- false, false, false,
- /*CurrentDir=*/StringRef(TestRoot), FS));
+ llvm::cl::ExpansionContext ECtx(Saver, cl::TokenizeGNUCommandLine, &FS);
+ ECtx.setCurrentDir(TestRoot);
+ ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
// ASSERT instead of EXPECT to prevent potential out-of-bounds access.
ASSERT_EQ(Argv.size(), 1 + NON_RSP_AT_ARGS + 2);
@@ -1007,9 +1007,9 @@
BumpPtrAllocator A;
StringSaver Saver(A);
- ASSERT_TRUE(cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv,
- false, true, false,
- /*CurrentDir=*/StringRef(TestRoot), FS));
+ llvm::cl::ExpansionContext ECtx(Saver, cl::TokenizeGNUCommandLine, &FS);
+ ECtx.setCurrentDir(TestRoot).setRelativeNames(true);
+ ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
EXPECT_THAT(Argv,
testing::Pointwise(StringEquality(), {"test/test", "-flag"}));
}
@@ -1027,9 +1027,9 @@
SmallVector<const char *, 2> Argv = {"clang", "@eols.rsp"};
BumpPtrAllocator A;
StringSaver Saver(A);
- ASSERT_TRUE(cl::ExpandResponseFiles(Saver, cl::TokenizeWindowsCommandLine,
- Argv, true, true, false,
- /*CurrentDir=*/StringRef(TestRoot), FS));
+ llvm::cl::ExpansionContext ECtx(Saver, cl::TokenizeWindowsCommandLine, &FS);
+ ECtx.setCurrentDir(TestRoot).setMarkEOLs(true).setRelativeNames(true);
+ ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
const char *Expected[] = {"clang", "-Xclang", "-Wno-whatever", nullptr,
"input.cpp"};
ASSERT_EQ(array_lengthof(Expected), Argv.size());
@@ -1126,7 +1126,8 @@
llvm::BumpPtrAllocator A;
llvm::StringSaver Saver(A);
- bool Result = llvm::cl::readConfigFile(ConfigFile.path(), Saver, Argv);
+ llvm::cl::ExpansionContext ECtx(Saver);
+ bool Result = ECtx.readConfigFile(ConfigFile.path(), Argv);
EXPECT_TRUE(Result);
EXPECT_EQ(Argv.size(), 13U);
Index: llvm/lib/Support/CommandLine.cpp
===================================================================
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1153,15 +1153,12 @@
}
// FName must be an absolute path.
-static llvm::Error ExpandResponseFile(StringRef FName, StringSaver &Saver,
- TokenizerCallback Tokenizer,
- SmallVectorImpl<const char *> &NewArgv,
- bool MarkEOLs, bool RelativeNames,
- bool ExpandBasePath,
- llvm::vfs::FileSystem &FS) {
+llvm::Error
+ExpansionContext::expandResponseFile(StringRef FName,
+ SmallVectorImpl<const char *> &NewArgv) {
assert(sys::path::is_absolute(FName));
llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> MemBufOrErr =
- FS.getBufferForFile(FName);
+ FS->getBufferForFile(FName);
if (!MemBufOrErr)
return llvm::errorCodeToError(MemBufOrErr.getError());
MemoryBuffer &MemBuf = *MemBufOrErr.get();
@@ -1196,7 +1193,7 @@
continue;
// Substitute <CFGDIR> with the file's base path.
- if (ExpandBasePath)
+ if (InConfigFile)
ExpandBasePaths(BasePath, Saver, Arg);
// Skip non-rsp file arguments.
@@ -1219,11 +1216,8 @@
/// Expand response files on a command line recursively using the given
/// StringSaver and tokenization strategy.
-bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
- SmallVectorImpl<const char *> &Argv, bool MarkEOLs,
- bool RelativeNames, bool ExpandBasePath,
- llvm::Optional<llvm::StringRef> CurrentDir,
- llvm::vfs::FileSystem &FS) {
+bool ExpansionContext::expandResponseFiles(
+ SmallVectorImpl<const char *> &Argv) {
bool AllExpanded = true;
struct ResponseFileRecord {
std::string File;
@@ -1263,22 +1257,23 @@
// Note that CurrentDir is only used for top-level rsp files, the rest will
// always have an absolute path deduced from the containing file.
SmallString<128> CurrDir;
- if (llvm::sys::path::is_relative(FName)) {
- if (!CurrentDir)
- llvm::sys::fs::current_path(CurrDir);
+ if (llvm::sys::path::is_relative(FName) &&
+ (!InConfigFile || llvm::sys::path::has_parent_path(FName))) {
+ if (CurrentDir.empty())
+ CurrDir.assign(*FS->getCurrentWorkingDirectory());
else
- CurrDir = *CurrentDir;
+ CurrDir = CurrentDir;
llvm::sys::path::append(CurrDir, FName);
FName = CurrDir.c_str();
}
- auto IsEquivalent = [FName, &FS](const ResponseFileRecord &RFile) {
- llvm::ErrorOr<llvm::vfs::Status> LHS = FS.status(FName);
+ auto IsEquivalent = [FName, this](const ResponseFileRecord &RFile) {
+ llvm::ErrorOr<llvm::vfs::Status> LHS = FS->status(FName);
if (!LHS) {
// TODO: The error should be propagated up the stack.
llvm::consumeError(llvm::errorCodeToError(LHS.getError()));
return false;
}
- llvm::ErrorOr<llvm::vfs::Status> RHS = FS.status(RFile.File);
+ llvm::ErrorOr<llvm::vfs::Status> RHS = FS->status(RFile.File);
if (!RHS) {
// TODO: The error should be propagated up the stack.
llvm::consumeError(llvm::errorCodeToError(RHS.getError()));
@@ -1299,9 +1294,7 @@
// Replace this response file argument with the tokenization of its
// contents. Nested response files are expanded in subsequent iterations.
SmallVector<const char *, 0> ExpandedArgv;
- if (llvm::Error Err =
- ExpandResponseFile(FName, Saver, Tokenizer, ExpandedArgv, MarkEOLs,
- RelativeNames, ExpandBasePath, FS)) {
+ if (llvm::Error Err = expandResponseFile(FName, ExpandedArgv)) {
// We couldn't read this file, so we leave it in the argument stream and
// move on.
// TODO: The error should be propagated up the stack.
@@ -1331,15 +1324,6 @@
return AllExpanded;
}
-bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
- SmallVectorImpl<const char *> &Argv, bool MarkEOLs,
- bool RelativeNames, bool ExpandBasePath,
- llvm::Optional<StringRef> CurrentDir) {
- return ExpandResponseFiles(Saver, std::move(Tokenizer), Argv, MarkEOLs,
- RelativeNames, ExpandBasePath,
- std::move(CurrentDir), *vfs::getRealFileSystem());
-}
-
bool cl::expandResponseFiles(int Argc, const char *const *Argv,
const char *EnvVar, StringSaver &Saver,
SmallVectorImpl<const char *> &NewArgv) {
@@ -1353,28 +1337,30 @@
// Command line options can override the environment variable.
NewArgv.append(Argv + 1, Argv + Argc);
- return ExpandResponseFiles(Saver, Tokenize, NewArgv);
+ ExpansionContext ECtx(Saver, Tokenize);
+ return ECtx.expandResponseFiles(NewArgv);
}
-bool cl::readConfigFile(StringRef CfgFile, StringSaver &Saver,
- SmallVectorImpl<const char *> &Argv) {
+ExpansionContext::ExpansionContext(StringSaver &S, TokenizerCallback T,
+ llvm::vfs::FileSystem *FS)
+ : Saver(S), Tokenizer(T), FS(FS ? FS : vfs::getRealFileSystem().get()) {}
+
+bool ExpansionContext::readConfigFile(StringRef CfgFile,
+ SmallVectorImpl<const char *> &Argv) {
SmallString<128> AbsPath;
if (sys::path::is_relative(CfgFile)) {
- llvm::sys::fs::current_path(AbsPath);
+ AbsPath.assign(*FS->getCurrentWorkingDirectory());
llvm::sys::path::append(AbsPath, CfgFile);
CfgFile = AbsPath.str();
}
- if (llvm::Error Err = ExpandResponseFile(
- CfgFile, Saver, cl::tokenizeConfigFile, Argv,
- /*MarkEOLs=*/false, /*RelativeNames=*/true, /*ExpandBasePath=*/true,
- *llvm::vfs::getRealFileSystem())) {
+ InConfigFile = true;
+ RelativeNames = true;
+ if (llvm::Error Err = expandResponseFile(CfgFile, Argv)) {
// TODO: The error should be propagated up the stack.
llvm::consumeError(std::move(Err));
return false;
}
- return ExpandResponseFiles(Saver, cl::tokenizeConfigFile, Argv,
- /*MarkEOLs=*/false, /*RelativeNames=*/true,
- /*ExpandBasePath=*/true, llvm::None);
+ return expandResponseFiles(Argv);
}
static void initCommonOptions();
@@ -1433,10 +1419,10 @@
SmallVector<const char *, 20> newArgv(argv, argv + argc);
BumpPtrAllocator A;
StringSaver Saver(A);
- ExpandResponseFiles(Saver,
- Triple(sys::getProcessTriple()).isOSWindows() ?
- cl::TokenizeWindowsCommandLine : cl::TokenizeGNUCommandLine,
- newArgv);
+ ExpansionContext ECtx(Saver, Triple(sys::getProcessTriple()).isOSWindows()
+ ? cl::TokenizeWindowsCommandLine
+ : cl::TokenizeGNUCommandLine);
+ ECtx.expandResponseFiles(newArgv);
argv = &newArgv[0];
argc = static_cast<int>(newArgv.size());
Index: llvm/include/llvm/Support/CommandLine.h
===================================================================
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -2065,55 +2065,87 @@
SmallVectorImpl<const char *> &NewArgv,
bool MarkEOLs = false);
-/// Reads command line options from the given configuration file.
-///
-/// \param [in] CfgFileName Path to configuration file.
-/// \param [in] Saver Objects that saves allocated strings.
-/// \param [out] Argv Array to which the read options are added.
-/// \return true if the file was successfully read.
-///
-/// It reads content of the specified file, tokenizes it and expands "@file"
-/// commands resolving file names in them relative to the directory where
-/// CfgFilename resides. It also expands "<CFGDIR>" to the base path of the
-/// current config file.
-///
-bool readConfigFile(StringRef CfgFileName, StringSaver &Saver,
- SmallVectorImpl<const char *> &Argv);
-
-/// Expand response files on a command line recursively using the given
-/// StringSaver and tokenization strategy. Argv should contain the command line
-/// before expansion and will be modified in place. If requested, Argv will
-/// also be populated with nullptrs indicating where each response file line
-/// ends, which is useful for the "/link" argument that needs to consume all
-/// remaining arguments only until the next end of line, when in a response
-/// file.
-///
-/// \param [in] Saver Delegates back to the caller for saving parsed strings.
-/// \param [in] Tokenizer Tokenization strategy. Typically Unix or Windows.
-/// \param [in,out] Argv Command line into which to expand response files.
-/// \param [in] MarkEOLs Mark end of lines and the end of the response file
-/// with nullptrs in the Argv vector.
-/// \param [in] RelativeNames true if names of nested response files must be
-/// resolved relative to including file.
-/// \param [in] ExpandBasePath If true, "<CFGDIR>" expands to the base path of
-/// the current response file.
-/// \param [in] FS File system used for all file access when running the tool.
-/// \param [in] CurrentDir Path used to resolve relative rsp files. If set to
-/// None, process' cwd is used instead.
-/// \return true if all @files were expanded successfully or there were none.
-bool ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
- SmallVectorImpl<const char *> &Argv, bool MarkEOLs,
- bool RelativeNames, bool ExpandBasePath,
- llvm::Optional<llvm::StringRef> CurrentDir,
- llvm::vfs::FileSystem &FS);
-
-/// An overload of ExpandResponseFiles() that uses
-/// llvm::vfs::getRealFileSystem().
-bool ExpandResponseFiles(
- StringSaver &Saver, TokenizerCallback Tokenizer,
- SmallVectorImpl<const char *> &Argv, bool MarkEOLs = false,
- bool RelativeNames = false, bool ExpandBasePath = false,
- llvm::Optional<llvm::StringRef> CurrentDir = llvm::None);
+/// Contains options that controls response file expansion.
+class ExpansionContext {
+ /// Delegates back to the caller for saving parsed strings.
+ StringSaver &Saver;
+
+ /// Tokenization strategy. Typically Unix or Windows.
+ TokenizerCallback Tokenizer;
+
+ /// File system used for all file access when running the expansion.
+ llvm::vfs::FileSystem *FS;
+
+ /// Path used to resolve relative rsp files. If empty, process' cwd is
+ /// used instead.
+ StringRef CurrentDir;
+
+ /// True if names of nested response files must be resolved relative to
+ /// including file.
+ bool RelativeNames = false;
+
+ /// If true, mark end of lines and the end of the response file with nullptrs
+ /// in the Argv vector.
+ bool MarkEOLs = false;
+
+ /// If true, body of config file is expanded.
+ bool InConfigFile = false;
+
+ llvm::Error expandResponseFile(StringRef FName,
+ SmallVectorImpl<const char *> &NewArgv);
+
+public:
+ ExpansionContext(StringSaver &S, TokenizerCallback T,
+ llvm::vfs::FileSystem *FS = nullptr);
+
+ bool getMarkEOLs() const { return MarkEOLs; }
+ ExpansionContext &setMarkEOLs(bool X) {
+ MarkEOLs = X;
+ return *this;
+ }
+
+ bool getRelativeNames() const { return RelativeNames; }
+ ExpansionContext &setRelativeNames(bool X) {
+ RelativeNames = X;
+ return *this;
+ }
+
+ StringRef getCurrentDir() const { return CurrentDir; }
+ ExpansionContext &setCurrentDir(StringRef X) {
+ CurrentDir = X;
+ return *this;
+ }
+
+ /// Reads command line options from the given configuration file.
+ ///
+ /// \param [in] CfgFile Path to configuration file.
+ /// \param [out] Argv Array to which the read options are added.
+ /// \return true if the file was successfully read.
+ ///
+ /// It reads content of the specified file, tokenizes it and expands "@file"
+ /// commands resolving file names in them relative to the directory where
+ /// CfgFilename resides. It also expands "<CFGDIR>" to the base path of the
+ /// current config file.
+ bool readConfigFile(StringRef CfgFile, SmallVectorImpl<const char *> &Argv);
+
+ bool expandResponseFiles(SmallVectorImpl<const char *> &Argv);
+
+ /// A convenience helper which concatenates the options specified by the
+ /// environment variable EnvVar and command line options, then expands
+ /// response files recursively.
+ /// \return true if all @files were expanded successfully or there were none.
+ bool expandResponseFiles(int Argc, const char *const *Argv,
+ const char *EnvVar,
+ SmallVectorImpl<const char *> &NewArgv);
+};
+
+/// A convenience helper which supports the typical use case of expansion
+/// function call.
+inline bool ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
+ SmallVectorImpl<const char *> &Argv) {
+ ExpansionContext ECtx(Saver, Tokenizer);
+ return ECtx.expandResponseFiles(Argv);
+}
/// A convenience helper which concatenates the options specified by the
/// environment variable EnvVar and command line options, then expands response
Index: clang/tools/driver/driver.cpp
===================================================================
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -309,8 +309,8 @@
llvm::BumpPtrAllocator A;
llvm::StringSaver Saver(A);
- llvm::cl::ExpandResponseFiles(Saver, &llvm::cl::TokenizeGNUCommandLine, ArgV,
- /*MarkEOLs=*/false);
+ llvm::cl::ExpansionContext ECtx(Saver, llvm::cl::TokenizeGNUCommandLine);
+ ECtx.expandResponseFiles(ArgV);
StringRef Tool = ArgV[1];
void *GetExecutablePathVP = (void *)(intptr_t)GetExecutablePath;
if (Tool == "-cc1")
@@ -373,7 +373,8 @@
if (MarkEOLs && Args.size() > 1 && StringRef(Args[1]).startswith("-cc1"))
MarkEOLs = false;
- llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Args, MarkEOLs);
+ llvm::cl::ExpansionContext ECtx(Saver, Tokenizer);
+ ECtx.setMarkEOLs(MarkEOLs).expandResponseFiles(Args);
// Handle -cc1 integrated tools, even if -cc1 was expanded from a response
// file.
Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -61,8 +61,8 @@
continue;
llvm::BumpPtrAllocator Alloc;
llvm::StringSaver Saver(Alloc);
- llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, false,
- llvm::StringRef(Cmd.Directory), *FS);
+ llvm::cl::ExpansionContext ECtx(Saver, Tokenizer, FS.get());
+ ECtx.setCurrentDir(Cmd.Directory).expandResponseFiles(Argv);
// Don't assign directly, Argv aliases CommandLine.
std::vector<std::string> ExpandedArgv(Argv.begin(), Argv.end());
Cmd.CommandLine = std::move(ExpandedArgv);
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -930,7 +930,8 @@
bool Driver::readConfigFile(StringRef FileName) {
// Try reading the given file.
SmallVector<const char *, 32> NewCfgArgs;
- if (!llvm::cl::readConfigFile(FileName, Saver, NewCfgArgs)) {
+ llvm::cl::ExpansionContext ExpCtx(Saver, llvm::cl::tokenizeConfigFile);
+ if (!ExpCtx.readConfigFile(FileName, NewCfgArgs)) {
Diag(diag::err_drv_cannot_read_config_file) << FileName;
return true;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
