[llvm][Support] Take in CurrentDirectory as a parameter in ExpandResponseFiles

[kadircet]

Summary

This is a follow-up to D70769 and D70222, which allows propagation of
current directory down to ExpandResponseFiles for handling of relative paths.

Previously clients had to mutate FS to achieve that, which is not thread-safe
and can even be thread-hostile in the case of real file system.

[sammccall]

llvm/include/llvm/Support/CommandLine.h

1972

This seems confusing.
Suppose we have @x/a, which includes @y/b

Relative=true, CurrentDir=none: we read ./x/y/b
Relative=true, CurrentDir=foo: we read foo/x/y/b
Relative=false, CurrentDir=none: we read ./y/b
Relative=false, CurrentDir=foo: we read... ./y/b?

Seems like it would be clearer to just say relative paths are relative to this directory, so the last one would be foo/y/b.

Actually looking at the code, I think that's what you implemented?

llvm/lib/Support/CommandLine.cpp

1105

This is @/BasePath I think (args are segments).

(I think this means you were running the wrong tests)

1183

this is subtle, consider a comment: CurrentDir is only relevant for "top-level" expansions || !RelativeNames, but nested ones always have absolute paths if RelativeNames so CurrentDir is ignored.

[kadircet]

• Address comments
• Change helper to take absolute paths, instead of taking a currentdir
• Update tests to make use of FS
• Add a new test for container-relative filenames

[merge_guards_bot]

Build result: pass - 60441 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt, CMakeCache.txt

[kadircet]

llvm/lib/Support/CommandLine.cpp

1183

did better, changed ExpandResponseFile to take FName as an absolute path, PTAL

[thakis]

Test fails on Windows: http://45.33.8.238/win/3379/step_10.txt

clang/include/clang/Tooling/CompilationDatabase.h

 /// Returns a wrapped CompilationDatabase that will add -target and -mode flags
 /// to commandline when they can be deduced from argv[0] of commandline returned
 /// by underlying database.
 std::unique_ptr<CompilationDatabase>
 inferTargetAndDriverMode(std::unique_ptr<CompilationDatabase> Base);
 
 /// Returns a wrapped CompilationDatabase that will expand all rsp(response)
 /// files on commandline returned by underlying database.
-/// Note: This may change the working directory of FS.
 std::unique_ptr<CompilationDatabase>
 expandResponseFiles(std::unique_ptr<CompilationDatabase> Base,
                     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
 
 } // namespace tooling
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLING_COMPILATIONDATABASE_H

clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp

 //===- ExpandResponseFileCompilationDataBase.cpp --------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
 
public:
 
   std::vector<CompileCommand> getAllCompileCommands() const override {
     return expand(Base->getAllCompileCommands());
   }
 
 private:
   std::vector<CompileCommand> expand(std::vector<CompileCommand> Cmds) const {
     for (auto &Cmd : Cmds) {
-      // FIXME: we should rather propagate the current directory into
-      // ExpandResponseFiles as well in addition to FS.
-      if (std::error_code EC = FS->setCurrentWorkingDirectory(Cmd.Directory)) {
-        llvm::consumeError(llvm::errorCodeToError(EC));
-        continue;
-      }
       bool SeenRSPFile = false;
       llvm::SmallVector<const char *, 20> Argv;
       Argv.reserve(Cmd.CommandLine.size());
       for (auto &Arg : Cmd.CommandLine) {
         Argv.push_back(Arg.c_str());
         SeenRSPFile |= Arg.front() == '@';
       }
       if (!SeenRSPFile)
         continue;
       llvm::BumpPtrAllocator Alloc;
       llvm::StringSaver Saver(Alloc);
-      llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS);
+      llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, *FS,
+                                    llvm::StringRef(Cmd.Directory));
       Cmd.CommandLine.assign(Argv.begin(), Argv.end());
     }
     return Cmds;
   }
 
 private:
   std::unique_ptr<CompilationDatabase> Base;
   llvm::cl::TokenizerCallback Tokenizer;

