Author: Serge Pavlov Date: 2022-09-28T13:33:28+07:00 New Revision: 5ddde5f80ab69cc69f64f6fd0114fc7992a29c61
URL: https://github.com/llvm/llvm-project/commit/5ddde5f80ab69cc69f64f6fd0114fc7992a29c61 DIFF: https://github.com/llvm/llvm-project/commit/5ddde5f80ab69cc69f64f6fd0114fc7992a29c61.diff LOG: Revert "[Support] Class for response file expansion (NFC)" This reverts commit 6e491c48d6b9cadcc5b77f730dd83a1448197329. There are missed changes in flang. Added: Modified: 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 Removed: ################################################################################ diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index f2ae25df5825..a40a992ac087 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -956,9 +956,7 @@ static void appendOneArg(InputArgList &Args, const Arg *Opt, bool Driver::readConfigFile(StringRef FileName) { // Try reading the given file. SmallVector<const char *, 32> NewCfgArgs; - llvm::cl::ExpansionContext ExpCtx(Alloc, llvm::cl::tokenizeConfigFile); - ExpCtx.setVFS(&getVFS()); - if (!ExpCtx.readConfigFile(FileName, NewCfgArgs)) { + if (!llvm::cl::readConfigFile(FileName, Saver, NewCfgArgs, getVFS())) { 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 index c4b3abc1a0a4..75d0d50d851f 100644 --- a/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp +++ b/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp @@ -60,10 +60,9 @@ class ExpandResponseFilesDatabase : public CompilationDatabase { if (!SeenRSPFile) continue; llvm::BumpPtrAllocator Alloc; - llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer); - ECtx.setVFS(FS.get()) - .setCurrentDir(Cmd.Directory) - .expandResponseFiles(Argv); + llvm::StringSaver Saver(Alloc); + llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, false, + llvm::StringRef(Cmd.Directory), *FS); // Don't assign directly, Argv aliases CommandLine. std::vector<std::string> ExpandedArgv(Argv.begin(), Argv.end()); Cmd.CommandLine = std::move(ExpandedArgv); diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 11eba44fcf22..34335a599a00 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -308,8 +308,9 @@ static int ExecuteCC1Tool(SmallVectorImpl<const char *> &ArgV) { llvm::cl::ResetAllOptionOccurrences(); llvm::BumpPtrAllocator A; - llvm::cl::ExpansionContext ECtx(A, llvm::cl::TokenizeGNUCommandLine); - ECtx.expandResponseFiles(ArgV); + llvm::StringSaver Saver(A); + llvm::cl::ExpandResponseFiles(Saver, &llvm::cl::TokenizeGNUCommandLine, ArgV, + /*MarkEOLs=*/false); StringRef Tool = ArgV[1]; void *GetExecutablePathVP = (void *)(intptr_t)GetExecutablePath; if (Tool == "-cc1") @@ -372,8 +373,7 @@ int clang_main(int Argc, char **Argv) { if (MarkEOLs && Args.size() > 1 && StringRef(Args[1]).startswith("-cc1")) MarkEOLs = false; - llvm::cl::ExpansionContext ECtx(A, Tokenizer); - ECtx.setMarkEOLs(MarkEOLs).expandResponseFiles(Args); + llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Args, MarkEOLs); // 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 index 15deb8a0b027..58ded2ceee9d 100644 --- a/llvm/include/llvm/Support/CommandLine.h +++ b/llvm/include/llvm/Support/CommandLine.h @@ -31,7 +31,6 @@ #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> @@ -2066,88 +2065,56 @@ void tokenizeConfigFile(StringRef Source, StringSaver &Saver, SmallVectorImpl<const char *> &NewArgv, bool MarkEOLs = false); -/// 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. +/// 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. /// \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.getAllocator(), Tokenizer); - return ECtx.expandResponseFiles(Argv); -} +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); /// 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 index 2ef87754a0cf..c54044e3986c 100644 --- a/llvm/include/llvm/Support/StringSaver.h +++ b/llvm/include/llvm/Support/StringSaver.h @@ -24,8 +24,6 @@ class StringSaver final { 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 index 899cc521056a..d81e115844d3 100644 --- a/llvm/lib/Support/CommandLine.cpp +++ b/llvm/lib/Support/CommandLine.cpp @@ -1153,12 +1153,15 @@ static void ExpandBasePaths(StringRef BasePath, StringSaver &Saver, } // FName must be an absolute path. -llvm::Error -ExpansionContext::expandResponseFile(StringRef FName, - SmallVectorImpl<const char *> &NewArgv) { +static llvm::Error ExpandResponseFile(StringRef FName, StringSaver &Saver, + TokenizerCallback Tokenizer, + SmallVectorImpl<const char *> &NewArgv, + bool MarkEOLs, bool RelativeNames, + bool ExpandBasePath, + llvm::vfs::FileSystem &FS) { 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(); @@ -1193,7 +1196,7 @@ ExpansionContext::expandResponseFile(StringRef FName, continue; // Substitute <CFGDIR> with the file's base path. - if (InConfigFile) + if (ExpandBasePath) ExpandBasePaths(BasePath, Saver, Arg); // Skip non-rsp file arguments. @@ -1216,8 +1219,11 @@ ExpansionContext::expandResponseFile(StringRef FName, /// Expand response files on a command line recursively using the given /// StringSaver and tokenization strategy. -bool ExpansionContext::expandResponseFiles( - SmallVectorImpl<const char *> &Argv) { +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 AllExpanded = true; struct ResponseFileRecord { std::string File; @@ -1258,8 +1264,8 @@ bool ExpansionContext::expandResponseFiles( // always have an absolute path deduced from the containing file. SmallString<128> CurrDir; if (llvm::sys::path::is_relative(FName)) { - if (CurrentDir.empty()) { - if (auto CWD = FS->getCurrentWorkingDirectory()) { + if (!CurrentDir) { + if (auto CWD = FS.getCurrentWorkingDirectory()) { CurrDir = *CWD; } else { // TODO: The error should be propagated up the stack. @@ -1267,19 +1273,19 @@ bool ExpansionContext::expandResponseFiles( return false; } } else { - CurrDir = CurrentDir; + CurrDir = *CurrentDir; } llvm::sys::path::append(CurrDir, FName); FName = CurrDir.c_str(); } - auto IsEquivalent = [FName, this](const ResponseFileRecord &RFile) { - llvm::ErrorOr<llvm::vfs::Status> LHS = FS->status(FName); + auto IsEquivalent = [FName, &FS](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())); @@ -1300,7 +1306,9 @@ bool ExpansionContext::expandResponseFiles( // 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, ExpandedArgv)) { + if (llvm::Error Err = + ExpandResponseFile(FName, Saver, Tokenizer, ExpandedArgv, MarkEOLs, + RelativeNames, ExpandBasePath, FS)) { // 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. @@ -1330,6 +1338,15 @@ bool ExpansionContext::expandResponseFiles( 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) { @@ -1343,30 +1360,30 @@ bool cl::expandResponseFiles(int Argc, const char *const *Argv, // Command line options can override the environment variable. NewArgv.append(Argv + 1, Argv + Argc); - ExpansionContext ECtx(Saver.getAllocator(), Tokenize); - return ECtx.expandResponseFiles(NewArgv); + return ExpandResponseFiles(Saver, Tokenize, NewArgv); } -ExpansionContext::ExpansionContext(BumpPtrAllocator &A, TokenizerCallback T) - : Saver(A), Tokenizer(T), FS(vfs::getRealFileSystem().get()) {} - -bool ExpansionContext::readConfigFile(StringRef CfgFile, - SmallVectorImpl<const char *> &Argv) { +bool cl::readConfigFile(StringRef CfgFile, StringSaver &Saver, + SmallVectorImpl<const char *> &Argv, + llvm::vfs::FileSystem &FS) { 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(); } - InConfigFile = true; - RelativeNames = true; - if (llvm::Error Err = expandResponseFile(CfgFile, Argv)) { + if (llvm::Error Err = + ExpandResponseFile(CfgFile, Saver, cl::tokenizeConfigFile, Argv, + /*MarkEOLs=*/false, /*RelativeNames=*/true, + /*ExpandBasePath=*/true, FS)) { // TODO: The error should be propagated up the stack. llvm::consumeError(std::move(Err)); return false; } - return expandResponseFiles(Argv); + return ExpandResponseFiles(Saver, cl::tokenizeConfigFile, Argv, + /*MarkEOLs=*/false, /*RelativeNames=*/true, + /*ExpandBasePath=*/true, llvm::None, FS); } static void initCommonOptions(); @@ -1424,10 +1441,11 @@ bool CommandLineParser::ParseCommandLineOptions(int argc, // Expand response files. SmallVector<const char *, 20> newArgv(argv, argv + argc); BumpPtrAllocator A; - ExpansionContext ECtx(A, Triple(sys::getProcessTriple()).isOSWindows() - ? cl::TokenizeWindowsCommandLine - : cl::TokenizeGNUCommandLine); - ECtx.expandResponseFiles(newArgv); + StringSaver Saver(A); + ExpandResponseFiles(Saver, + Triple(sys::getProcessTriple()).isOSWindows() ? + cl::TokenizeWindowsCommandLine : cl::TokenizeGNUCommandLine, + newArgv); argv = &newArgv[0]; argc = static_cast<int>(newArgv.size()); diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp index 7af8c89e90e0..4fd037e8638a 100644 --- a/llvm/unittests/Support/CommandLineTest.cpp +++ b/llvm/unittests/Support/CommandLineTest.cpp @@ -869,9 +869,10 @@ TEST(CommandLineTest, ResponseFiles) { // Expand response files. llvm::BumpPtrAllocator A; - llvm::cl::ExpansionContext ECtx(A, llvm::cl::TokenizeGNUCommandLine); - ECtx.setVFS(&FS).setCurrentDir(TestRoot).setRelativeNames(true); - ASSERT_TRUE(ECtx.expandResponseFiles(Argv)); + llvm::StringSaver Saver(A); + ASSERT_TRUE(llvm::cl::ExpandResponseFiles( + Saver, llvm::cl::TokenizeGNUCommandLine, Argv, false, true, false, + /*CurrentDir=*/StringRef(TestRoot), FS)); EXPECT_THAT(Argv, testing::Pointwise( StringEquality(), {"test/test", "-flag_1", "-option_1", "-option_2", @@ -926,14 +927,15 @@ TEST(CommandLineTest, RecursiveResponseFiles) { 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 - llvm::cl::ExpansionContext ECtx(A, Tokenizer); - ECtx.setVFS(&FS).setCurrentDir(TestRoot); - ASSERT_FALSE(ECtx.expandResponseFiles(Argv)); + ASSERT_FALSE( + cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, false, + /*CurrentDir=*/llvm::StringRef(TestRoot), FS)); EXPECT_THAT(Argv, testing::Pointwise(StringEquality(), @@ -969,9 +971,10 @@ TEST(CommandLineTest, ResponseFilesAtArguments) { Argv.push_back(ResponseFileRef.c_str()); BumpPtrAllocator A; - llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine); - ECtx.setVFS(&FS).setCurrentDir(TestRoot); - ASSERT_FALSE(ECtx.expandResponseFiles(Argv)); + StringSaver Saver(A); + ASSERT_FALSE(cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv, + false, false, false, + /*CurrentDir=*/StringRef(TestRoot), FS)); // ASSERT instead of EXPECT to prevent potential out-of-bounds access. ASSERT_EQ(Argv.size(), 1 + NON_RSP_AT_ARGS + 2); @@ -1003,9 +1006,10 @@ TEST(CommandLineTest, ResponseFileRelativePath) { SmallVector<const char *, 2> Argv = {"test/test", "@dir/outer.rsp"}; BumpPtrAllocator A; - llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine); - ECtx.setVFS(&FS).setCurrentDir(TestRoot).setRelativeNames(true); - ASSERT_TRUE(ECtx.expandResponseFiles(Argv)); + StringSaver Saver(A); + ASSERT_TRUE(cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv, + false, true, false, + /*CurrentDir=*/StringRef(TestRoot), FS)); EXPECT_THAT(Argv, testing::Pointwise(StringEquality(), {"test/test", "-flag"})); } @@ -1022,10 +1026,10 @@ TEST(CommandLineTest, ResponseFileEOLs) { MemoryBuffer::getMemBuffer("-Xclang -Wno-whatever\n input.cpp")); SmallVector<const char *, 2> Argv = {"clang", "@eols.rsp"}; BumpPtrAllocator A; - llvm::cl::ExpansionContext ECtx(A, cl::TokenizeWindowsCommandLine); - ECtx.setVFS(&FS).setCurrentDir(TestRoot).setMarkEOLs(true).setRelativeNames( - true); - ASSERT_TRUE(ECtx.expandResponseFiles(Argv)); + StringSaver Saver(A); + ASSERT_TRUE(cl::ExpandResponseFiles(Saver, cl::TokenizeWindowsCommandLine, + Argv, true, true, false, + /*CurrentDir=*/StringRef(TestRoot), FS)); const char *Expected[] = {"clang", "-Xclang", "-Wno-whatever", nullptr, "input.cpp"}; ASSERT_EQ(std::size(Expected), Argv.size()); @@ -1121,8 +1125,9 @@ TEST(CommandLineTest, ReadConfigFile) { EXPECT_NE(CurrDir.str(), TestDir.path()); llvm::BumpPtrAllocator A; - llvm::cl::ExpansionContext ECtx(A, cl::tokenizeConfigFile); - bool Result = ECtx.readConfigFile(ConfigFile.path(), Argv); + llvm::StringSaver Saver(A); + bool Result = llvm::cl::readConfigFile(ConfigFile.path(), Saver, Argv, + *llvm::vfs::getRealFileSystem()); EXPECT_TRUE(Result); EXPECT_EQ(Argv.size(), 13U); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
