[VFS] RedirectingFileSystem only replace path if not already mapped

If the `ExternalFS` has already remapped a path then the
`RedirectingFileSystem` should not change it to the originally provided
path. This fixes the original path always being used if multiple VFS
overlays were provided and the path wasn't found in the highest (ie.
first in the chain).

This also renames `IsVFSMapped` to `ExposesExternalVFSPath` and only
sets it if `UseExternalName` is true. This flag then represents that the
`Status` has an external path that's different from its virtual path.
Right now the contained path is still the external path, but further PRs
will change this to *always* be the virtual path. Clients that need the
external can then request it specifically.

Note that even though `ExposesExternalVFSPath` isn't set for all
VFS-mapped paths, `IsVFSMapped` was only being used by a hack in
`FileManager` that was specific to module searching. In that case
`UseExternalNames` is always `true` and so that hack still applies.

Resolves rdar://90578880 and llvm-project#53306.

Differential Revision: https://reviews.llvm.org/D122549

1 files changed, 20 insertions, 12 deletions

diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index f4cf278..d30a5f7 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -270,12 +270,15 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
   // This occurs when one dir is symlinked to another, for example.
   FileEntry &UFE = UniqueRealFiles[Status.getUniqueID()];
 
+  // FIXME: This should just check `!Status.ExposesExternalVFSPath`, but the
+  // else branch also ends up fixing up relative paths to be the actually
+  // looked up absolute path. This isn't necessarily desired, but does seem to
+  // be relied on in some clients.
   if (Status.getName() == Filename) {
     // 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
@@ -285,9 +288,11 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
     //
     // FIXME: This is pretty complicated. It's also inconsistent with how
     // "real" filesystems behave and confuses parts of clang expect to see the
-    // name-as-accessed on the \a FileEntryRef. Maybe the returned \a
-    // FileEntryRef::getName() could return the accessed name unmodified, but
-    // make the external name available via a separate API.
+    // name-as-accessed on the \a FileEntryRef. To remove this we should
+    // implement the FIXME on `ExposesExternalVFSPath`, ie. update the
+    // `FileEntryRef::getName()` path to *always* be the virtual path and have
+    // clients request the external path only when required through a separate
+    // API.
     auto &Redirection =
         *SeenFileEntries
              .insert({Status.getName(), FileEntryRef::MapValue(UFE, DirInfo)})
@@ -308,13 +313,16 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
   FileEntryRef ReturnedRef(*NamedFileEnt);
   if (UFE.isValid()) { // Already have an entry with this inode, return it.
 
-    // FIXME: this hack ensures that if we look up a file by a virtual path in
-    // 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.
-    if (&DirInfo.getDirEntry() != UFE.Dir && Status.IsVFSMapped)
+    // FIXME: This hack ensures that `getDir()` will use the path that was
+    // used to lookup this file, even if we found a file by different path
+    // first. This is required in order to find a module's structure when its
+    // headers/module map are mapped in the VFS.
+    //
+    // This should be removed once `HeaderSearch` is updated to use `*Ref`s
+    // *and* the redirection hack above is removed. The removal of the latter
+    // is required since otherwise the ref will have the exposed external VFS
+    // path still.
+    if (&DirInfo.getDirEntry() != UFE.Dir && Status.ExposesExternalVFSPath)
       UFE.Dir = &DirInfo.getDirEntry();
 
     // Always update LastRef to the last name by which a file was accessed.