[Lldb-commits] [PATCH] D59681: Update the lldb driver to support the -O and -S options when passing --repl

2019-03-21 Thread Nathan Hawes via Phabricator via lldb-commits
nathawes created this revision.
nathawes added reviewers: aprantl, jingham.
Herald added a subscriber: abidh.
Herald added a project: LLDB.

At the moment when `--repl` is passed to `lldb` it silently ignores any 
commands passed via the options below:

  --one-line-before-file 
   Tells the debugger to execute this one-line lldb command 
before any file provided on the command line has been loaded.
  --one-line  
   Tells the debugger to execute this one-line lldb command 
after any file provided on the command line has been loaded.  
  --source-before-file 
   Tells the debugger to read in and execute the lldb 
commands in the given file, before any file has been loaded.
  --source 
   Tells the debugger to read in and execute the lldb 
commands in the given file, after any file has been loaded.
  -OAlias for --one-line-before-file
  -oAlias for --one-line
  -SAlias for --source-before-file
  -sAlias for --source

The `-O` and `-S` options are quite useful when writing tests for the REPL 
though, e.g. to change settings prior to entering REPL mode. This patch updates 
the driver to still respect the commands supplied via `-O` and `-S` when 
passing `--repl` instead of silently ignoring them. As `-s` and `-o` don't 
really make sense in REPL mode, commands supplied via those options are still 
ignored, but the driver now emits a warning to make that clear to the user.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59681

Files:
  lldb/lit/Driver/TestRepl.test
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -609,47 +609,55 @@
   // are processed.
   WriteCommandsForSourcing(eCommandPlacementBeforeFile, commands_stream);
 
-  const size_t num_args = m_option_data.m_args.size();
-  if (num_args > 0) {
-char arch_name[64];
-if (lldb::SBDebugger::GetDefaultArchitecture(arch_name, sizeof(arch_name)))
-  commands_stream.Printf("target create --arch=%s %s", arch_name,
- EscapeString(m_option_data.m_args[0]).c_str());
-else
-  commands_stream.Printf("target create %s",
- EscapeString(m_option_data.m_args[0]).c_str());
-
-if (!m_option_data.m_core_file.empty()) {
-  commands_stream.Printf(" --core %s",
- EscapeString(m_option_data.m_core_file).c_str());
-}
-commands_stream.Printf("\n");
+  // If we're not in --repl mode, add the commands to process the file
+  // arguments, and the commands specified to run afterwards.
+  if (!m_option_data.m_repl) {
+const size_t num_args = m_option_data.m_args.size();
+if (num_args > 0) {
+  char arch_name[64];
+  if (lldb::SBDebugger::GetDefaultArchitecture(arch_name, sizeof(arch_name)))
+commands_stream.Printf("target create --arch=%s %s", arch_name,
+   EscapeString(m_option_data.m_args[0]).c_str());
+  else
+commands_stream.Printf("target create %s",
+   EscapeString(m_option_data.m_args[0]).c_str());
 
-if (num_args > 1) {
-  commands_stream.Printf("settings set -- target.run-args ");
-  for (size_t arg_idx = 1; arg_idx < num_args; ++arg_idx)
-commands_stream.Printf(
-" %s", EscapeString(m_option_data.m_args[arg_idx]).c_str());
+  if (!m_option_data.m_core_file.empty()) {
+commands_stream.Printf(" --core %s",
+   EscapeString(m_option_data.m_core_file).c_str());
+  }
   commands_stream.Printf("\n");
-}
-  } else if (!m_option_data.m_core_file.empty()) {
-commands_stream.Printf("target create --core %s\n",
-   EscapeString(m_option_data.m_core_file).c_str());
-  } else if (!m_option_data.m_process_name.empty()) {
-commands_stream.Printf("process attach --name %s",
-   EscapeString(m_option_data.m_process_name).c_str());
 
-if (m_option_data.m_wait_for)
-  commands_stream.Printf(" --waitfor");
+  if (num_args > 1) {
+commands_stream.Printf("settings set -- target.run-args ");
+for (size_t arg_idx = 1; arg_idx < num_args; ++arg_idx)
+  commands_stream.Printf(
+  " %s", EscapeString(m_option_data.m_args[arg_idx]).c_str());
+commands_stream.Printf("\n");
+  }
+} else if (!m_option_data.m_core_file.empty()) {
+  commands_stream.Printf("target create --core %s\n",
+ EscapeString(m_option_data.m_core_file).c_str());
+} else if (!m_option_data.m_process_name.empty()) {
+  commands_stream.Printf("process attach --name %s",
+ EscapeString(m_option_data.m_process_name).c_str());
+
+  if (m_option_data.m_wait_for)
+   

[Lldb-commits] [PATCH] D59681: Update the lldb driver to support the -O and -S options when passing --repl

2019-03-22 Thread Nathan Hawes via Phabricator via lldb-commits
nathawes marked an inline comment as done.
nathawes added inline comments.



Comment at: lldb/lit/Driver/TestRepl.test:2
+# RUN: echo ':quit' | %lldb -x --repl -O 'expr 42' -S %S/Inputs/Print2.in -o 
'expr something invalid' -s %s 2>&1 | FileCheck %s
+# CHECK: warning: commands specified to run after file load (via -o or -s) are 
ignored in REPL mode
+# CHECK: (int) $0 = 42

aprantl wrote:
> I think the REPL by default echoes all comments, so this may actually be 
> matching the comment in the input file rather than the warning?
> 
> I think this should do the trick:
> 
> `# CHECK: {{w}}arning: commands specified to run after file load (via -o or 
> -s) are ignored in REPL mode`
Good catch! I think this patch actually changes that behavior because the %lldb 
expansion includes `-S path/to/lldb/lit/lit-lldb-init` which sets 
echo-comment-commands false and isn't ignored anymore, but if we ever regress 
and stop respecting `-S` this test would still pass...


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59681/new/

https://reviews.llvm.org/D59681



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D59681: Update the lldb driver to support the -O and -S options when passing --repl

2019-03-22 Thread Nathan Hawes via Phabricator via lldb-commits
nathawes updated this revision to Diff 191904.
nathawes added a comment.

Updated the test based on feedback from @aprantl and also to specifically check 
we don't see the output from the -o and -s commands.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59681/new/

https://reviews.llvm.org/D59681

Files:
  lldb/lit/Driver/TestRepl.test
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -609,47 +609,55 @@
   // are processed.
   WriteCommandsForSourcing(eCommandPlacementBeforeFile, commands_stream);
 
-  const size_t num_args = m_option_data.m_args.size();
-  if (num_args > 0) {
-char arch_name[64];
-if (lldb::SBDebugger::GetDefaultArchitecture(arch_name, sizeof(arch_name)))
-  commands_stream.Printf("target create --arch=%s %s", arch_name,
- EscapeString(m_option_data.m_args[0]).c_str());
-else
-  commands_stream.Printf("target create %s",
- EscapeString(m_option_data.m_args[0]).c_str());
-
-if (!m_option_data.m_core_file.empty()) {
-  commands_stream.Printf(" --core %s",
- EscapeString(m_option_data.m_core_file).c_str());
-}
-commands_stream.Printf("\n");
+  // If we're not in --repl mode, add the commands to process the file
+  // arguments, and the commands specified to run afterwards.
+  if (!m_option_data.m_repl) {
+const size_t num_args = m_option_data.m_args.size();
+if (num_args > 0) {
+  char arch_name[64];
+  if (lldb::SBDebugger::GetDefaultArchitecture(arch_name, sizeof(arch_name)))
+commands_stream.Printf("target create --arch=%s %s", arch_name,
+   EscapeString(m_option_data.m_args[0]).c_str());
+  else
+commands_stream.Printf("target create %s",
+   EscapeString(m_option_data.m_args[0]).c_str());
 
-if (num_args > 1) {
-  commands_stream.Printf("settings set -- target.run-args ");
-  for (size_t arg_idx = 1; arg_idx < num_args; ++arg_idx)
-commands_stream.Printf(
-" %s", EscapeString(m_option_data.m_args[arg_idx]).c_str());
+  if (!m_option_data.m_core_file.empty()) {
+commands_stream.Printf(" --core %s",
+   EscapeString(m_option_data.m_core_file).c_str());
+  }
   commands_stream.Printf("\n");
-}
-  } else if (!m_option_data.m_core_file.empty()) {
-commands_stream.Printf("target create --core %s\n",
-   EscapeString(m_option_data.m_core_file).c_str());
-  } else if (!m_option_data.m_process_name.empty()) {
-commands_stream.Printf("process attach --name %s",
-   EscapeString(m_option_data.m_process_name).c_str());
 
-if (m_option_data.m_wait_for)
-  commands_stream.Printf(" --waitfor");
+  if (num_args > 1) {
+commands_stream.Printf("settings set -- target.run-args ");
+for (size_t arg_idx = 1; arg_idx < num_args; ++arg_idx)
+  commands_stream.Printf(
+  " %s", EscapeString(m_option_data.m_args[arg_idx]).c_str());
+commands_stream.Printf("\n");
+  }
+} else if (!m_option_data.m_core_file.empty()) {
+  commands_stream.Printf("target create --core %s\n",
+ EscapeString(m_option_data.m_core_file).c_str());
+} else if (!m_option_data.m_process_name.empty()) {
+  commands_stream.Printf("process attach --name %s",
+ EscapeString(m_option_data.m_process_name).c_str());
+
+  if (m_option_data.m_wait_for)
+commands_stream.Printf(" --waitfor");
 
-commands_stream.Printf("\n");
+  commands_stream.Printf("\n");
 
-  } else if (LLDB_INVALID_PROCESS_ID != m_option_data.m_process_pid) {
-commands_stream.Printf("process attach --pid %" PRIu64 "\n",
-   m_option_data.m_process_pid);
-  }
+} else if (LLDB_INVALID_PROCESS_ID != m_option_data.m_process_pid) {
+  commands_stream.Printf("process attach --pid %" PRIu64 "\n",
+ m_option_data.m_process_pid);
+}
 
-  WriteCommandsForSourcing(eCommandPlacementAfterFile, commands_stream);
+WriteCommandsForSourcing(eCommandPlacementAfterFile, commands_stream);
+  } else if (!m_option_data.m_after_file_commands.empty()) {
+// We're in repl mode and after-file-load commands were specified.
+WithColor::warning() << "commands specified to run after file load (via -o "
+  "or -s) are ignored in REPL mode.\n";
+  }
 
   if (GetDebugMode()) {
 result.PutError(m_debugger.GetErrorFileHandle());
@@ -659,100 +667,101 @@
   bool handle_events = true;
   bool spawn_thread = false;
 
-  if (m_option_data.m_repl) {
-const char *repl_options = nullptr;
-if (!m_option_data.m_repl_options.empty())
-  repl_options = m_option_data.m_rep

[Lldb-commits] [PATCH] D94857: [VFS] Combine VFSFromYamlDirIterImpl and OverlayFSDirIterImpl into a single implementation (NFC)

2021-01-28 Thread Nathan Hawes via Phabricator via lldb-commits
nathawes updated this revision to Diff 320031.
nathawes added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- Rename `error_code` to `EC` in `shouldFallBackToExternalFS()` to match 
existing conventions in the file.
- Update a missed `RedirectingFileEntry` reference in lldb to its new name 
(`FileEntry`)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94857/new/

https://reviews.llvm.org/D94857

Files:
  lldb/source/Host/common/FileSystem.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp

Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -452,19 +452,27 @@
 
 namespace {
 
-class OverlayFSDirIterImpl : public llvm::vfs::detail::DirIterImpl {
-  OverlayFileSystem &Overlays;
-  std::string Path;
-  OverlayFileSystem::iterator CurrentFS;
+/// Combines and deduplicates directory entries across multiple file systems.
+class CombiningDirIterImpl : public llvm::vfs::detail::DirIterImpl {
+  using FileSystemPtr = llvm::IntrusiveRefCntPtr;
+
+  /// File systems to check for entries in. Processed in reverse order.
+  SmallVector FSList;
+  /// The directory iterator for the current filesystem.
   directory_iterator CurrentDirIter;
+  /// The path of the directory to iterate the entries of.
+  std::string DirPath;
+  /// The set of names already returned as entries.
   llvm::StringSet<> SeenNames;
 
+  /// Sets \c CurrentDirIter to an iterator of \c DirPath in the next file
+  /// system in the list, or leaves it as is (at its end position) if we've
+  /// already gone through them all.
   std::error_code incrementFS() {
-assert(CurrentFS != Overlays.overlays_end() && "incrementing past end");
-++CurrentFS;
-for (auto E = Overlays.overlays_end(); CurrentFS != E; ++CurrentFS) {
+while (!FSList.empty()) {
   std::error_code EC;
-  CurrentDirIter = (*CurrentFS)->dir_begin(Path, EC);
+  CurrentDirIter = FSList.back()->dir_begin(DirPath, EC);
+  FSList.pop_back();
   if (EC && EC != errc::no_such_file_or_directory)
 return EC;
   if (CurrentDirIter != directory_iterator())
@@ -500,11 +508,24 @@
   }
 
 public:
-  OverlayFSDirIterImpl(const Twine &Path, OverlayFileSystem &FS,
+  CombiningDirIterImpl(ArrayRef FileSystems, std::string Dir,
std::error_code &EC)
-  : Overlays(FS), Path(Path.str()), CurrentFS(Overlays.overlays_begin()) {
-CurrentDirIter = (*CurrentFS)->dir_begin(Path, EC);
-EC = incrementImpl(true);
+  : FSList(FileSystems.begin(), FileSystems.end()),
+DirPath(std::move(Dir)) {
+if (!FSList.empty()) {
+  CurrentDirIter = FSList.back()->dir_begin(DirPath, EC);
+  FSList.pop_back();
+  if (!EC || EC == errc::no_such_file_or_directory)
+EC = incrementImpl(true);
+}
+  }
+
+  CombiningDirIterImpl(directory_iterator FirstIter, FileSystemPtr Fallback,
+   std::string FallbackDir, std::error_code &EC)
+  : FSList({Fallback}), CurrentDirIter(FirstIter),
+DirPath(std::move(FallbackDir)) {
+if (!EC || EC == errc::no_such_file_or_directory)
+  EC = incrementImpl(true);
   }
 
   std::error_code increment() override { return incrementImpl(false); }
@@ -515,7 +536,7 @@
 directory_iterator OverlayFileSystem::dir_begin(const Twine &Dir,
 std::error_code &EC) {
   return directory_iterator(
-  std::make_shared(Dir, *this, EC));
+  std::make_shared(FSList, Dir.str(), EC));
 }
 
 void ProxyFileSystem::anchor() {}
@@ -1019,48 +1040,47 @@
 }
 }
 
-// FIXME: reuse implementation common with OverlayFSDirIterImpl as these
-// iterators are conceptually similar.
-class llvm::vfs::VFSFromYamlDirIterImpl
+/// Directory iterator implementation for \c RedirectingFileSystem's
+/// directory entries.
+class llvm::vfs::RedirectingFSDirIterImpl
 : public llvm::vfs::detail::DirIterImpl {
   std::string Dir;
-  RedirectingFileSystem::RedirectingDirectoryEntry::iterator Current, End;
-
-  // To handle 'fallthrough' mode we need to iterate at first through
-  // RedirectingDirectoryEntry and then through ExternalFS. These operations are
-  // done sequentially, we just need to keep a track of what kind of iteration
-  // we are currently performing.
-
-  /// Flag telling if we should iterate through ExternalFS or stop at the last
-  /// RedirectingDirectoryEntry::iterator.
-  bool IterateExternalFS;
-  /// Flag telling if we have switched to iterating through ExternalFS.
-  bool IsExternalFSCurrent = false;
-  FileSystem &ExternalFS;
-  directory_iterator ExternalDirIter;
-  llvm::StringSet<> SeenNames;
-
-  /// To combine multiple iterations, different methods are responsible for
-  /// different iteration steps.
-  /// @{
+  RedirectingFileSystem

[Lldb-commits] [PATCH] D94857: [VFS] Combine VFSFromYamlDirIterImpl and OverlayFSDirIterImpl into a single implementation (NFC)

2021-01-29 Thread Nathan Hawes via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG719f77844175: [VFS] Combine VFSFromYamlDirIterImpl and 
OverlayFSDirIterImpl into a single… (authored by nathawes).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94857/new/

https://reviews.llvm.org/D94857

Files:
  lldb/source/Host/common/FileSystem.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp

Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -452,19 +452,27 @@
 
 namespace {
 
-class OverlayFSDirIterImpl : public llvm::vfs::detail::DirIterImpl {
-  OverlayFileSystem &Overlays;
-  std::string Path;
-  OverlayFileSystem::iterator CurrentFS;
+/// Combines and deduplicates directory entries across multiple file systems.
+class CombiningDirIterImpl : public llvm::vfs::detail::DirIterImpl {
+  using FileSystemPtr = llvm::IntrusiveRefCntPtr;
+
+  /// File systems to check for entries in. Processed in reverse order.
+  SmallVector FSList;
+  /// The directory iterator for the current filesystem.
   directory_iterator CurrentDirIter;
+  /// The path of the directory to iterate the entries of.
+  std::string DirPath;
+  /// The set of names already returned as entries.
   llvm::StringSet<> SeenNames;
 
+  /// Sets \c CurrentDirIter to an iterator of \c DirPath in the next file
+  /// system in the list, or leaves it as is (at its end position) if we've
+  /// already gone through them all.
   std::error_code incrementFS() {
-assert(CurrentFS != Overlays.overlays_end() && "incrementing past end");
-++CurrentFS;
-for (auto E = Overlays.overlays_end(); CurrentFS != E; ++CurrentFS) {
+while (!FSList.empty()) {
   std::error_code EC;
-  CurrentDirIter = (*CurrentFS)->dir_begin(Path, EC);
+  CurrentDirIter = FSList.back()->dir_begin(DirPath, EC);
+  FSList.pop_back();
   if (EC && EC != errc::no_such_file_or_directory)
 return EC;
   if (CurrentDirIter != directory_iterator())
@@ -500,11 +508,24 @@
   }
 
 public:
-  OverlayFSDirIterImpl(const Twine &Path, OverlayFileSystem &FS,
+  CombiningDirIterImpl(ArrayRef FileSystems, std::string Dir,
std::error_code &EC)
-  : Overlays(FS), Path(Path.str()), CurrentFS(Overlays.overlays_begin()) {
-CurrentDirIter = (*CurrentFS)->dir_begin(Path, EC);
-EC = incrementImpl(true);
+  : FSList(FileSystems.begin(), FileSystems.end()),
+DirPath(std::move(Dir)) {
+if (!FSList.empty()) {
+  CurrentDirIter = FSList.back()->dir_begin(DirPath, EC);
+  FSList.pop_back();
+  if (!EC || EC == errc::no_such_file_or_directory)
+EC = incrementImpl(true);
+}
+  }
+
+  CombiningDirIterImpl(directory_iterator FirstIter, FileSystemPtr Fallback,
+   std::string FallbackDir, std::error_code &EC)
+  : FSList({Fallback}), CurrentDirIter(FirstIter),
+DirPath(std::move(FallbackDir)) {
+if (!EC || EC == errc::no_such_file_or_directory)
+  EC = incrementImpl(true);
   }
 
   std::error_code increment() override { return incrementImpl(false); }
@@ -515,7 +536,7 @@
 directory_iterator OverlayFileSystem::dir_begin(const Twine &Dir,
 std::error_code &EC) {
   return directory_iterator(
-  std::make_shared(Dir, *this, EC));
+  std::make_shared(FSList, Dir.str(), EC));
 }
 
 void ProxyFileSystem::anchor() {}
@@ -1019,48 +1040,47 @@
 }
 }
 
-// FIXME: reuse implementation common with OverlayFSDirIterImpl as these
-// iterators are conceptually similar.
-class llvm::vfs::VFSFromYamlDirIterImpl
+/// Directory iterator implementation for \c RedirectingFileSystem's
+/// directory entries.
+class llvm::vfs::RedirectingFSDirIterImpl
 : public llvm::vfs::detail::DirIterImpl {
   std::string Dir;
-  RedirectingFileSystem::RedirectingDirectoryEntry::iterator Current, End;
-
-  // To handle 'fallthrough' mode we need to iterate at first through
-  // RedirectingDirectoryEntry and then through ExternalFS. These operations are
-  // done sequentially, we just need to keep a track of what kind of iteration
-  // we are currently performing.
-
-  /// Flag telling if we should iterate through ExternalFS or stop at the last
-  /// RedirectingDirectoryEntry::iterator.
-  bool IterateExternalFS;
-  /// Flag telling if we have switched to iterating through ExternalFS.
-  bool IsExternalFSCurrent = false;
-  FileSystem &ExternalFS;
-  directory_iterator ExternalDirIter;
-  llvm::StringSet<> SeenNames;
-
-  /// To combine multiple iterations, different methods are responsible for
-  /// different iteration steps.
-  /// @{
+  RedirectingFileSystem::DirectoryEntry::iterator Current, End;
 
-  /// Responsible for dispatching b

[Lldb-commits] [PATCH] D94844: [VFS] Add support to RedirectingFileSystem for mapping a virtual directory to one in the external FS.

2021-01-29 Thread Nathan Hawes via Phabricator via lldb-commits
nathawes updated this revision to Diff 320257.
nathawes marked an inline comment as done.
nathawes edited the summary of this revision.
nathawes set the repository for this revision to rG LLVM Github Monorepo.
nathawes added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- Updated shouldFallBackToExternalFS() to accept an Entry * rather than an 
Optional to avoid an unnecessary copy.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94844/new/

https://reviews.llvm.org/D94844

Files:
  clang/test/VFS/Inputs/vfsoverlay-directory-relative.yaml
  clang/test/VFS/Inputs/vfsoverlay-directory.yaml
  clang/test/VFS/directory.c
  lldb/source/Host/common/FileSystem.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1328,6 +1328,7 @@
 
 TEST_F(VFSFromYAMLTest, MappedFiles) {
   IntrusiveRefCntPtr Lower(new DummyFileSystem());
+  Lower->addDirectory("//root/foo/bar");
   Lower->addRegularFile("//root/foo/bar/a");
   IntrusiveRefCntPtr FS = getFromYAMLString(
   "{ 'roots': [\n"
@@ -1343,6 +1344,17 @@
   "  'type': 'file',\n"
   "  'name': 'file2',\n"
   "  'external-contents': '//root/foo/b'\n"
+  "},\n"
+  "{\n"
+  "  'type': 'directory-remap',\n"
+  "  'name': 'mappeddir',\n"
+  "  'external-contents': '//root/foo/bar'\n"
+  "},\n"
+  "{\n"
+  "  'type': 'directory-remap',\n"
+  "  'name': 'mappeddir2',\n"
+  "  'use-external-name': false,\n"
+  "  'external-contents': '//root/foo/bar'\n"
   "}\n"
   "  ]\n"
   "}\n"
@@ -1380,12 +1392,221 @@
   EXPECT_TRUE(S->isDirectory());
   EXPECT_TRUE(S->equivalent(*O->status("//root/"))); // non-volatile UniqueID
 
+  // remapped directory
+  S = O->status("//root/mappeddir");
+  ASSERT_FALSE(S.getError());
+  EXPECT_TRUE(S->isDirectory());
+  EXPECT_TRUE(S->IsVFSMapped);
+  EXPECT_TRUE(S->equivalent(*O->status("//root/foo/bar")));
+
+  SLower = O->status("//root/foo/bar");
+  EXPECT_EQ("//root/foo/bar", SLower->getName());
+  EXPECT_TRUE(S->equivalent(*SLower));
+  EXPECT_FALSE(SLower->IsVFSMapped);
+
+  // file in remapped directory
+  S = O->status("//root/mappeddir/a");
+  ASSERT_FALSE(S.getError());
+  ASSERT_FALSE(S->isDirectory());
+  ASSERT_TRUE(S->IsVFSMapped);
+  ASSERT_EQ("//root/foo/bar/a", S->getName());
+
+  // file in remapped directory, with use-external-name=false
+  S = O->status("//root/mappeddir2/a");
+  ASSERT_FALSE(S.getError());
+  ASSERT_FALSE(S->isDirectory());
+  ASSERT_TRUE(S->IsVFSMapped);
+  ASSERT_EQ("//root/mappeddir2/a", S->getName());
+
+  // file contents in remapped directory
+  OpenedF = O->openFileForRead("//root/mappeddir/a");
+  ASSERT_FALSE(OpenedF.getError());
+  OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("//root/foo/bar/a", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+  // file contents in remapped directory, with use-external-name=false
+  OpenedF = O->openFileForRead("//root/mappeddir2/a");
+  ASSERT_FALSE(OpenedF.getError());
+  OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("//root/mappeddir2/a", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
   // broken mapping
   EXPECT_EQ(O->status("//root/file2").getError(),
 llvm::errc::no_such_file_or_directory);
   EXPECT_EQ(0, NumDiagnostics);
 }
 
+TEST_F(VFSFromYAMLTest, MappedRoot) {
+  IntrusiveRefCntPtr Lower(new DummyFileSystem());
+  Lower->addDirectory("//root/foo/bar");
+  Lower->addRegularFile("//root/foo/bar/a");
+  IntrusiveRefCntPtr FS =
+  getFromYAMLString("{ 'roots': [\n"
+"{\n"
+"  'type': 'directory-remap',\n"
+"  'name': '//mappedroot/',\n"
+"  'external-contents': '//root/foo/bar'\n"
+"}\n"
+"]\n"
+"}",
+Lower);
+  ASSERT_TRUE(FS.get() != nullptr);
+
+  IntrusiveRefCntPtr O(
+  new vfs::OverlayFileSystem(Lower));
+  O->pushOverlay(FS);
+
+  // file
+  ErrorOr S = O->status("//mappedroot/a");
+  ASSERT_FALSE(S.getError());
+  EXPECT_EQ("//root/foo/bar/a", S->getName());
+  EXPECT_TRUE(S->IsVFSMapped);
+
+  ErrorOr SLower = O->status("//root/foo/bar/a");
+  EXPECT_EQ("//root/foo/bar/a", SLower->getName());
+  EXPECT_TRUE(S->equivalent(*SLower

[Lldb-commits] [PATCH] D94844: [VFS] Add support to RedirectingFileSystem for mapping a virtual directory to one in the external FS.

2021-02-01 Thread Nathan Hawes via Phabricator via lldb-commits
nathawes updated this revision to Diff 320646.
nathawes added a comment.

Made the following changes as a speculative fix for the windows test failures 
in the pre-merge checks:

- When appending the remaining path components to the external redirect path of 
a directory-remap entry, use the separator type of the redirect path rather 
than the host system.
- For a directory-remap entry, when changing the paths returned in the external 
file system's directory iterator to appear to be in the virtual file system's 
directory, use the separator type from the virtual directory's path rather than 
the host system's.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94844/new/

https://reviews.llvm.org/D94844

Files:
  clang/test/VFS/Inputs/vfsoverlay-directory-relative.yaml
  clang/test/VFS/Inputs/vfsoverlay-directory.yaml
  clang/test/VFS/directory.c
  lldb/source/Host/common/FileSystem.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1328,6 +1328,7 @@
 
 TEST_F(VFSFromYAMLTest, MappedFiles) {
   IntrusiveRefCntPtr Lower(new DummyFileSystem());
+  Lower->addDirectory("//root/foo/bar");
   Lower->addRegularFile("//root/foo/bar/a");
   IntrusiveRefCntPtr FS = getFromYAMLString(
   "{ 'roots': [\n"
@@ -1343,6 +1344,17 @@
   "  'type': 'file',\n"
   "  'name': 'file2',\n"
   "  'external-contents': '//root/foo/b'\n"
+  "},\n"
+  "{\n"
+  "  'type': 'directory-remap',\n"
+  "  'name': 'mappeddir',\n"
+  "  'external-contents': '//root/foo/bar'\n"
+  "},\n"
+  "{\n"
+  "  'type': 'directory-remap',\n"
+  "  'name': 'mappeddir2',\n"
+  "  'use-external-name': false,\n"
+  "  'external-contents': '//root/foo/bar'\n"
   "}\n"
   "  ]\n"
   "}\n"
@@ -1380,12 +1392,221 @@
   EXPECT_TRUE(S->isDirectory());
   EXPECT_TRUE(S->equivalent(*O->status("//root/"))); // non-volatile UniqueID
 
+  // remapped directory
+  S = O->status("//root/mappeddir");
+  ASSERT_FALSE(S.getError());
+  EXPECT_TRUE(S->isDirectory());
+  EXPECT_TRUE(S->IsVFSMapped);
+  EXPECT_TRUE(S->equivalent(*O->status("//root/foo/bar")));
+
+  SLower = O->status("//root/foo/bar");
+  EXPECT_EQ("//root/foo/bar", SLower->getName());
+  EXPECT_TRUE(S->equivalent(*SLower));
+  EXPECT_FALSE(SLower->IsVFSMapped);
+
+  // file in remapped directory
+  S = O->status("//root/mappeddir/a");
+  ASSERT_FALSE(S.getError());
+  ASSERT_FALSE(S->isDirectory());
+  ASSERT_TRUE(S->IsVFSMapped);
+  ASSERT_EQ("//root/foo/bar/a", S->getName());
+
+  // file in remapped directory, with use-external-name=false
+  S = O->status("//root/mappeddir2/a");
+  ASSERT_FALSE(S.getError());
+  ASSERT_FALSE(S->isDirectory());
+  ASSERT_TRUE(S->IsVFSMapped);
+  ASSERT_EQ("//root/mappeddir2/a", S->getName());
+
+  // file contents in remapped directory
+  OpenedF = O->openFileForRead("//root/mappeddir/a");
+  ASSERT_FALSE(OpenedF.getError());
+  OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("//root/foo/bar/a", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+  // file contents in remapped directory, with use-external-name=false
+  OpenedF = O->openFileForRead("//root/mappeddir2/a");
+  ASSERT_FALSE(OpenedF.getError());
+  OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("//root/mappeddir2/a", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
   // broken mapping
   EXPECT_EQ(O->status("//root/file2").getError(),
 llvm::errc::no_such_file_or_directory);
   EXPECT_EQ(0, NumDiagnostics);
 }
 
+TEST_F(VFSFromYAMLTest, MappedRoot) {
+  IntrusiveRefCntPtr Lower(new DummyFileSystem());
+  Lower->addDirectory("//root/foo/bar");
+  Lower->addRegularFile("//root/foo/bar/a");
+  IntrusiveRefCntPtr FS =
+  getFromYAMLString("{ 'roots': [\n"
+"{\n"
+"  'type': 'directory-remap',\n"
+"  'name': '//mappedroot/',\n"
+"  'external-contents': '//root/foo/bar'\n"
+"}\n"
+"]\n"
+"}",
+Lower);
+  ASSERT_TRUE(FS.get() != nullptr);
+
+  IntrusiveRefCntPtr O(
+  new vfs::OverlayFileSystem(Lower));
+  O->pushOverlay(FS);
+
+  // file
+  ErrorOr S = O->status("//mappedroot/a");
+  ASSERT_FALSE(S.getError());
+  EXPECT_EQ("//root/foo/b

[Lldb-commits] [PATCH] D94844: [VFS] Add support to RedirectingFileSystem for mapping a virtual directory to one in the external FS.

2021-02-01 Thread Nathan Hawes via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGecb00a77624c: [VFS] Add support to RedirectingFileSystem for 
mapping a virtual directory to… (authored by nathawes).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94844/new/

https://reviews.llvm.org/D94844

Files:
  clang/test/VFS/Inputs/vfsoverlay-directory-relative.yaml
  clang/test/VFS/Inputs/vfsoverlay-directory.yaml
  clang/test/VFS/directory.c
  lldb/source/Host/common/FileSystem.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1328,6 +1328,7 @@
 
 TEST_F(VFSFromYAMLTest, MappedFiles) {
   IntrusiveRefCntPtr Lower(new DummyFileSystem());
+  Lower->addDirectory("//root/foo/bar");
   Lower->addRegularFile("//root/foo/bar/a");
   IntrusiveRefCntPtr FS = getFromYAMLString(
   "{ 'roots': [\n"
@@ -1343,6 +1344,17 @@
   "  'type': 'file',\n"
   "  'name': 'file2',\n"
   "  'external-contents': '//root/foo/b'\n"
+  "},\n"
+  "{\n"
+  "  'type': 'directory-remap',\n"
+  "  'name': 'mappeddir',\n"
+  "  'external-contents': '//root/foo/bar'\n"
+  "},\n"
+  "{\n"
+  "  'type': 'directory-remap',\n"
+  "  'name': 'mappeddir2',\n"
+  "  'use-external-name': false,\n"
+  "  'external-contents': '//root/foo/bar'\n"
   "}\n"
   "  ]\n"
   "}\n"
@@ -1380,12 +1392,221 @@
   EXPECT_TRUE(S->isDirectory());
   EXPECT_TRUE(S->equivalent(*O->status("//root/"))); // non-volatile UniqueID
 
+  // remapped directory
+  S = O->status("//root/mappeddir");
+  ASSERT_FALSE(S.getError());
+  EXPECT_TRUE(S->isDirectory());
+  EXPECT_TRUE(S->IsVFSMapped);
+  EXPECT_TRUE(S->equivalent(*O->status("//root/foo/bar")));
+
+  SLower = O->status("//root/foo/bar");
+  EXPECT_EQ("//root/foo/bar", SLower->getName());
+  EXPECT_TRUE(S->equivalent(*SLower));
+  EXPECT_FALSE(SLower->IsVFSMapped);
+
+  // file in remapped directory
+  S = O->status("//root/mappeddir/a");
+  ASSERT_FALSE(S.getError());
+  ASSERT_FALSE(S->isDirectory());
+  ASSERT_TRUE(S->IsVFSMapped);
+  ASSERT_EQ("//root/foo/bar/a", S->getName());
+
+  // file in remapped directory, with use-external-name=false
+  S = O->status("//root/mappeddir2/a");
+  ASSERT_FALSE(S.getError());
+  ASSERT_FALSE(S->isDirectory());
+  ASSERT_TRUE(S->IsVFSMapped);
+  ASSERT_EQ("//root/mappeddir2/a", S->getName());
+
+  // file contents in remapped directory
+  OpenedF = O->openFileForRead("//root/mappeddir/a");
+  ASSERT_FALSE(OpenedF.getError());
+  OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("//root/foo/bar/a", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+  // file contents in remapped directory, with use-external-name=false
+  OpenedF = O->openFileForRead("//root/mappeddir2/a");
+  ASSERT_FALSE(OpenedF.getError());
+  OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("//root/mappeddir2/a", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
   // broken mapping
   EXPECT_EQ(O->status("//root/file2").getError(),
 llvm::errc::no_such_file_or_directory);
   EXPECT_EQ(0, NumDiagnostics);
 }
 
+TEST_F(VFSFromYAMLTest, MappedRoot) {
+  IntrusiveRefCntPtr Lower(new DummyFileSystem());
+  Lower->addDirectory("//root/foo/bar");
+  Lower->addRegularFile("//root/foo/bar/a");
+  IntrusiveRefCntPtr FS =
+  getFromYAMLString("{ 'roots': [\n"
+"{\n"
+"  'type': 'directory-remap',\n"
+"  'name': '//mappedroot/',\n"
+"  'external-contents': '//root/foo/bar'\n"
+"}\n"
+"]\n"
+"}",
+Lower);
+  ASSERT_TRUE(FS.get() != nullptr);
+
+  IntrusiveRefCntPtr O(
+  new vfs::OverlayFileSystem(Lower));
+  O->pushOverlay(FS);
+
+  // file
+  ErrorOr S = O->status("//mappedroot/a");
+  ASSERT_FALSE(S.getError());
+  EXPECT_EQ("//root/foo/bar/a", S->getName());
+  EXPECT_TRUE(S->IsVFSMapped);
+
+  ErrorOr SLower = O->status("//root/foo/bar/a");
+  EXPECT_EQ("//root/foo/bar/a", SLower->getName());
+  EXPECT_TRUE(S->equivalent(*SLower));
+  EXPECT_FALSE(SLower->IsVFSMapped);
+
+  // file after opening
+  auto OpenedF = O->openFileForRead("//mappedroot/a");
+  ASSERT_FALSE(OpenedF.getError