Author: Serge Pavlov Date: 2022-10-30T02:03:12+07:00 New Revision: c1728a40aae31abc0a5d4d07f6f6a6773d803f2c
URL: https://github.com/llvm/llvm-project/commit/c1728a40aae31abc0a5d4d07f6f6a6773d803f2c DIFF: https://github.com/llvm/llvm-project/commit/c1728a40aae31abc0a5d4d07f6f6a6773d803f2c.diff LOG: Revert "Handle errors in expansion of response files" This reverts commit 17eb198de934eced784e16ec15e020a574ba07e1. Reverted for investigation, because ClangDriverTests failed on some builders. Added: Modified: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/lib/Driver/Driver.cpp clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp clang/test/Driver/config-file-errs.c clang/tools/driver/driver.cpp clang/unittests/Driver/ToolChainTest.cpp flang/tools/flang-driver/driver.cpp llvm/include/llvm/Support/CommandLine.h llvm/lib/Support/CommandLine.cpp llvm/unittests/Support/CommandLineTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index e477d93ba067f..b09e124206213 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -214,14 +214,14 @@ def err_drv_malformed_sanitizer_coverage_ignorelist : Error< "malformed sanitizer coverage ignorelist: '%0'">; def err_drv_duplicate_config : Error< "no more than one option '--config' is allowed">; -def err_drv_cannot_open_config_file : Error< - "configuration file '%0' cannot be opened: %1">; +def err_drv_config_file_not_exist : Error< + "configuration file '%0' does not exist">; def err_drv_config_file_not_found : Error< "configuration file '%0' cannot be found">; def note_drv_config_file_searched_in : Note< "was searched for in the directory: %0">; def err_drv_cannot_read_config_file : Error< - "cannot read configuration file '%0': %1">; + "cannot read configuration file '%0'">; def err_drv_nested_config_file: Error< "option '--config' is not allowed inside configuration file">; def err_drv_arg_requires_bitcode_input: Error< diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 80e6ec76d16f7..26729f1d03ea8 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -940,24 +940,10 @@ static void appendOneArg(InputArgList &Args, const Arg *Opt, bool Driver::readConfigFile(StringRef FileName, llvm::cl::ExpansionContext &ExpCtx) { - // Try opening the given file. - auto Status = getVFS().status(FileName); - if (!Status) { - Diag(diag::err_drv_cannot_open_config_file) - << FileName << Status.getError().message(); - return true; - } - if (Status->getType() != llvm::sys::fs::file_type::regular_file) { - Diag(diag::err_drv_cannot_open_config_file) - << FileName << "not a regular file"; - return true; - } - // Try reading the given file. SmallVector<const char *, 32> NewCfgArgs; - if (llvm::Error Err = ExpCtx.readConfigFile(FileName, NewCfgArgs)) { - Diag(diag::err_drv_cannot_read_config_file) - << FileName << toString(std::move(Err)); + if (!ExpCtx.readConfigFile(FileName, NewCfgArgs)) { + Diag(diag::err_drv_cannot_read_config_file) << FileName; return true; } @@ -1039,9 +1025,12 @@ bool Driver::loadConfigFiles() { if (llvm::sys::path::has_parent_path(CfgFileName)) { CfgFilePath.assign(CfgFileName); if (llvm::sys::path::is_relative(CfgFilePath)) { - if (getVFS().makeAbsolute(CfgFilePath)) { - Diag(diag::err_drv_cannot_open_config_file) - << CfgFilePath << "cannot get absolute path"; + if (getVFS().makeAbsolute(CfgFilePath)) + return true; + auto Status = getVFS().status(CfgFilePath); + if (!Status || + Status->getType() != llvm::sys::fs::file_type::regular_file) { + Diag(diag::err_drv_config_file_not_exist) << CfgFilePath; return true; } } diff --git a/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp b/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp index 88d20ba3957d2..c4b3abc1a0a45 100644 --- a/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp +++ b/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp @@ -61,11 +61,9 @@ class ExpandResponseFilesDatabase : public CompilationDatabase { continue; llvm::BumpPtrAllocator Alloc; llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer); - llvm::Error Err = ECtx.setVFS(FS.get()) - .setCurrentDir(Cmd.Directory) - .expandResponseFiles(Argv); - if (Err) - llvm::errs() << Err; + 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); diff --git a/clang/test/Driver/config-file-errs.c b/clang/test/Driver/config-file-errs.c index 85b3443a7e530..497e6cb4a5c2b 100644 --- a/clang/test/Driver/config-file-errs.c +++ b/clang/test/Driver/config-file-errs.c @@ -13,7 +13,7 @@ //--- Argument of '--config' must be existing file, if it is specified by path. // // RUN: not %clang --config somewhere/nonexistent-config-file 2>&1 | FileCheck %s -check-prefix CHECK-NONEXISTENT -// CHECK-NONEXISTENT: configuration file '{{.*}}somewhere{{.}}nonexistent-config-file' cannot be opened: {{[Nn]}}o such file or directory +// CHECK-NONEXISTENT: configuration file '{{.*}}somewhere/nonexistent-config-file' does not exist //--- All '--config' arguments must be existing files. diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 2cc3b48609cb3..11eba44fcf22e 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -309,10 +309,7 @@ static int ExecuteCC1Tool(SmallVectorImpl<const char *> &ArgV) { llvm::BumpPtrAllocator A; llvm::cl::ExpansionContext ECtx(A, llvm::cl::TokenizeGNUCommandLine); - if (llvm::Error Err = ECtx.expandResponseFiles(ArgV)) { - llvm::errs() << toString(std::move(Err)) << '\n'; - return 1; - } + ECtx.expandResponseFiles(ArgV); StringRef Tool = ArgV[1]; void *GetExecutablePathVP = (void *)(intptr_t)GetExecutablePath; if (Tool == "-cc1") @@ -376,11 +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); - if (llvm::Error Err = ECtx.expandResponseFiles(Args)) { - llvm::errs() << Err << '\n'; - return 1; - } + ECtx.setMarkEOLs(MarkEOLs).expandResponseFiles(Args); // Handle -cc1 integrated tools, even if -cc1 was expanded from a response // file. diff --git a/clang/unittests/Driver/ToolChainTest.cpp b/clang/unittests/Driver/ToolChainTest.cpp index 04c9bb13161bf..10b20a91aee27 100644 --- a/clang/unittests/Driver/ToolChainTest.cpp +++ b/clang/unittests/Driver/ToolChainTest.cpp @@ -450,204 +450,4 @@ TEST(ToolChainTest, ConfigFileSearch) { } } -struct FileSystemWithError : public llvm::vfs::FileSystem { - llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override { - return std::make_error_code(std::errc::no_such_file_or_directory); - } - llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>> - openFileForRead(const Twine &Path) override { - return std::make_error_code(std::errc::permission_denied); - } - llvm::vfs::directory_iterator dir_begin(const Twine &Dir, - std::error_code &EC) override { - return llvm::vfs::directory_iterator(); - } - std::error_code setCurrentWorkingDirectory(const Twine &Path) override { - return std::make_error_code(std::errc::permission_denied); - } - llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override { - return std::make_error_code(std::errc::permission_denied); - } -}; - -TEST(ToolChainTest, ConfigFileError) { - IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions(); - IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs()); - std::unique_ptr<SimpleDiagnosticConsumer> DiagConsumer( - new SimpleDiagnosticConsumer()); - DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer.get(), false); - IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS(new FileSystemWithError); - - Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags, - "clang LLVM compiler", FS); - std::unique_ptr<Compilation> C( - TheDriver.BuildCompilation({"/home/test/bin/clang", "--no-default-config", - "--config", "./root.cfg", "--version"})); - ASSERT_TRUE(C); - ASSERT_TRUE(C->containsError()); - EXPECT_EQ(1U, Diags.getNumErrors()); - EXPECT_STREQ("configuration file './root.cfg' cannot be opened: cannot get " - "absolute path", - DiagConsumer->Errors[0].c_str()); -} - -TEST(ToolChainTest, BadConfigFile) { - IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions(); - IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs()); - std::unique_ptr<SimpleDiagnosticConsumer> DiagConsumer( - new SimpleDiagnosticConsumer()); - DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer.get(), false); - IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS( - new llvm::vfs::InMemoryFileSystem); - -#ifdef _WIN32 - const char *TestRoot = "C:\\"; -#define FILENAME "C:/opt/root.cfg" -#define DIRNAME "C:/opt" -#else - const char *TestRoot = "/"; -#define FILENAME "/opt/root.cfg" -#define DIRNAME "/opt" -#endif - // UTF-16 string must be aligned on 2-byte boundary. Strings and char arrays - // do not provide necessary alignment, so copy constant string into properly - // allocated memory in heap. - llvm::BumpPtrAllocator Alloc; - char *StrBuff = (char *)Alloc.Allocate(6, 2); - std::memcpy(StrBuff, "\xFF\xFE\x00\xD8\x00\x00", 6); - StringRef BadUTF(StrBuff, 6); - FS->setCurrentWorkingDirectory(TestRoot); - FS->addFile("/opt/root.cfg", 0, llvm::MemoryBuffer::getMemBuffer(BadUTF)); - FS->addFile("/home/user/test.cfg", 0, - llvm::MemoryBuffer::getMemBuffer("@file.rsp")); - - { - Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags, - "clang LLVM compiler", FS); - std::unique_ptr<Compilation> C(TheDriver.BuildCompilation( - {"/home/test/bin/clang", "--config", "/opt/root.cfg", "--version"})); - ASSERT_TRUE(C); - ASSERT_TRUE(C->containsError()); - EXPECT_EQ(1U, DiagConsumer->Errors.size()); - EXPECT_STREQ("cannot read configuration file '" FILENAME - "': Could not convert UTF16 to UTF8", - DiagConsumer->Errors[0].c_str()); - } - DiagConsumer->clear(); - { - Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags, - "clang LLVM compiler", FS); - std::unique_ptr<Compilation> C(TheDriver.BuildCompilation( - {"/home/test/bin/clang", "--config", "/opt", "--version"})); - ASSERT_TRUE(C); - ASSERT_TRUE(C->containsError()); - EXPECT_EQ(1U, DiagConsumer->Errors.size()); - EXPECT_STREQ("configuration file '" DIRNAME - "' cannot be opened: not a regular file", - DiagConsumer->Errors[0].c_str()); - } - DiagConsumer->clear(); - { - Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags, - "clang LLVM compiler", FS); - std::unique_ptr<Compilation> C(TheDriver.BuildCompilation( - {"/home/test/bin/clang", "--config", "root", - "--config-system-dir=", "--config-user-dir=", "--version"})); - ASSERT_TRUE(C); - ASSERT_TRUE(C->containsError()); - EXPECT_EQ(1U, DiagConsumer->Errors.size()); - EXPECT_STREQ("configuration file 'root' cannot be found", - DiagConsumer->Errors[0].c_str()); - } - -#undef FILENAME -#undef DIRNAME -} - -TEST(ToolChainTest, ConfigInexistentInclude) { - IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions(); - IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs()); - std::unique_ptr<SimpleDiagnosticConsumer> DiagConsumer( - new SimpleDiagnosticConsumer()); - DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer.get(), false); - IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS( - new llvm::vfs::InMemoryFileSystem); - -#ifdef _WIN32 - const char *TestRoot = "C:\\"; -#define USERCONFIG "C:\\home\\user\\test.cfg" -#define UNEXISTENT "C:\\home\\user\\file.rsp" -#else - const char *TestRoot = "/"; -#define USERCONFIG "/home/user/test.cfg" -#define UNEXISTENT "/home/user/file.rsp" -#endif - FS->setCurrentWorkingDirectory(TestRoot); - FS->addFile("/home/user/test.cfg", 0, - llvm::MemoryBuffer::getMemBuffer("@file.rsp")); - - { - Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags, - "clang LLVM compiler", FS); - std::unique_ptr<Compilation> C(TheDriver.BuildCompilation( - {"/home/test/bin/clang", "--config", "test.cfg", - "--config-system-dir=", "--config-user-dir=/home/user", "--version"})); - ASSERT_TRUE(C); - ASSERT_TRUE(C->containsError()); - EXPECT_EQ(1U, DiagConsumer->Errors.size()); - EXPECT_STREQ("cannot read configuration file '" USERCONFIG - "': cannot not open file '" UNEXISTENT "'", - DiagConsumer->Errors[0].c_str()); - } - -#undef USERCONFIG -#undef UNEXISTENT -} - -TEST(ToolChainTest, ConfigRecursiveInclude) { - IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions(); - IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs()); - std::unique_ptr<SimpleDiagnosticConsumer> DiagConsumer( - new SimpleDiagnosticConsumer()); - DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer.get(), false); - IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS( - new llvm::vfs::InMemoryFileSystem); - -#ifdef _WIN32 - const char *TestRoot = "C:\\"; -#define USERCONFIG "C:\\home\\user\\test.cfg" -#define INCLUDED1 "C:\\home\\user\\file1.cfg" -#else - const char *TestRoot = "/"; -#define USERCONFIG "/home/user/test.cfg" -#define INCLUDED1 "/home/user/file1.cfg" -#endif - FS->setCurrentWorkingDirectory(TestRoot); - FS->addFile("/home/user/test.cfg", 0, - llvm::MemoryBuffer::getMemBuffer("@file1.cfg")); - FS->addFile("/home/user/file1.cfg", 0, - llvm::MemoryBuffer::getMemBuffer("@file2.cfg")); - FS->addFile("/home/user/file2.cfg", 0, - llvm::MemoryBuffer::getMemBuffer("@file3.cfg")); - FS->addFile("/home/user/file3.cfg", 0, - llvm::MemoryBuffer::getMemBuffer("@file1.cfg")); - - { - Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags, - "clang LLVM compiler", FS); - std::unique_ptr<Compilation> C(TheDriver.BuildCompilation( - {"/home/test/bin/clang", "--config", "test.cfg", - "--config-system-dir=", "--config-user-dir=/home/user", "--version"})); - ASSERT_TRUE(C); - ASSERT_TRUE(C->containsError()); - EXPECT_EQ(1U, DiagConsumer->Errors.size()); - EXPECT_STREQ("cannot read configuration file '" USERCONFIG - "': recursive expansion of: '" INCLUDED1 "'", - DiagConsumer->Errors[0].c_str()); - } - -#undef USERCONFIG -#undef INCLUDED1 -} - } // end anonymous namespace. diff --git a/flang/tools/flang-driver/driver.cpp b/flang/tools/flang-driver/driver.cpp index 28a8db2584b5c..3ab6fc6205011 100644 --- a/flang/tools/flang-driver/driver.cpp +++ b/flang/tools/flang-driver/driver.cpp @@ -77,9 +77,7 @@ static void ExpandResponseFiles( // We're defaulting to the GNU syntax, since we don't have a CL mode. llvm::cl::TokenizerCallback tokenizer = &llvm::cl::TokenizeGNUCommandLine; llvm::cl::ExpansionContext ExpCtx(saver.getAllocator(), tokenizer); - if (llvm::Error Err = ExpCtx.expandResponseFiles(args)) { - llvm::errs() << toString(std::move(Err)) << '\n'; - } + ExpCtx.expandResponseFiles(args); } int main(int argc, const char **argv) { diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h index e68addcaded09..f30d844dc3dd3 100644 --- a/llvm/include/llvm/Support/CommandLine.h +++ b/llvm/include/llvm/Support/CommandLine.h @@ -2206,10 +2206,10 @@ class ExpansionContext { /// 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. - Error readConfigFile(StringRef CfgFile, SmallVectorImpl<const char *> &Argv); + bool readConfigFile(StringRef CfgFile, SmallVectorImpl<const char *> &Argv); /// Expands constructs "@file" in the provided array of arguments recursively. - Error expandResponseFiles(SmallVectorImpl<const char *> &Argv); + bool expandResponseFiles(SmallVectorImpl<const char *> &Argv); }; /// A convenience helper which concatenates the options specified by the @@ -2221,8 +2221,11 @@ bool expandResponseFiles(int Argc, const char *const *Argv, const char *EnvVar, /// A convenience helper which supports the typical use case of expansion /// function call. -bool ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer, - SmallVectorImpl<const char *> &Argv); +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 diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp index 136b813b1f6c8..3986c9103754d 100644 --- a/llvm/lib/Support/CommandLine.cpp +++ b/llvm/lib/Support/CommandLine.cpp @@ -1153,14 +1153,14 @@ static void ExpandBasePaths(StringRef BasePath, StringSaver &Saver, } // FName must be an absolute path. -Error ExpansionContext::expandResponseFile( - StringRef FName, SmallVectorImpl<const char *> &NewArgv) { +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); if (!MemBufOrErr) - return llvm::createStringError( - MemBufOrErr.getError(), Twine("cannot not open file '") + FName + "'"); + return llvm::errorCodeToError(MemBufOrErr.getError()); MemoryBuffer &MemBuf = *MemBufOrErr.get(); StringRef Str(MemBuf.getBufferStart(), MemBuf.getBufferSize()); @@ -1182,30 +1182,29 @@ Error ExpansionContext::expandResponseFile( // Tokenize the contents into NewArgv. Tokenizer(Str, Saver, NewArgv, MarkEOLs); - // Expanded file content may require additional transformations, like using - // absolute paths instead of relative in '@file' constructs or expanding - // macros. - if (!RelativeNames && !InConfigFile) + if (!RelativeNames) return Error::success(); - - StringRef BasePath = llvm::sys::path::parent_path(FName); - for (auto I = NewArgv.begin(), E = NewArgv.end(); I != E; ++I) { - const char *&Arg = *I; - if (Arg == nullptr) + llvm::StringRef BasePath = llvm::sys::path::parent_path(FName); + // If names of nested response files should be resolved relative to including + // file, replace the included response file names with their full paths + // obtained by required resolution. + for (auto &Arg : NewArgv) { + if (!Arg) continue; // Substitute <CFGDIR> with the file's base path. if (InConfigFile) ExpandBasePaths(BasePath, Saver, Arg); - // Get expanded file name. - StringRef FileName(Arg); - if (!FileName.consume_front("@")) + // Skip non-rsp file arguments. + if (Arg[0] != '@') continue; + + StringRef FileName(Arg + 1); + // Skip if non-relative. if (!llvm::sys::path::is_relative(FileName)) continue; - // Update expansion construct. SmallString<128> ResponseFile; ResponseFile.push_back('@'); ResponseFile.append(BasePath); @@ -1217,8 +1216,9 @@ Error ExpansionContext::expandResponseFile( /// Expand response files on a command line recursively using the given /// StringSaver and tokenization strategy. -Error ExpansionContext::expandResponseFiles( +bool ExpansionContext::expandResponseFiles( SmallVectorImpl<const char *> &Argv) { + bool AllExpanded = true; struct ResponseFileRecord { std::string File; size_t End; @@ -1262,8 +1262,9 @@ Error ExpansionContext::expandResponseFiles( if (auto CWD = FS->getCurrentWorkingDirectory()) { CurrDir = *CWD; } else { - return make_error<StringError>( - CWD.getError(), Twine("cannot get absolute path for: ") + FName); + // TODO: The error should be propagated up the stack. + llvm::consumeError(llvm::errorCodeToError(CWD.getError())); + return false; } } else { CurrDir = CurrentDir; @@ -1271,51 +1272,43 @@ Error ExpansionContext::expandResponseFiles( llvm::sys::path::append(CurrDir, FName); FName = CurrDir.c_str(); } - auto IsEquivalent = - [FName, this](const ResponseFileRecord &RFile) -> ErrorOr<bool> { - ErrorOr<llvm::vfs::Status> LHS = FS->status(FName); - if (!LHS) - return LHS.getError(); - ErrorOr<llvm::vfs::Status> RHS = FS->status(RFile.File); - if (!RHS) - return RHS.getError(); + 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); + if (!RHS) { + // TODO: The error should be propagated up the stack. + llvm::consumeError(llvm::errorCodeToError(RHS.getError())); + return false; + } return LHS->equivalent(*RHS); }; // Check for recursive response files. - for (const auto &F : drop_begin(FileStack)) { - if (ErrorOr<bool> R = IsEquivalent(F)) { - if (R.get()) - return make_error<StringError>( - Twine("recursive expansion of: '") + F.File + "'", R.getError()); - } else { - return make_error<StringError>(Twine("cannot open file: ") + F.File, - R.getError()); - } + if (any_of(drop_begin(FileStack), IsEquivalent)) { + // This file is recursive, so we leave it in the argument stream and + // move on. + AllExpanded = false; + ++I; + continue; } // 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 (!InConfigFile) { - // If the specified file does not exist, leave '@file' unexpanded, as - // libiberty does. - ErrorOr<llvm::vfs::Status> Res = FS->status(FName); - if (!Res) { - std::error_code EC = Res.getError(); - if (EC == llvm::errc::no_such_file_or_directory) { - ++I; - continue; - } - } else { - if (!Res->exists()) { - ++I; - continue; - } - } + 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. + llvm::consumeError(std::move(Err)); + AllExpanded = false; + ++I; + continue; } - if (Error Err = expandResponseFile(FName, ExpandedArgv)) - return Err; for (ResponseFileRecord &Record : FileStack) { // Increase the end of all active records by the number of newly expanded @@ -1334,7 +1327,7 @@ Error ExpansionContext::expandResponseFiles( // don't have a chance to pop the stack when encountering recursive files at // the end of the stream, so seeing that doesn't indicate a bug. assert(FileStack.size() > 0 && Argv.size() == FileStack.back().End); - return Error::success(); + return AllExpanded; } bool cl::expandResponseFiles(int Argc, const char *const *Argv, @@ -1351,21 +1344,7 @@ 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); - if (Error Err = ECtx.expandResponseFiles(NewArgv)) { - errs() << toString(std::move(Err)) << '\n'; - return false; - } - return true; -} - -bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer, - SmallVectorImpl<const char *> &Argv) { - ExpansionContext ECtx(Saver.getAllocator(), Tokenizer); - if (Error Err = ECtx.expandResponseFiles(Argv)) { - errs() << toString(std::move(Err)) << '\n'; - return false; - } - return true; + return ECtx.expandResponseFiles(NewArgv); } ExpansionContext::ExpansionContext(BumpPtrAllocator &A, TokenizerCallback T) @@ -1408,20 +1387,22 @@ bool ExpansionContext::findConfigFile(StringRef FileName, return false; } -Error ExpansionContext::readConfigFile(StringRef CfgFile, - SmallVectorImpl<const char *> &Argv) { +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)) - return make_error<StringError>( - EC, Twine("cannot get absolute path for " + CfgFile)); + return false; CfgFile = AbsPath.str(); } InConfigFile = true; RelativeNames = true; - if (Error Err = expandResponseFile(CfgFile, Argv)) - return Err; + 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(Argv); } @@ -1477,28 +1458,25 @@ bool CommandLineParser::ParseCommandLineOptions(int argc, bool LongOptionsUseDoubleDash) { assert(hasOptions() && "No options specified!"); - ProgramOverview = Overview; - bool IgnoreErrors = Errs; - if (!Errs) - Errs = &errs(); - bool ErrorParsing = false; - // Expand response files. SmallVector<const char *, 20> newArgv(argv, argv + argc); BumpPtrAllocator A; ExpansionContext ECtx(A, Triple(sys::getProcessTriple()).isOSWindows() ? cl::TokenizeWindowsCommandLine : cl::TokenizeGNUCommandLine); - if (Error Err = ECtx.expandResponseFiles(newArgv)) { - *Errs << toString(std::move(Err)) << '\n'; - return false; - } + ECtx.expandResponseFiles(newArgv); argv = &newArgv[0]; argc = static_cast<int>(newArgv.size()); // Copy the program name into ProgName, making sure not to overflow it. ProgramName = std::string(sys::path::filename(StringRef(argv[0]))); + ProgramOverview = Overview; + bool IgnoreErrors = Errs; + if (!Errs) + Errs = &errs(); + bool ErrorParsing = false; + // Check out the positional arguments to collect information about them. unsigned NumPositionalRequired = 0; diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp index 26e82d13e799c..dd6e12223dacb 100644 --- a/llvm/unittests/Support/CommandLineTest.cpp +++ b/llvm/unittests/Support/CommandLineTest.cpp @@ -871,7 +871,7 @@ TEST(CommandLineTest, ResponseFiles) { llvm::BumpPtrAllocator A; llvm::cl::ExpansionContext ECtx(A, llvm::cl::TokenizeGNUCommandLine); ECtx.setVFS(&FS).setCurrentDir(TestRoot).setRelativeNames(true); - ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv)); + ASSERT_TRUE(ECtx.expandResponseFiles(Argv)); EXPECT_THAT(Argv, testing::Pointwise( StringEquality(), {"test/test", "-flag_1", "-option_1", "-option_2", @@ -933,14 +933,7 @@ TEST(CommandLineTest, RecursiveResponseFiles) { #endif llvm::cl::ExpansionContext ECtx(A, Tokenizer); ECtx.setVFS(&FS).setCurrentDir(TestRoot); - llvm::Error Err = ECtx.expandResponseFiles(Argv); - ASSERT_TRUE((bool)Err); - SmallString<128> FilePath = SelfFilePath; - std::error_code EC = FS.makeAbsolute(FilePath); - ASSERT_FALSE((bool)EC); - std::string ExpectedMessage = - std::string("recursive expansion of: '") + std::string(FilePath) + "'"; - ASSERT_TRUE(toString(std::move(Err)) == ExpectedMessage); + ASSERT_FALSE(ECtx.expandResponseFiles(Argv)); EXPECT_THAT(Argv, testing::Pointwise(StringEquality(), @@ -978,7 +971,7 @@ TEST(CommandLineTest, ResponseFilesAtArguments) { BumpPtrAllocator A; llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine); ECtx.setVFS(&FS).setCurrentDir(TestRoot); - ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv)); + 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); @@ -1012,7 +1005,7 @@ TEST(CommandLineTest, ResponseFileRelativePath) { BumpPtrAllocator A; llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine); ECtx.setVFS(&FS).setCurrentDir(TestRoot).setRelativeNames(true); - ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv)); + ASSERT_TRUE(ECtx.expandResponseFiles(Argv)); EXPECT_THAT(Argv, testing::Pointwise(StringEquality(), {"test/test", "-flag"})); } @@ -1032,7 +1025,7 @@ TEST(CommandLineTest, ResponseFileEOLs) { llvm::cl::ExpansionContext ECtx(A, cl::TokenizeWindowsCommandLine); ECtx.setVFS(&FS).setCurrentDir(TestRoot).setMarkEOLs(true).setRelativeNames( true); - ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv)); + ASSERT_TRUE(ECtx.expandResponseFiles(Argv)); const char *Expected[] = {"clang", "-Xclang", "-Wno-whatever", nullptr, "input.cpp"}; ASSERT_EQ(std::size(Expected), Argv.size()); @@ -1045,30 +1038,6 @@ TEST(CommandLineTest, ResponseFileEOLs) { } } -TEST(CommandLineTest, BadResponseFile) { - BumpPtrAllocator A; - StringSaver Saver(A); - TempDir ADir("dir", /*Unique*/ true); - SmallString<128> AFilePath = ADir.path(); - llvm::sys::path::append(AFilePath, "file.rsp"); - std::string AFileExp = std::string("@") + std::string(AFilePath.str()); - SmallVector<const char *, 2> Argv = {"clang", AFileExp.c_str()}; - - bool Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv); - ASSERT_TRUE(Res); - ASSERT_EQ(2U, Argv.size()); - ASSERT_STREQ(Argv[0], "clang"); - ASSERT_STREQ(Argv[1], AFileExp.c_str()); - - std::string ADirExp = std::string("@") + std::string(ADir.path()); - Argv = {"clang", ADirExp.c_str()}; - Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv); - ASSERT_FALSE(Res); - ASSERT_EQ(2U, Argv.size()); - ASSERT_STREQ(Argv[0], "clang"); - ASSERT_STREQ(Argv[1], ADirExp.c_str()); -} - TEST(CommandLineTest, SetDefaultValue) { cl::ResetCommandLineParser(); @@ -1176,9 +1145,9 @@ TEST(CommandLineTest, ReadConfigFile) { llvm::BumpPtrAllocator A; llvm::cl::ExpansionContext ECtx(A, cl::tokenizeConfigFile); - llvm::Error Result = ECtx.readConfigFile(ConfigFile.path(), Argv); + bool Result = ECtx.readConfigFile(ConfigFile.path(), Argv); - EXPECT_FALSE((bool)Result); + EXPECT_TRUE(Result); EXPECT_EQ(Argv.size(), 13U); EXPECT_STREQ(Argv[0], "-option_1"); EXPECT_STREQ(Argv[1], _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits