bnbarham updated this revision to Diff 402130.
bnbarham added a comment.
Modified `directory.c` to make the test difference clearer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117730/new/
https://reviews.llvm.org/D117730
Files:
clang/test/VFS/directory.c
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/FileCollector.cpp
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
@@ -64,10 +64,13 @@
return I->second;
}
ErrorOr<std::unique_ptr<vfs::File>>
- openFileForRead(const Twine &Path) override {
- auto S = status(Path);
- if (S)
- return std::unique_ptr<vfs::File>(new DummyFile{*S});
+ openFileForReadImpl(const Twine &LookupPath,
+ const Twine &OriginalPath) override {
+ auto S = status(LookupPath);
+ if (S) {
+ auto FixedS = vfs::Status::copyWithNewName(*S, OriginalPath);
+ return std::unique_ptr<vfs::File>(new DummyFile{FixedS});
+ }
return S.getError();
}
llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
@@ -2708,6 +2711,80 @@
EXPECT_FALSE(FS->exists(_c.path("c")));
}
+TEST_F(VFSFromYAMLTest, MultipleRemappedDirectories) {
+ // Test the interaction of two overlays where one maps back to the other,
+ // ie. `a` -> `b` and then `b` -> `a`. This should always use `a` if it
+ // exists and fallback to `b` otherwise.
+
+ IntrusiveRefCntPtr<DummyFileSystem> Both(new DummyFileSystem());
+ Both->addDirectory("//root/a");
+ Both->addRegularFile("//root/a/f");
+ Both->addDirectory("//root/b");
+ Both->addRegularFile("//root/b/f");
+
+ IntrusiveRefCntPtr<DummyFileSystem> AOnly(new DummyFileSystem());
+ AOnly->addDirectory("//root/a");
+ AOnly->addRegularFile("//root/a/f");
+
+ IntrusiveRefCntPtr<DummyFileSystem> BOnly(new DummyFileSystem());
+ BOnly->addDirectory("//root/b");
+ BOnly->addRegularFile("//root/b/f");
+
+ auto AToBStr = "{ 'roots': [\n"
+ " {\n"
+ " 'type': 'directory-remap',\n"
+ " 'name': '//root/a',\n"
+ " 'external-contents': '//root/b'\n"
+ " }"
+ "]}";
+ auto BToAStr = "{ 'roots': [\n"
+ " {\n"
+ " 'type': 'directory-remap',\n"
+ " 'name': '//root/b',\n"
+ " 'external-contents': '//root/a'\n"
+ " }"
+ "]}";
+
+ auto ExpectPath = [&](vfs::FileSystem &FS, StringRef Expected) {
+ auto AF = FS.openFileForRead("//root/a/f");
+ ASSERT_FALSE(AF.getError());
+ auto AFName = (*AF)->getName();
+ ASSERT_FALSE(AFName.getError());
+ EXPECT_EQ(Expected.str(), AFName.get());
+
+ auto AS = FS.status("//root/a/f");
+ ASSERT_FALSE(AS.getError());
+ EXPECT_EQ(Expected.str(), AS->getName());
+
+ auto BF = FS.openFileForRead("//root/b/f");
+ ASSERT_FALSE(BF.getError());
+ auto BName = (*BF)->getName();
+ ASSERT_FALSE(BName.getError());
+ EXPECT_EQ(Expected.str(), BName.get());
+
+ auto BS = FS.status("//root/b/f");
+ ASSERT_FALSE(BS.getError());
+ EXPECT_EQ(Expected.str(), BS->getName());
+ };
+
+ IntrusiveRefCntPtr<vfs::FileSystem> BothFS =
+ getFromYAMLString(AToBStr, getFromYAMLString(BToAStr, Both));
+ ASSERT_TRUE(BothFS.get() != nullptr);
+ ExpectPath(*BothFS, "//root/a/f");
+
+ IntrusiveRefCntPtr<vfs::FileSystem> OnlyAFS =
+ getFromYAMLString(AToBStr, getFromYAMLString(BToAStr, AOnly));
+ ASSERT_TRUE(OnlyAFS.get() != nullptr);
+ ExpectPath(*OnlyAFS, "//root/a/f");
+
+ IntrusiveRefCntPtr<vfs::FileSystem> OnlyBFS =
+ getFromYAMLString(AToBStr, getFromYAMLString(BToAStr, BOnly));
+ ASSERT_TRUE(OnlyBFS.get() != nullptr);
+ ExpectPath(*OnlyBFS, "//root/b/f");
+
+ EXPECT_EQ(0, NumDiagnostics);
+}
+
TEST(VFSFromRemappedFilesTest, Basic) {
IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS =
new vfs::InMemoryFileSystem;
Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -267,7 +267,9 @@
}
ErrorOr<Status> status(const Twine &Path) override;
- ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) override;
+ ErrorOr<std::unique_ptr<File>>
+ openFileForReadImpl(const Twine &LookupPath,
+ const Twine &OriginalPath) override;
directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;
llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override;
@@ -308,14 +310,15 @@
}
ErrorOr<std::unique_ptr<File>>
-RealFileSystem::openFileForRead(const Twine &Name) {
+RealFileSystem::openFileForReadImpl(const Twine &LookupPath,
+ const Twine &OriginalPath) {
SmallString<256> RealName, Storage;
Expected<file_t> FDOrErr = sys::fs::openNativeFileForRead(
- adjustPath(Name, Storage), sys::fs::OF_None, &RealName);
+ adjustPath(LookupPath, Storage), sys::fs::OF_None, &RealName);
if (!FDOrErr)
return errorToErrorCode(FDOrErr.takeError());
return std::unique_ptr<File>(
- new RealFile(*FDOrErr, Name.str(), RealName.str()));
+ new RealFile(*FDOrErr, OriginalPath.str(), RealName.str()));
}
llvm::ErrorOr<std::string> RealFileSystem::getCurrentWorkingDirectory() const {
@@ -422,10 +425,11 @@
}
ErrorOr<std::unique_ptr<File>>
-OverlayFileSystem::openFileForRead(const llvm::Twine &Path) {
+OverlayFileSystem::openFileForReadImpl(const Twine &LookupPath,
+ const Twine &OriginalPath) {
// FIXME: handle symlinks that cross file systems
for (iterator I = overlays_begin(), E = overlays_end(); I != E; ++I) {
- auto Result = (*I)->openFileForRead(Path);
+ auto Result = (*I)->openFileForReadImpl(LookupPath, OriginalPath);
if (Result || Result.getError() != llvm::errc::no_such_file_or_directory)
return Result;
}
@@ -928,8 +932,9 @@
}
llvm::ErrorOr<std::unique_ptr<File>>
-InMemoryFileSystem::openFileForRead(const Twine &Path) {
- auto Node = lookupInMemoryNode(*this, Root.get(), Path);
+InMemoryFileSystem::openFileForReadImpl(const Twine &LookupPath,
+ const Twine &OriginalPath) {
+ auto Node = lookupInMemoryNode(*this, Root.get(), LookupPath);
if (!Node)
return Node.getError();
@@ -937,7 +942,7 @@
// to match the ownership semantics for File.
if (auto *F = dyn_cast<detail::InMemoryFile>(*Node))
return std::unique_ptr<File>(
- new detail::InMemoryFileAdaptor(*F, Path.str()));
+ new detail::InMemoryFileAdaptor(*F, OriginalPath.str()));
// FIXME: errc::not_a_file?
return make_error_code(llvm::errc::invalid_argument);
@@ -2100,9 +2105,10 @@
}
ErrorOr<std::unique_ptr<File>>
-RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) {
+RedirectingFileSystem::openFileForReadImpl(const Twine &LookupPath,
+ const Twine &OriginalPath) {
SmallString<256> CanonicalPath;
- OriginalPath.toVector(CanonicalPath);
+ LookupPath.toVector(CanonicalPath);
if (std::error_code EC = makeCanonical(CanonicalPath))
return EC;
@@ -2111,8 +2117,7 @@
lookupPath(CanonicalPath);
if (!Result) {
if (shouldFallBackToExternalFS(Result.getError()))
- return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath),
- OriginalPath);
+ return ExternalFS->openFileForReadImpl(CanonicalPath, OriginalPath);
return Result.getError();
}
@@ -2127,12 +2132,12 @@
auto *RE = cast<RedirectingFileSystem::RemapEntry>(Result->E);
- auto ExternalFile = File::getWithPath(
- ExternalFS->openFileForRead(CanonicalRemappedPath), ExtRedirect);
+ auto ExternalFile = ExternalFS->openFileForReadImpl(
+ CanonicalRemappedPath,
+ RE->useExternalName(UseExternalNames) ? ExtRedirect : OriginalPath);
if (!ExternalFile) {
if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E))
- return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath),
- OriginalPath);
+ return ExternalFS->openFileForReadImpl(CanonicalPath, OriginalPath);
return ExternalFile;
}
@@ -2140,9 +2145,8 @@
if (!ExternalStatus)
return ExternalStatus.getError();
- // FIXME: Update the status with the name and VFSMapped.
- Status S = getRedirectedFileStatus(
- OriginalPath, RE->useExternalName(UseExternalNames), *ExternalStatus);
+ Status S = *ExternalStatus;
+ S.IsVFSMapped = true;
return std::unique_ptr<File>(
std::make_unique<FileWithFixedStatus>(std::move(*ExternalFile), S));
}
Index: llvm/lib/Support/FileCollector.cpp
===================================================================
--- llvm/lib/Support/FileCollector.cpp
+++ llvm/lib/Support/FileCollector.cpp
@@ -266,10 +266,11 @@
}
llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
- openFileForRead(const Twine &Path) override {
- auto Result = FS->openFileForRead(Path);
+ openFileForReadImpl(const Twine &LookupPath,
+ const Twine &OriginalPath) override {
+ auto Result = FS->openFileForReadImpl(LookupPath, OriginalPath);
if (Result && *Result)
- Collector->addFile(Path);
+ Collector->addFile(LookupPath);
return Result;
}
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===================================================================
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -260,8 +260,17 @@
virtual llvm::ErrorOr<Status> status(const Twine &Path) = 0;
/// Get a \p File object for the file at \p Path, if one exists.
+ llvm::ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) {
+ return openFileForReadImpl(Path, Path);
+ }
+
+ /// Get a \p File object for the file at \p LookupPath. In most cases the
+ /// filesystem should ensure that the returned file has a path of
+ /// \p OriginalPath, but this is not a guarantee. In some cases the path will
+ /// have to differ, eg. if it has been remapped and is intentionally being
+ /// leaked.
virtual llvm::ErrorOr<std::unique_ptr<File>>
- openFileForRead(const Twine &Path) = 0;
+ openFileForReadImpl(const Twine &LookupPath, const Twine &OriginalPath) = 0;
/// This is a convenience method that opens a file, gets its content and then
/// closes the file.
@@ -343,8 +352,9 @@
void pushOverlay(IntrusiveRefCntPtr<FileSystem> FS);
llvm::ErrorOr<Status> status(const Twine &Path) override;
- llvm::ErrorOr<std::unique_ptr<File>>
- openFileForRead(const Twine &Path) override;
+ llvm::ErrorOr<std::unique_ptr<File>> ErrorOr<std::unique_ptr<File>>
+ openFileForReadImpl(const Twine &LookupPath,
+ const Twine &OriginalPath) override;
directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;
llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override;
std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
@@ -386,8 +396,9 @@
return FS->status(Path);
}
llvm::ErrorOr<std::unique_ptr<File>>
- openFileForRead(const Twine &Path) override {
- return FS->openFileForRead(Path);
+ openFileForReadImpl(const Twine &LookupPath,
+ const Twine &OriginalPath) override {
+ return FS->openFileForReadImpl(LookupPath, OriginalPath);
}
directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override {
return FS->dir_begin(Dir, EC);
@@ -487,7 +498,8 @@
llvm::ErrorOr<Status> status(const Twine &Path) override;
llvm::ErrorOr<std::unique_ptr<File>>
- openFileForRead(const Twine &Path) override;
+ openFileForReadImpl(const Twine &LookupPath,
+ const Twine &OriginalPath) override;
directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;
llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
@@ -855,7 +867,9 @@
bool UseExternalNames, FileSystem &ExternalFS);
ErrorOr<Status> status(const Twine &Path) override;
- ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) override;
+ llvm::ErrorOr<std::unique_ptr<File>>
+ openFileForReadImpl(const Twine &LookupPath,
+ const Twine &OriginalPath) override;
std::error_code getRealPath(const Twine &Path,
SmallVectorImpl<char> &Output) const override;
Index: clang/test/VFS/directory.c
===================================================================
--- clang/test/VFS/directory.c
+++ clang/test/VFS/directory.c
@@ -17,7 +17,9 @@
// RUN: sed -e "s@INPUT_DIR@Overlay@g" -e "s@OUT_DIR@%{/t:regex_replacement}/Underlying@g" %S/Inputs/vfsoverlay-directory-relative.yaml > %t/vfs-relative.yaml
// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs-relative.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
+// DIRECT: {{^}}# 1 "{{.*(/|\\\\)Underlying(/|\\\\)}}B.h"
// DIRECT: {{^}}// B.h in Underlying
+// DIRECT: {{^}}# 1 "{{.*(/|\\\\)Overlay(/|\\\\)}}C.h"
// DIRECT: {{^}}// C.h in Overlay
// 2) Underlying -> Middle -> Overlay (C.h found, B.h falling back to Underlying)
@@ -32,14 +34,18 @@
// RUN: rm -f %t/Overlay/C.h
// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=FALLBACK %s
+// FALLBACK: {{^}}# 1 "{{.*(/|\\\\)Underlying(/|\\\\)}}B.h"
// FALLBACK: {{^}}// B.h in Underlying
+// FALLBACK: {{^}}# 1 "{{.*(/|\\\\)Middle(/|\\\\)}}C.h"
// FALLBACK: {{^}}// C.h in Middle
// 3) Underlying -> Middle -> Overlay (C.h falling back to Underlying, B.h falling back to Underlying)
// RUN: rm -f %t/Middle/C.h
// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=FALLBACK2 %s
+// FALLBACK2: {{^}}# 1 "{{.*(/|\\\\)Underlying(/|\\\\)}}B.h"
// FALLBACK2: {{^}}// B.h in Underlying
+// FALLBACK2: {{^}}# 1 "{{.*(/|\\\\)Underlying(/|\\\\)}}C.h"
// FALLBACK2: {{^}}// C.h in Underlying
#include "B.h"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