clang/lib/Tooling/JSONCompilationDatabase.cpp

 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <memory>
 #include <string>
 #include <system_error>
 #include <tuple>
 #include <utility>
class JSONCompilationDatabasePlugin : public CompilationDatabasePlugin {
   std::unique_ptr<CompilationDatabase>
   loadFromDirectory(StringRef Directory, std::string &ErrorMessage) override {
     SmallString<1024> JSONDatabasePath(Directory);
     llvm::sys::path::append(JSONDatabasePath, "compile_commands.json");
     auto Base = JSONCompilationDatabase::loadFromFile(
         JSONDatabasePath, ErrorMessage, JSONCommandLineSyntax::AutoDetect);
     return Base ? inferTargetAndDriverMode(
                       inferMissingCompileCommands(expandResponseFiles(
-                          std::move(Base),
-                          llvm::vfs::createPhysicalFileSystem().release())))
+                          std::move(Base), llvm::vfs::getRealFileSystem())))
                 : nullptr;
   }
 };
 
 } // namespace
 
 // Register the JSONCompilationDatabasePlugin with the
 // CompilationDatabasePluginRegistry using this statically initialized variable.

llvm/include/llvm/Support/CommandLine.h

 // the many example usages in tools/*/*.cpp
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_COMMANDLINE_H
 #define LLVM_SUPPORT_COMMANDLINE_H
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/ErrorHandling.h"

 /// \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] FS File system used for all file access when running the tool.
+/// \param [in] CurrentDir Path used to resolve relative rsp files when \p
+/// RelativeNames is set to true, when set to None, process' cwd is used
+/// instead.
 /// \return true if all @files were expanded successfully or there were none.
 bool ExpandResponseFiles(
     StringSaver &Saver, TokenizerCallback Tokenizer,
     SmallVectorImpl<const char *> &Argv, bool MarkEOLs = false,
     bool RelativeNames = false,
-    llvm::vfs::FileSystem &FS = *llvm::vfs::getRealFileSystem());
+    llvm::vfs::FileSystem &FS = *llvm::vfs::getRealFileSystem(),
+    llvm::Optional<llvm::StringRef> CurrentDir = llvm::None);
 
 /// Mark all options not part of this category as cl::ReallyHidden.
 ///
 /// \param Category the category of options to keep displaying
 ///
 /// Some tools (like clang-format) like to be able to hide all options that are
 /// not specific to the tool. This function allows a tool to specify a single
 /// option category to display in the -help output.

llvm/lib/Support/CommandLine.cpp

 #include "llvm-c/Support.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"

 }
 
 // It is called byte order marker but the UTF-8 BOM is actually not affected
 // by the host system's endianness.
 static bool hasUTF8ByteOrderMark(ArrayRef<char> S) {
   return (S.size() >= 3 && S[0] == '\xef' && S[1] == '\xbb' && S[2] == '\xbf');
 }
 
