This revision was automatically updated to reflect the committed changes.
Closed by commit rG6e491c48d6b9: [Support] Class for response file expansion
(NFC) (authored by sepavloff).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132379/new/
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/include/llvm/Support/StringSaver.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
@@ -869,10 +869,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(A, llvm::cl::TokenizeGNUCommandLine);
+ ECtx.setVFS(&FS).setCurrentDir(TestRoot).setRelativeNames(true);
+ ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
EXPECT_THAT(Argv, testing::Pointwise(
StringEquality(),
{"test/test", "-flag_1", "-option_1", "-option_2",
@@ -927,15 +926,14 @@
SmallVector<const char *, 4> Argv = {"test/test", SelfFileRef.c_str(),
"-option_3"};
BumpPtrAllocator A;
- StringSaver Saver(A);
#ifdef _WIN32
cl::TokenizerCallback Tokenizer = cl::TokenizeWindowsCommandLine;
#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(A, Tokenizer);
+ ECtx.setVFS(&FS).setCurrentDir(TestRoot);
+ ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
EXPECT_THAT(Argv,
testing::Pointwise(StringEquality(),
@@ -971,10 +969,9 @@
Argv.push_back(ResponseFileRef.c_str());
BumpPtrAllocator A;
- StringSaver Saver(A);
- ASSERT_FALSE(cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv,
- false, false, false,
- /*CurrentDir=*/StringRef(TestRoot), FS));
+ llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
+ ECtx.setVFS(&FS).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);
@@ -1006,10 +1003,9 @@
SmallVector<const char *, 2> Argv = {"test/test", "@dir/outer.rsp"};
BumpPtrAllocator A;
- StringSaver Saver(A);
- ASSERT_TRUE(cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv,
- false, true, false,
- /*CurrentDir=*/StringRef(TestRoot), FS));
+ llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
+ ECtx.setVFS(&FS).setCurrentDir(TestRoot).setRelativeNames(true);
+ ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
EXPECT_THAT(Argv,
testing::Pointwise(StringEquality(), {"test/test", "-flag"}));
}
@@ -1026,10 +1022,10 @@
MemoryBuffer::getMemBuffer("-Xclang -Wno-whatever\n input.cpp"));
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(A, cl::TokenizeWindowsCommandLine);
+ ECtx.setVFS(&FS).setCurrentDir(TestRoot).setMarkEOLs(true).setRelativeNames(
+ true);
+ ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
const char *Expected[] = {"clang", "-Xclang", "-Wno-whatever", nullptr,
"input.cpp"};
ASSERT_EQ(std::size(Expected), Argv.size());
@@ -1125,9 +1121,8 @@
EXPECT_NE(CurrDir.str(), TestDir.path());
llvm::BumpPtrAllocator A;
- llvm::StringSaver Saver(A);
- bool Result = llvm::cl::readConfigFile(ConfigFile.path(), Saver, Argv,
- *llvm::vfs::getRealFileSystem());
+ llvm::cl::ExpansionContext ECtx(A, cl::tokenizeConfigFile);
+ 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;
@@ -1264,8 +1258,8 @@
// always have an absolute path deduced from the containing file.
SmallString<128> CurrDir;
if (llvm::sys::path::is_relative(FName)) {
- if (!CurrentDir) {
- if (auto CWD = FS.getCurrentWorkingDirectory()) {
+ if (CurrentDir.empty()) {
+ if (auto CWD = FS->getCurrentWorkingDirectory()) {
CurrDir = *CWD;
} else {
// TODO: The error should be propagated up the stack.
@@ -1273,19 +1267,19 @@
return false;
}
} 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()));
@@ -1306,9 +1300,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.
@@ -1338,15 +1330,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) {
@@ -1360,30 +1343,30 @@
// Command line options can override the environment variable.
NewArgv.append(Argv + 1, Argv + Argc);
- return ExpandResponseFiles(Saver, Tokenize, NewArgv);
+ ExpansionContext ECtx(Saver.getAllocator(), Tokenize);
+ return ECtx.expandResponseFiles(NewArgv);
}
-bool cl::readConfigFile(StringRef CfgFile, StringSaver &Saver,
- SmallVectorImpl<const char *> &Argv,
- llvm::vfs::FileSystem &FS) {
+ExpansionContext::ExpansionContext(BumpPtrAllocator &A, TokenizerCallback T)
+ : Saver(A), Tokenizer(T), FS(vfs::getRealFileSystem().get()) {}
+
+bool ExpansionContext::readConfigFile(StringRef CfgFile,
+ SmallVectorImpl<const char *> &Argv) {
SmallString<128> AbsPath;
if (sys::path::is_relative(CfgFile)) {
AbsPath.assign(CfgFile);
- if (std::error_code EC = FS.makeAbsolute(AbsPath))
+ if (std::error_code EC = FS->makeAbsolute(AbsPath))
return false;
CfgFile = AbsPath.str();
}
- if (llvm::Error Err =
- ExpandResponseFile(CfgFile, Saver, cl::tokenizeConfigFile, Argv,
- /*MarkEOLs=*/false, /*RelativeNames=*/true,
- /*ExpandBasePath=*/true, FS)) {
+ 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, FS);
+ return expandResponseFiles(Argv);
}
static void initCommonOptions();
@@ -1441,11 +1424,10 @@
// Expand response files.
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(A, 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/StringSaver.h
===================================================================
--- llvm/include/llvm/Support/StringSaver.h
+++ llvm/include/llvm/Support/StringSaver.h
@@ -24,6 +24,8 @@
public:
StringSaver(BumpPtrAllocator &Alloc) : Alloc(Alloc) {}
+ BumpPtrAllocator &getAllocator() const { return Alloc; }
+
// All returned strings are null-terminated: *save(S).end() == 0.
StringRef save(const char *S) { return save(StringRef(S)); }
StringRef save(StringRef S);
Index: llvm/include/llvm/Support/CommandLine.h
===================================================================
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -31,6 +31,7 @@
#include "llvm/ADT/iterator_range.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/StringSaver.h"
#include "llvm/Support/raw_ostream.h"
#include <cassert>
#include <climits>
@@ -2065,56 +2066,88 @@
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,
- llvm::vfs::FileSystem &FS);
-
-/// 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, the file system current directory is used instead.
+/// Contains options that control response file expansion.
+class ExpansionContext {
+ /// Provides persistent storage for parsed strings.
+ StringSaver Saver;
+
+ /// Tokenization strategy. Typically Unix or Windows.
+ TokenizerCallback Tokenizer;
+
+ /// File system used for all file access when running the expansion.
+ vfs::FileSystem *FS;
+
+ /// Path used to resolve relative rsp files. If empty, the file system
+ /// current directory 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(BumpPtrAllocator &A, TokenizerCallback T);
+
+ ExpansionContext &setMarkEOLs(bool X) {
+ MarkEOLs = X;
+ return *this;
+ }
+
+ ExpansionContext &setRelativeNames(bool X) {
+ RelativeNames = X;
+ return *this;
+ }
+
+ ExpansionContext &setCurrentDir(StringRef X) {
+ CurrentDir = X;
+ return *this;
+ }
+
+ ExpansionContext &setVFS(vfs::FileSystem *X) {
+ FS = 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);
+
+ /// Expands constructs "@file" in the provided array of arguments recursively.
+ 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(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);
+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.getAllocator(), 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
@@ -308,9 +308,8 @@
llvm::cl::ResetAllOptionOccurrences();
llvm::BumpPtrAllocator A;
- llvm::StringSaver Saver(A);
- llvm::cl::ExpandResponseFiles(Saver, &llvm::cl::TokenizeGNUCommandLine, ArgV,
- /*MarkEOLs=*/false);
+ llvm::cl::ExpansionContext ECtx(A, llvm::cl::TokenizeGNUCommandLine);
+ ECtx.expandResponseFiles(ArgV);
StringRef Tool = ArgV[1];
void *GetExecutablePathVP = (void *)(intptr_t)GetExecutablePath;
if (Tool == "-cc1")
@@ -373,7 +372,8 @@
if (MarkEOLs && Args.size() > 1 && StringRef(Args[1]).startswith("-cc1"))
MarkEOLs = false;
- llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Args, MarkEOLs);
+ llvm::cl::ExpansionContext ECtx(A, 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
@@ -60,9 +60,10 @@
if (!SeenRSPFile)
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(Alloc, Tokenizer);
+ ECtx.setVFS(FS.get())
+ .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
@@ -956,7 +956,9 @@
bool Driver::readConfigFile(StringRef FileName) {
// Try reading the given file.
SmallVector<const char *, 32> NewCfgArgs;
- if (!llvm::cl::readConfigFile(FileName, Saver, NewCfgArgs, getVFS())) {
+ llvm::cl::ExpansionContext ExpCtx(Alloc, llvm::cl::tokenizeConfigFile);
+ ExpCtx.setVFS(&getVFS());
+ 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
