keith updated this revision to Diff 372041.
keith added a comment.

Handle all cases by passing relative path to ExternalFS first


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109128

Files:
  clang/test/VFS/relative-path-errors.c
  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
@@ -1607,6 +1607,26 @@
   EXPECT_EQ(0, NumDiagnostics);
 }
 
+TEST_F(VFSFromYAMLTest, RelativePathFallthrough) {
+  IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS =
+      new vfs::InMemoryFileSystem;
+  BaseFS->addFile("//root/foo/a", 0, MemoryBuffer::getMemBuffer("contents of a"));
+  ASSERT_FALSE(BaseFS->setCurrentWorkingDirectory("//root/foo"));
+  auto RemappedFS = vfs::RedirectingFileSystem::create(
+      {}, /*UseExternalNames=*/false, *BaseFS);
+
+  auto OpenedF = RemappedFS->openFileForRead("a");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr<std::string> Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("a", Name.get());
+  EXPECT_EQ("a", OpenedS->getName());
+  EXPECT_FALSE(OpenedS->IsVFSMapped);
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
 TEST_F(VFSFromYAMLTest, CaseInsensitive) {
   IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
   Lower->addRegularFile("//root/foo/bar/a");
Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -1141,6 +1141,9 @@
   if (!exists(Path))
     return errc::no_such_file_or_directory;
 
+  if (ExternalFS)
+    ExternalFS->setCurrentWorkingDirectory(Path);
+
   SmallString<128> AbsolutePath;
   Path.toVector(AbsolutePath);
   if (std::error_code EC = makeAbsolute(AbsolutePath))
@@ -1149,15 +1152,17 @@
   return {};
 }
 
-std::error_code RedirectingFileSystem::isLocal(const Twine &Path_,
+std::error_code RedirectingFileSystem::isLocal(const Twine &Path,
                                                bool &Result) {
-  SmallString<256> Path;
-  Path_.toVector(Path);
+  if (!ExternalFS->isLocal(Path, Result))
+    return {};
 
-  if (std::error_code EC = makeCanonical(Path))
+  SmallString<256> CanonicalPath;
+  Path.toVector(CanonicalPath);
+  if (std::error_code EC = makeCanonical(CanonicalPath))
     return {};
 
-  return ExternalFS->isLocal(Path, Result);
+  return ExternalFS->isLocal(CanonicalPath, Result);
 }
 
 std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl<char> &Path) const {
@@ -1191,23 +1196,28 @@
 
 directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
                                                     std::error_code &EC) {
-  SmallString<256> Path;
-  Dir.toVector(Path);
+  SmallString<256> CanonicalPath;
+  Dir.toVector(CanonicalPath);
 
-  EC = makeCanonical(Path);
+  EC = makeCanonical(CanonicalPath);
   if (EC)
     return {};
 
-  ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
+  ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(CanonicalPath);
   if (!Result) {
     EC = Result.getError();
-    if (shouldFallBackToExternalFS(EC))
-      return ExternalFS->dir_begin(Path, EC);
+    if (shouldFallBackToExternalFS(EC)) {
+      directory_iterator Result = ExternalFS->dir_begin(Dir, EC);
+      if (!EC)
+        return Result;
+
+      return ExternalFS->dir_begin(CanonicalPath, EC);
+    }
     return {};
   }
 
   // Use status to make sure the path exists and refers to a directory.
-  ErrorOr<Status> S = status(Path, *Result);
+  ErrorOr<Status> S = status(CanonicalPath, *Result);
   if (!S) {
     if (shouldFallBackToExternalFS(S.getError(), Result->E))
       return ExternalFS->dir_begin(Dir, EC);
@@ -1231,18 +1241,18 @@
       // Update the paths in the results to use the virtual directory's path.
       DirIter =
           directory_iterator(std::make_shared<RedirectingFSDirRemapIterImpl>(
-              std::string(Path), DirIter));
+              std::string(CanonicalPath), DirIter));
     }
   } else {
     auto DE = cast<DirectoryEntry>(Result->E);
     DirIter = directory_iterator(std::make_shared<RedirectingFSDirIterImpl>(
-        Path, DE->contents_begin(), DE->contents_end(), EC));
+        CanonicalPath, DE->contents_begin(), DE->contents_end(), EC));
   }
 
   if (!shouldUseExternalFS())
     return DirIter;
   return directory_iterator(std::make_shared<CombiningDirIterImpl>(
-      DirIter, ExternalFS, std::string(Path), EC));
+      DirIter, ExternalFS, std::string(CanonicalPath), EC));
 }
 
 void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) {
@@ -1957,23 +1967,33 @@
   return Status::copyWithNewName(DE->getStatus(), Path);
 }
 
-ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path_) {
-  SmallString<256> Path;
-  Path_.toVector(Path);
+ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path) {
+  SmallString<256> CanonicalPath;
+  Path.toVector(CanonicalPath);
 
-  if (std::error_code EC = makeCanonical(Path))
+  if (std::error_code EC = makeCanonical(CanonicalPath))
     return EC;
 
-  ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
+  ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(CanonicalPath);
   if (!Result) {
-    if (shouldFallBackToExternalFS(Result.getError()))
-      return ExternalFS->status(Path);
+    if (shouldFallBackToExternalFS(Result.getError())) {
+      auto Result = ExternalFS->status(Path);
+      if (!Result.getError())
+        return Result;
+
+      return ExternalFS->status(CanonicalPath);
+    }
     return Result.getError();
   }
 
-  ErrorOr<Status> S = status(Path, *Result);
-  if (!S && shouldFallBackToExternalFS(S.getError(), Result->E))
-    S = ExternalFS->status(Path);
+  ErrorOr<Status> S = status(CanonicalPath, *Result);
+  if (!S && shouldFallBackToExternalFS(S.getError(), Result->E)) {
+    auto Result = ExternalFS->status(Path);
+    if (!Result.getError())
+      return Result;
+
+    S = ExternalFS->status(CanonicalPath);
+  }
   return S;
 }
 
@@ -2003,17 +2023,22 @@
 } // namespace
 
 ErrorOr<std::unique_ptr<File>>
-RedirectingFileSystem::openFileForRead(const Twine &Path_) {
-  SmallString<256> Path;
-  Path_.toVector(Path);
+RedirectingFileSystem::openFileForRead(const Twine &Path) {
+  SmallString<256> CanonicalPath;
+  Path.toVector(CanonicalPath);
 
-  if (std::error_code EC = makeCanonical(Path))
+  if (std::error_code EC = makeCanonical(CanonicalPath))
     return EC;
 
-  ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
+  ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(CanonicalPath);
   if (!Result) {
-    if (shouldFallBackToExternalFS(Result.getError()))
-      return ExternalFS->openFileForRead(Path);
+    if (shouldFallBackToExternalFS(Result.getError())) {
+      auto Result = ExternalFS->openFileForRead(Path);
+      if (!Result.getError())
+        return Result;
+
+      return ExternalFS->openFileForRead(CanonicalPath);
+    }
     return Result.getError();
   }
 
@@ -2025,8 +2050,13 @@
 
   auto ExternalFile = ExternalFS->openFileForRead(ExtRedirect);
   if (!ExternalFile) {
-    if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E))
-      return ExternalFS->openFileForRead(Path);
+    if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E)) {
+      auto Result = ExternalFS->openFileForRead(Path);
+      if (!Result.getError())
+        return Result;
+
+      return ExternalFS->openFileForRead(CanonicalPath);
+    }
     return ExternalFile;
   }
 
@@ -2036,24 +2066,28 @@
 
   // FIXME: Update the status with the name and VFSMapped.
   Status S = getRedirectedFileStatus(
-      Path, RE->useExternalName(UseExternalNames), *ExternalStatus);
+      CanonicalPath, RE->useExternalName(UseExternalNames), *ExternalStatus);
   return std::unique_ptr<File>(
       std::make_unique<FileWithFixedStatus>(std::move(*ExternalFile), S));
 }
 
 std::error_code
-RedirectingFileSystem::getRealPath(const Twine &Path_,
+RedirectingFileSystem::getRealPath(const Twine &Path,
                                    SmallVectorImpl<char> &Output) const {
-  SmallString<256> Path;
-  Path_.toVector(Path);
+  SmallString<256> CanonicalPath;
+  Path.toVector(CanonicalPath);
 
-  if (std::error_code EC = makeCanonical(Path))
+  if (std::error_code EC = makeCanonical(CanonicalPath))
     return EC;
 
-  ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
+  ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(CanonicalPath);
   if (!Result) {
-    if (shouldFallBackToExternalFS(Result.getError()))
-      return ExternalFS->getRealPath(Path, Output);
+    if (shouldFallBackToExternalFS(Result.getError())) {
+      if (!ExternalFS->getRealPath(Path, Output))
+          return {};
+
+      return ExternalFS->getRealPath(CanonicalPath, Output);
+    }
     return Result.getError();
   }
 
@@ -2062,15 +2096,23 @@
   if (auto ExtRedirect = Result->getExternalRedirect()) {
     auto P = ExternalFS->getRealPath(*ExtRedirect, Output);
     if (!P && shouldFallBackToExternalFS(P, Result->E)) {
-      return ExternalFS->getRealPath(Path, Output);
+      if (!ExternalFS->getRealPath(Path, Output))
+          return {};
+
+      return ExternalFS->getRealPath(CanonicalPath, Output);
     }
     return P;
   }
 
   // If we found a DirectoryEntry, still fall back to ExternalFS if allowed,
   // because directories don't have a single external contents path.
-  return shouldUseExternalFS() ? ExternalFS->getRealPath(Path, Output)
-                               : llvm::errc::invalid_argument;
+  if (shouldUseExternalFS()) {
+    if (!ExternalFS->getRealPath(Path, Output))
+      return {};
+    return ExternalFS->getRealPath(CanonicalPath, Output);
+  }
+
+  return llvm::errc::invalid_argument;
 }
 
 std::unique_ptr<FileSystem>
Index: clang/test/VFS/relative-path-errors.c
===================================================================
--- /dev/null
+++ clang/test/VFS/relative-path-errors.c
@@ -0,0 +1,8 @@
+// RUN: mkdir -p %t
+// RUN: cd %t
+// RUN: echo '{"roots": [],"version": 0}' > %t.yaml
+// RUN: cp %s main.c
+// RUN: not %clang_cc1 -ivfsoverlay %t.yaml main.c 2>&1 | FileCheck %s
+
+// CHECK: {{^}}main.c:8:1: error:
+foobarbaz
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to