diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -270,12 +270,11 @@ // This occurs when one dir is symlinked to another, for example. FileEntry &UFE = UniqueRealFiles[Status.getUniqueID()]; - if (Status.getName() == Filename) { + if (!Status.IsVFSMapped) { // The name matches. Set the FileEntry. NamedFileEnt->second = FileEntryRef::MapValue(UFE, DirInfo); } else { - // Name mismatch. We need a redirect. First grab the actual entry we want - // to return. + // We need a redirect. First grab the actual entry we want to return. // // This redirection logic intentionally leaks the external name of a // redirected file that uses 'use-external-name' in \a @@ -312,8 +311,11 @@ // the VFS that the getDir() will have the virtual path, even if we found // the file by a 'real' path first. This is required in order to find a // module's structure when its headers/module map are mapped in the VFS. - // We should remove this as soon as we can properly support a file having - // multiple names. + // + // This can be removed once \c HeaderSearch uses Refs *and* the redirection + // case above is removed. That one can be removed once we always return + // virtual paths and anywhere that needs external paths specifically + // requests them. if (&DirInfo.getDirEntry() != UFE.Dir && Status.IsVFSMapped) UFE.Dir = &DirInfo.getDirEntry(); diff --git a/clang/test/VFS/external-names-multi-overlay.c b/clang/test/VFS/external-names-multi-overlay.c new file mode 100644 --- /dev/null +++ b/clang/test/VFS/external-names-multi-overlay.c @@ -0,0 +1,39 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: sed -e "s@NAME_DIR@%{/t:regex_replacement}/A@g" -e "s@EXTERNAL_DIR@%{/t:regex_replacement}/B@g" %t/vfs/base.yaml > %t/vfs/a-b.yaml +// RUN: sed -e "s@NAME_DIR@%{/t:regex_replacement}/C@g" -e "s@EXTERNAL_DIR@%{/t:regex_replacement}/D@g" %t/vfs/base.yaml > %t/vfs/c-d.yaml + +// Check that the external name is given when multiple overlays are provided + +// RUN: %clang_cc1 -Werror -I %t/A -ivfsoverlay %t/vfs/c-d.yaml -ivfsoverlay %t/vfs/a-b.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=FROM_B %s +// FROM_B: # 1 "{{.*(/|\\\\)B(/|\\\\)}}Header.h" +// FROM_B: // Header.h in B + +// RUN: %clang_cc1 -Werror -I %t/B -ivfsoverlay %t/vfs/c-d.yaml -ivfsoverlay %t/vfs/a-b.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=FROM_B %s + +// RUN: %clang_cc1 -Werror -I %t/C -ivfsoverlay %t/vfs/c-d.yaml -ivfsoverlay %t/vfs/a-b.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=FROM_D %s +// FROM_D: # 1 "{{.*(/|\\\\)D(/|\\\\)}}Header.h" +// FROM_D: // Header.h in D + +// RUN: %clang_cc1 -Werror -I %t/C -ivfsoverlay %t/vfs/c-d.yaml -ivfsoverlay %t/vfs/a-b.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=FROM_D %s + +//--- main.c +#include "Header.h" + +//--- B/Header.h +// Header.h in B + +//--- D/Header.h +// Header.h in D + +//--- vfs/base.yaml +{ + 'version': 0, + 'redirecting-with': 'fallthrough', + 'roots': [ + { 'name': 'NAME_DIR', + 'type': 'directory-remap', + 'external-contents': 'EXTERNAL_DIR' + } + ] +} diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -55,8 +55,12 @@ llvm::sys::fs::perms Perms; public: - // FIXME: remove when files support multiple names + /// Whether the path in this status has been mapped by a VFS to another path. bool IsVFSMapped = false; + // Note: Currently used by a hack in \c FileManager and internally in + // \c RedirectingFileSystem. We should change this to "HasVFSMapping" and + // *always* return the virtual path, only providing the mapped/external path + // when requested. Status() = default; Status(const llvm::sys::fs::file_status &Status); diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -2163,10 +2163,16 @@ static Status getRedirectedFileStatus(const Twine &OriginalPath, bool UseExternalNames, Status ExternalStatus) { + // The path has been mapped by some nested VFS, don't override it with the + // original path. + if (ExternalStatus.IsVFSMapped) + return ExternalStatus; + Status S = ExternalStatus; if (!UseExternalNames) S = Status::copyWithNewName(S, OriginalPath); - S.IsVFSMapped = true; + else + S.IsVFSMapped = true; return S; } @@ -2268,7 +2274,9 @@ ErrorOr> File::getWithPath(ErrorOr> Result, const Twine &P) { - if (!Result) + // See \c getRedirectedFileStatus - don't update path if it's already been + // mapped. + if (!Result || (*Result)->status()->IsVFSMapped) return Result; ErrorOr> F = std::move(*Result); diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -1478,16 +1478,16 @@ // 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()); + EXPECT_FALSE(S->isDirectory()); + EXPECT_TRUE(S->IsVFSMapped); + EXPECT_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()); + EXPECT_FALSE(S->isDirectory()); + EXPECT_FALSE(S->IsVFSMapped); + EXPECT_EQ("//root/mappeddir2/a", S->getName()); // file contents in remapped directory OpenedF = O->openFileForRead("//root/mappeddir/a"); @@ -1503,7 +1503,7 @@ OpenedS = (*OpenedF)->status(); ASSERT_FALSE(OpenedS.getError()); EXPECT_EQ("//root/mappeddir2/a", OpenedS->getName()); - EXPECT_TRUE(OpenedS->IsVFSMapped); + EXPECT_FALSE(OpenedS->IsVFSMapped); // broken mapping EXPECT_EQ(O->status("//root/file2").getError(), @@ -1776,12 +1776,12 @@ auto OpenedS = (*OpenedF)->status(); ASSERT_FALSE(OpenedS.getError()); EXPECT_EQ("vfsname", OpenedS->getName()); - EXPECT_TRUE(OpenedS->IsVFSMapped); + EXPECT_FALSE(OpenedS->IsVFSMapped); auto DirectS = FS->status("vfsname"); ASSERT_FALSE(DirectS.getError()); EXPECT_EQ("vfsname", DirectS->getName()); - EXPECT_TRUE(DirectS->IsVFSMapped); + EXPECT_FALSE(DirectS->IsVFSMapped); EXPECT_EQ(0, NumDiagnostics); }