-static llvm::Error ExpandResponseFile(StringRef FName, StringSaver &Saver,
-                               TokenizerCallback Tokenizer,
-                               SmallVectorImpl<const char *> &NewArgv,
-                               bool MarkEOLs, bool RelativeNames,
-                               llvm::vfs::FileSystem &FS) {
-  llvm::ErrorOr<std::string> CurrDirOrErr = FS.getCurrentWorkingDirectory();
-  if (!CurrDirOrErr)
-    return llvm::errorCodeToError(CurrDirOrErr.getError());
+static llvm::Error ExpandResponseFile(
+    StringRef FName, StringSaver &Saver, TokenizerCallback Tokenizer,
+    SmallVectorImpl<const char *> &NewArgv, bool MarkEOLs, bool RelativeNames,
+    llvm::vfs::FileSystem &FS, llvm::Optional<llvm::StringRef> CurrentDir) {
+  SmallString<128> CurrDir;
+  if (llvm::sys::path::is_relative(FName)) {
+    if (!CurrentDir)
+      llvm::sys::fs::current_path(CurrDir);
+    else
+      CurrDir = *CurrentDir;
+    llvm::sys::path::append(CurrDir, FName);
+    FName = CurrDir.str();
+  }
+
   llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> MemBufOrErr =
       FS.getBufferForFile(FName);
   if (!MemBufOrErr)
     return llvm::errorCodeToError(MemBufOrErr.getError());
   MemoryBuffer &MemBuf = *MemBufOrErr.get();
   StringRef Str(MemBuf.getBufferStart(), MemBuf.getBufferSize());
 
   // If we have a UTF-16 byte order mark, convert to UTF-8 for parsing.
static llvm::Error ExpandResponseFile(
   // these bytes before parsing.
   // Reference: http://en.wikipedia.org/wiki/UTF-8#Byte_order_mark
   else if (hasUTF8ByteOrderMark(BufRef))
     Str = StringRef(BufRef.data() + 3, BufRef.size() - 3);
 
   // Tokenize the contents into NewArgv.
   Tokenizer(Str, Saver, NewArgv, MarkEOLs);
 
+  if (!RelativeNames)
+    return Error::success();
+  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.
-  if (RelativeNames)
-    for (unsigned I = 0; I < NewArgv.size(); ++I)
-      if (NewArgv[I]) {
-        StringRef Arg = NewArgv[I];
-        if (Arg.front() == '@') {
-          StringRef FileName = Arg.drop_front();
-          if (llvm::sys::path::is_relative(FileName)) {
-            SmallString<128> ResponseFile;
-            ResponseFile.append(1, '@');
-            if (llvm::sys::path::is_relative(FName)) {
-              ResponseFile.append(CurrDirOrErr.get());
-            }
-            llvm::sys::path::append(
-                ResponseFile, llvm::sys::path::parent_path(FName), FileName);
-            NewArgv[I] = Saver.save(ResponseFile.c_str()).data();
-          }
-        }
-      }
-
+  for (auto &Arg : NewArgv) {
+    // Skip non-rsp file arguments.
+    if (!Arg || Arg[0] != '@')
+      continue;
+
+    StringRef FileName(Arg + 1);
+    // Skip if non-relative.
+    if (!llvm::sys::path::is_relative(FileName))
+      continue;
+
+    llvm::sys::path::append(ResponseFile, "@", BasePath, FileName);
+    Arg = Saver.save(ResponseFile.c_str()).data();
   return Error::success();
 }
 
 /// 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, llvm::vfs::FileSystem &FS) {
+                             bool RelativeNames, llvm::vfs::FileSystem &FS,
+                             llvm::Optional<llvm::StringRef> CurrentDir) {
   bool AllExpanded = true;
   struct ResponseFileRecord {
     const char *File;
     size_t End;
   };
 
   // To detect recursive response files, we maintain a stack of files and the
   // position of the last argument in the file. This position is updated
if (std::any_of(FileStack.begin() + 1, FileStack.end(), IsEquivalent)) {
       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 (llvm::Error Err =
             ExpandResponseFile(FName, Saver, Tokenizer, ExpandedArgv, MarkEOLs,
-                               RelativeNames, FS)) {
+                               RelativeNames, FS, CurrentDir)) {
       // 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;
     }
bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
   return AllExpanded;
 }
 
 bool cl::readConfigFile(StringRef CfgFile, StringSaver &Saver,
                         SmallVectorImpl<const char *> &Argv) {
   if (llvm::Error Err =
           ExpandResponseFile(CfgFile, Saver, cl::tokenizeConfigFile, Argv,
                              /*MarkEOLs*/ false, /*RelativeNames*/ true,
-                             *llvm::vfs::getRealFileSystem())) {
+                             *llvm::vfs::getRealFileSystem(), llvm::None)) {
     // 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);
 }