[Clang] Check for response file existence prior to check for recursion

As now errors in file operation are handled, check for file existence
must be done prior to check for recursion, otherwise reported errors are
misleading.

Differential Revision: https://reviews.llvm.org/D136090

diff --git a/clang/test/Driver/Inputs/inc-inexistent.rsp b/clang/test/Driver/Inputs/inc-inexistent.rsp
new file mode 100644 (file)
index 0000000..c9ecfdf
--- /dev/null
+++ b/clang/test/Driver/Inputs/inc-inexistent.rsp
@@ -0,0 +1 @@
+@inexistent.txt

diff --git a/clang/test/Driver/response-file-errs.c b/clang/test/Driver/response-file-errs.c
new file mode 100644 (file)
index 0000000..c0e02a9
--- /dev/null
+++ b/clang/test/Driver/response-file-errs.c
@@ -0,0 +1,15 @@
+// If response file does not exist, '@file; directive remains unexpanded in
+// command line.
+//
+// RUN: %clang @%S/Inputs/inexistent.rsp -### 2>&1 | FileCheck --check-prefix=INEXISTENT %s
+// INEXISTENT: @{{.*}}Inputs/inexistent.rsp
+
+// As the above case but '@file' is in response file.
+//
+// RUN: %clang @%S/Inputs/inc-inexistent.rsp -### 2>&1 | FileCheck --check-prefix=INEXISTENT2 %s
+// INEXISTENT2: @{{.*}}inexistent.txt
+
+// If file in `@file` is a directory, it is an error.
+//
+// RUN: not %clang @%S/Inputs -### 2>&1 | FileCheck --check-prefix=DIRECTORY %s
+// DIRECTORY: cannot not open file '{{.*}}Inputs': {{[Ii]}}s a directory

diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp
index 2cc3b48..4b1a246 100644 (file)
--- a/clang/tools/driver/driver.cpp
+++ b/clang/tools/driver/driver.cpp
@@ -378,7 +378,7 @@ int clang_main(int Argc, char **Argv) {
   llvm::cl::ExpansionContext ECtx(A, Tokenizer);
   ECtx.setMarkEOLs(MarkEOLs);
   if (llvm::Error Err = ECtx.expandResponseFiles(Args)) {
-    llvm::errs() << Err << '\n';
+    llvm::errs() << toString(std::move(Err)) << '\n';
     return 1;
   }
 

diff --git a/clang/unittests/Driver/ToolChainTest.cpp b/clang/unittests/Driver/ToolChainTest.cpp
index b143cd6..b45bab0 100644 (file)
--- a/clang/unittests/Driver/ToolChainTest.cpp
+++ b/clang/unittests/Driver/ToolChainTest.cpp
@@ -596,9 +596,10 @@ TEST(ToolChainTest, ConfigInexistentInclude) {
     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());
+    EXPECT_STRCASEEQ("cannot read configuration file '" USERCONFIG
+                     "': cannot not open file '" UNEXISTENT
+                     "': no such file or directory",
+                     DiagConsumer->Errors[0].c_str());
   }
 
 #undef USERCONFIG

diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index 136b813..fbaacbb 100644 (file)
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -1158,9 +1158,11 @@ Error ExpansionContext::expandResponseFile(
   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 + "'");
+  if (!MemBufOrErr) {
+    std::error_code EC = MemBufOrErr.getError();
+    return llvm::createStringError(EC, Twine("cannot not open file '") + FName +
+                                           "': " + EC.message());
+  }
   MemoryBuffer &MemBuf = *MemBufOrErr.get();
   StringRef Str(MemBuf.getBufferStart(), MemBuf.getBufferSize());
 
@@ -1262,7 +1264,7 @@ Error ExpansionContext::expandResponseFiles(
         if (auto CWD = FS->getCurrentWorkingDirectory()) {
           CurrDir = *CWD;
         } else {
-          return make_error<StringError>(
+          return createStringError(
               CWD.getError(), Twine("cannot get absolute path for: ") + FName);
         }
       } else {
@@ -1271,49 +1273,48 @@ Error ExpansionContext::expandResponseFiles(
       llvm::sys::path::append(CurrDir, FName);
       FName = CurrDir.c_str();
     }
+
+    ErrorOr<llvm::vfs::Status> Res = FS->status(FName);
+    if (!Res || !Res->exists()) {
+      std::error_code EC = Res.getError();
+      if (!InConfigFile) {
+        // If the specified file does not exist, leave '@file' unexpanded, as
+        // libiberty does.
+        if (!EC || EC == llvm::errc::no_such_file_or_directory) {
+          ++I;
+          continue;
+        }
+      }
+      if (!EC)
+        EC = llvm::errc::no_such_file_or_directory;
+      return createStringError(EC, Twine("cannot not open file '") + FName +
+                                       "': " + EC.message());
+    }
+    const llvm::vfs::Status &FileStatus = Res.get();
+
     auto IsEquivalent =
-        [FName, this](const ResponseFileRecord &RFile) -> ErrorOr<bool> {
-      ErrorOr<llvm::vfs::Status> LHS = FS->status(FName);
-      if (!LHS)
-        return LHS.getError();
+        [FileStatus, this](const ResponseFileRecord &RFile) -> ErrorOr<bool> {
       ErrorOr<llvm::vfs::Status> RHS = FS->status(RFile.File);
       if (!RHS)
         return RHS.getError();
-      return LHS->equivalent(*RHS);
+      return FileStatus.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());
+          return createStringError(
+              R.getError(), Twine("recursive expansion of: '") + F.File + "'");
       } else {
-        return make_error<StringError>(Twine("cannot open file: ") + F.File,
-                                       R.getError());
+        return createStringError(R.getError(),
+                                 Twine("cannot open file: ") + F.File);
       }
     }
 
     // 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 (Error Err = expandResponseFile(FName, ExpandedArgv))
       return Err;