diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -956,7 +956,9 @@ bool Driver::readConfigFile(StringRef FileName) { // Try reading the given file. SmallVector 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; } diff --git a/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp b/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp --- a/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp +++ b/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 ExpandedArgv(Argv.begin(), Argv.end()); Cmd.CommandLine = std::move(ExpandedArgv); diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp --- a/clang/tools/driver/driver.cpp +++ b/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. diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h --- a/llvm/include/llvm/Support/CommandLine.h +++ b/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 #include @@ -2065,56 +2066,88 @@ SmallVectorImpl &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 "" to the base path of the -/// current config file. -/// -bool readConfigFile(StringRef CfgFileName, StringSaver &Saver, - SmallVectorImpl &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, "" 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 &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 "" to the base path of the + /// current config file. + bool readConfigFile(StringRef CfgFile, SmallVectorImpl &Argv); + + /// Expands constructs "@file" in the provided array of arguments recursively. + bool expandResponseFiles(SmallVectorImpl &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 &Argv, bool MarkEOLs, - bool RelativeNames, bool ExpandBasePath, - llvm::Optional CurrentDir, - llvm::vfs::FileSystem &FS); - -/// An overload of ExpandResponseFiles() that uses -/// llvm::vfs::getRealFileSystem(). -bool ExpandResponseFiles( - StringSaver &Saver, TokenizerCallback Tokenizer, - SmallVectorImpl &Argv, bool MarkEOLs = false, - bool RelativeNames = false, bool ExpandBasePath = false, - llvm::Optional CurrentDir = llvm::None); +bool expandResponseFiles(int Argc, const char *const *Argv, const char *EnvVar, + SmallVectorImpl &NewArgv); + +/// A convenience helper which supports the typical use case of expansion +/// function call. +inline bool ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer, + SmallVectorImpl &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 diff --git a/llvm/include/llvm/Support/StringSaver.h b/llvm/include/llvm/Support/StringSaver.h --- a/llvm/include/llvm/Support/StringSaver.h +++ b/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); diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp --- a/llvm/lib/Support/CommandLine.cpp +++ b/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 &NewArgv, - bool MarkEOLs, bool RelativeNames, - bool ExpandBasePath, - llvm::vfs::FileSystem &FS) { +llvm::Error +ExpansionContext::expandResponseFile(StringRef FName, + SmallVectorImpl &NewArgv) { assert(sys::path::is_absolute(FName)); llvm::ErrorOr> MemBufOrErr = - FS.getBufferForFile(FName); + FS->getBufferForFile(FName); if (!MemBufOrErr) return llvm::errorCodeToError(MemBufOrErr.getError()); MemoryBuffer &MemBuf = *MemBufOrErr.get(); @@ -1196,7 +1193,7 @@ continue; // Substitute 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 &Argv, bool MarkEOLs, - bool RelativeNames, bool ExpandBasePath, - llvm::Optional CurrentDir, - llvm::vfs::FileSystem &FS) { +bool ExpansionContext::expandResponseFiles( + SmallVectorImpl &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 LHS = FS.status(FName); + auto IsEquivalent = [FName, this](const ResponseFileRecord &RFile) { + llvm::ErrorOr 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 RHS = FS.status(RFile.File); + llvm::ErrorOr 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 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 &Argv, bool MarkEOLs, - bool RelativeNames, bool ExpandBasePath, - llvm::Optional 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 &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 &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 &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 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(newArgv.size()); diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp --- a/llvm/unittests/Support/CommandLineTest.cpp +++ b/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 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 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 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);