[VFS] RedirectingFileSystem only replace path if not already mapped

[bnbarham]

Summary

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.

[dexonsmith]

This looks nice!

clang/lib/Basic/FileManager.cpp

290–295

I'd suggest updating this text to match the new FIXME next to IsVFSMapped (instead of "maybe", list the plan).

317–319

It's not obvious to me that the redirection case above depends on this logic sticking around.

• If that's what you mean, can you explain in more detail why the redirection above depends on this UFE.Dir logic?
• If you're just talking about IsVFSMapped having to stick around, I think that part should be covered by the comment for the redirection.

llvm/include/llvm/Support/VirtualFileSystem.h

57–71

I think "mapped to another path" has a broader definition in my mind than you're using here. IMO, all things in a redirecting filesystem map from one path to another. The question is whether the mapping is leaked/available due to use-external-names.

I think this comment be more precise, and the note should be a FIXME that's part of the main comment, not something that comes later.

I also think it good to change the field name ~now (or in an immediately following NFC patch) to reflect the new meaning. I like "exposes" or "leaks" because that is clear that this indicates whether information about the mapping is available / exposed in the abstraction, whereas "mapped" and "mapping" sound to me like they're just talking about whether something was redirected. And I like the word "external" because it ties back to use-external-names.

Suggested text (feel free to change entirely, but I think it covers a couple of important points):

/// Whether this entity has an external path different from the virtual path, and
/// the external path is exposed by leaking it through the abstraction. For example,
/// a RedirectingFileSystem will set this for paths where UseExternalName is true.
///
/// FIXME: Currently the external path is exposed by replacing the virtual path
/// in this Status object. Instead, we should leave the path in the Status intact
/// (matching the requested virtual path), and just use this flag as hint that a
/// new API, FileSystem::getExternalVFSPath(), will not return `None`. Clients can
/// then request the external path only where needed (e.g., for diagnostics, or to
/// report discovered dependencies to external tools).
bool ExposesExternalVFSPath = false;

[bnbarham]

clang-apply-replacements/relative-paths.cpp is failing, I haven't looked into it but my guess would be that it's from the Status.getName() == Filename -> !Status.IsVFSMapped change. That seems very odd to me.

clang/lib/Basic/FileManager.cpp

290–295

Will do

317–319

I actually meant the reverse - the UFE.Dir logic being removed depends on the redirection logic above being removed. That's because for this to be removed, anywhere that cares about the requested path needs to be changed to use Refs. But even if they change to use Refs, the redirection logic above would itself replace the path with the external one. So it needs to be removed as well.

llvm/include/llvm/Support/VirtualFileSystem.h

57–71

That's fair. I suppose I was thinking that without use-external-names: true then nothing should be aware that it is "mapped" and thus "mapped" == "external". But I can see how that could be confusing.

I also thought that renaming now would be a little odd since eg. "HasExternalPath" seems a little different to "the actual path is currently the external path" (which is what it is right now). But with the "exposes" naming I think this could still make sense (especially with the FIXME you have). So I'll rename 👍

[dexonsmith]

In D122549#3412021, @bnbarham wrote:

clang-apply-replacements/relative-paths.cpp is failing, I haven't looked into it but my guess would be that it's from the Status.getName() == Filename -> !Status.IsVFSMapped change. That seems very odd to me.

Is it just failing on Windows? I wonder (rather speculatively...) whether https://reviews.llvm.org/D121733 would help.

clang/lib/Basic/FileManager.cpp

317–319

Okay, I suggest just making the link between them more clear.

[bnbarham]

In D122549#3412064, @dexonsmith wrote:

In D122549#3412021, @bnbarham wrote:

clang-apply-replacements/relative-paths.cpp is failing, I haven't looked into it but my guess would be that it's from the Status.getName() == Filename -> !Status.IsVFSMapped change. That seems very odd to me.

Is it just failing on Windows? I wonder (rather speculatively...) whether https://reviews.llvm.org/D121733 would help.

No, also debian. Not sure why it isn't saying debian failed in the review.

[dexonsmith]

clang/lib/Basic/FileManager.cpp

277

An incremental patch you could try would be:

if (Status.getName() == Filename || !Status.IsVFSMapped)

... dropping all the other changes.

This limits the redirection hack to only apply when a RedirectingFS is involved (leaving until later the fine-tuning of when IsVFSMapped gets set). If this smaller change still causes a test failure, it might be easier to reason about why / how to fix it / be sure that the fix is sound.

Eventually we might want something like:

if (!Status.IsVFSMapped) {
  assert(Status.getName() == Filename);

I imagine that's not going to succeed yet due to the CWD behaviour in the VFSes, but as a speculative patch it might help track down whatever the problem is.

[bnbarham]

Rename IsVFSMapped as suggested by Duncan. Update comments.

[bnbarham]

clang/lib/Basic/FileManager.cpp

277

That assertion currently won't succeed for any relative path, since getStatValue does the absolute pathing for relative paths. So Status.getName() is guaranteed to be different to Filename in that case.

Possibly even more odd is that the failing test passes locally on MacOSX. I'll try the suggestion above and see if they fail again.

[bnbarham]

clang/lib/Basic/FileManager.cpp

277

Actually, thinking about this a bit more - the failing test doesn't use an overlay at all, so changing to the above wouldn't help at all.

[dexonsmith]

clang/lib/Basic/FileManager.cpp

277

The test was added in 8188484daa4195a2c8b5253765036fa2c6da7263 / https://reviews.llvm.org/D112647, with a change to do:

+    auto PrevWorkingDir = SM.getFileManager().getFileSystemOpts().WorkingDir;
+    if (BuildDir)
+      SM.getFileManager().getFileSystemOpts().WorkingDir = std::move(*BuildDir);
     if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
       if (SourceTU) {
         auto &Replaces = DiagReplacements[*Entry];
@@ -170,18 +175,19 @@ groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs,
       errs() << "Described file '" << R.getFilePath()
              << "' doesn't exist. Ignoring...\n";
     }
+    SM.getFileManager().getFileSystemOpts().WorkingDir = PrevWorkingDir;

This looks incredibly suspicious. Changing the working directory mid-stream on the FileManager isn't sound. Consider:

/dir1/file1.h
/dir2/file1.h
/dir2/file2.h

if you do:

FM.WorkingDir = "/dir1";
FM.getFile("file1.h"); // returns /dir1/file1.h
FM.WorkingDir = "/dir2";
FM.getFile("file1.h"); // returns /dir1/file1.h !!!
FM.getFile("file2.h"); // returns /dir2/file2.h

That doesn't explain why it's not reproducing locally for you, though.

[bnbarham]

clang/lib/Basic/FileManager.cpp

277

I ended up building in a docker container and tracked it down.

The underlying problem is two fold:

1. What you've said (shouldn't change WorkingDir)
2. clang-apply-replacements stores the FileEntry and uses that to read the file later. Because relative paths are absolute pathed inside getStatValue, the path in the FileEntryRef remains relative (without the redirection hack).

The reason I'm not seeing this test fail on MacOS is down to the iteration order of files. On MacOS we process file2.yaml and then file1.yaml. On others it looks like we're processing file1.yaml and then file2.yaml.

If file1.yaml is processed last, its last lookup is an absolute path and thus everything works fine. But if file2.yaml is processed last, its last lookup is relative and then the file can't be found.

This would normally be fine since the relative path would be resolved using the working directory, but because that's continually changed... 💥

Another quirk here is that if getBufferForFile(FileEntry) was used and getFile originally passed openFile=true then this would work, because it would be cached. And wouldn't work if openFile=false was passed, which is the default case.

TLDR: I don't think we should modify the redirection-hack condition in this patch.

[bnbarham]

Keep using the redirection hack for the time being

[dexonsmith]

LGTM.

clang/lib/Basic/FileManager.cpp

FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
   }
 
   assert((openFile || !F) && "undesired open file");
 
   // It exists.  See if we have already opened a file with the same inode.
   // 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
     // vfs::RedirectionFileSystem. This allows clang to report the external
     // name to users (in diagnostics) and to tools that don't have access to
     // the VFS (in debug info and dependency '.d' files).
     //
     // 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)})
              .first;
     assert(Redirection.second->V.is<FileEntry *>() &&
            "filename redirected to a non-canonical filename?");
     assert(Redirection.second->V.get<FileEntry *>() == &UFE &&
            "filename from getStatValue() refers to wrong file");
 
     // Cache the redirection in the previously-inserted entry, still available
     // in the tentative return value.
     NamedFileEnt->second = FileEntryRef::MapValue(Redirection);
 
     // Fix the tentative return value.
     NamedFileEnt = &Redirection;
   }
 
   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.
     // FIXME: Neither this nor always using the first reference is correct; we
     // want to switch towards a design where we return a FileName object that
     // encapsulates both the name by which the file was accessed and the
     // corresponding FileEntry.
     // FIXME: LastRef should be removed from FileEntry once all clients adopt

clang/test/VFS/external-names-multi-overlay.c

+// 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'
+    }
+  ]
+}

llvm/include/llvm/Support/VirtualFileSystem.h

class Status {
   llvm::sys::fs::UniqueID UID;
   llvm::sys::TimePoint<> MTime;
   uint32_t User;
   uint32_t Group;
   uint64_t Size;
   llvm::sys::fs::file_type Type = llvm::sys::fs::file_type::status_error;
   llvm::sys::fs::perms Perms;
 
 public:
-  // FIXME: remove when files support multiple names
-  bool IsVFSMapped = false;
+  /// Whether this entity has an external path different from the virtual path,
+  /// and the external path is exposed by leaking it through the abstraction.
+  /// For example, a RedirectingFileSystem will set this for paths where
+  /// UseExternalName is true.
+  ///
+  /// FIXME: Currently the external path is exposed by replacing the virtual
+  /// path in this Status object. Instead, we should leave the path in the
+  /// Status intact (matching the requested virtual path), and just use this
+  /// flag as hint that a new API, FileSystem::getExternalVFSPath(), will not
+  /// return `None`. Clients can then request the external path only where
+  /// needed (e.g. for diagnostics, or to report discovered dependencies to
+  /// external tools).
+  bool ExposesExternalVFSPath = false;
 
   Status() = default;
   Status(const llvm::sys::fs::file_status &Status);
   Status(const Twine &Name, llvm::sys::fs::UniqueID UID,
          llvm::sys::TimePoint<> MTime, uint32_t User, uint32_t Group,
          uint64_t Size, llvm::sys::fs::file_type Type,
          llvm::sys::fs::perms Perms);
 
   /// Get a copy of a Status with a different size.

llvm/lib/Support/VirtualFileSystem.cpp

RedirectingFileSystem::lookupPathImpl(
   }
 
   return make_error_code(llvm::errc::no_such_file_or_directory);
 }
 
 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.ExposesExternalVFSPath)
+    return ExternalStatus;
+
   Status S = ExternalStatus;
   if (!UseExternalNames)
     S = Status::copyWithNewName(S, OriginalPath);
-  S.IsVFSMapped = true;
+  else
+    S.ExposesExternalVFSPath = true;
   return S;
 }
 
 ErrorOr<Status> RedirectingFileSystem::status(
     const Twine &CanonicalPath, const Twine &OriginalPath,
     const RedirectingFileSystem::LookupResult &Result) {
   if (Optional<StringRef> ExtRedirect = Result.getExternalRedirect()) {
     SmallString<256> CanonicalRemappedPath((*ExtRedirect).str());
public:
 
   void setPath(const Twine &Path) override { S = S.copyWithNewName(S, Path); }
 };
 
 } // namespace
 
 ErrorOr<std::unique_ptr<File>>
 File::getWithPath(ErrorOr<std::unique_ptr<File>> Result, const Twine &P) {
-  if (!Result)
+  // See \c getRedirectedFileStatus - don't update path if it's already been
+  // mapped.
+  if (!Result || (*Result)->status()->ExposesExternalVFSPath)
     return Result;
 
   ErrorOr<std::unique_ptr<File>> F = std::move(*Result);
   auto Name = F->get()->getName();
   if (Name && Name.get() != P.str())
     F->get()->setPath(P);
   return F;
 }

llvm/unittests/Support/VirtualFileSystemTest.cpp

TEST_F(VFSFromYAMLTest, MappedFiles) {
   IntrusiveRefCntPtr<vfs::OverlayFileSystem> O(
       new vfs::OverlayFileSystem(Lower));
   O->pushOverlay(FS);
 
   // file
   ErrorOr<vfs::Status> S = O->status("//root/file1");
   ASSERT_FALSE(S.getError());
   EXPECT_EQ("//root/foo/bar/a", S->getName());
-  EXPECT_TRUE(S->IsVFSMapped);
+  EXPECT_TRUE(S->ExposesExternalVFSPath);
 
   ErrorOr<vfs::Status> SLower = O->status("//root/foo/bar/a");
   EXPECT_EQ("//root/foo/bar/a", SLower->getName());
   EXPECT_TRUE(S->equivalent(*SLower));
-  EXPECT_FALSE(SLower->IsVFSMapped);
+  EXPECT_FALSE(SLower->ExposesExternalVFSPath);
 
   // file after opening
   auto OpenedF = O->openFileForRead("//root/file1");
   ASSERT_FALSE(OpenedF.getError());
   auto OpenedS = (*OpenedF)->status();
   ASSERT_FALSE(OpenedS.getError());
   EXPECT_EQ("//root/foo/bar/a", OpenedS->getName());
-  EXPECT_TRUE(OpenedS->IsVFSMapped);
+  EXPECT_TRUE(OpenedS->ExposesExternalVFSPath);
 
   // directory
   S = O->status("//root/");
   ASSERT_FALSE(S.getError());
   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->ExposesExternalVFSPath);
   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);
+  EXPECT_FALSE(SLower->ExposesExternalVFSPath);
 
   // 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->ExposesExternalVFSPath);
+  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->ExposesExternalVFSPath);
+  EXPECT_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);
+  EXPECT_TRUE(OpenedS->ExposesExternalVFSPath);
 
   // 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);
+  EXPECT_FALSE(OpenedS->ExposesExternalVFSPath);
 
   // broken mapping
   EXPECT_EQ(O->status("//root/file2").getError(),
             llvm::errc::no_such_file_or_directory);
   EXPECT_EQ(0, NumDiagnostics);
 }
 
 TEST_F(VFSFromYAMLTest, MappedRoot) {
TEST_F(VFSFromYAMLTest, MappedRoot) {
   IntrusiveRefCntPtr<vfs::OverlayFileSystem> O(
       new vfs::OverlayFileSystem(Lower));
   O->pushOverlay(FS);
 
   // file
   ErrorOr<vfs::Status> S = O->status("//mappedroot/a");
   ASSERT_FALSE(S.getError());
   EXPECT_EQ("//root/foo/bar/a", S->getName());
-  EXPECT_TRUE(S->IsVFSMapped);
+  EXPECT_TRUE(S->ExposesExternalVFSPath);
 
   ErrorOr<vfs::Status> SLower = O->status("//root/foo/bar/a");
   EXPECT_EQ("//root/foo/bar/a", SLower->getName());
   EXPECT_TRUE(S->equivalent(*SLower));
-  EXPECT_FALSE(SLower->IsVFSMapped);
+  EXPECT_FALSE(SLower->ExposesExternalVFSPath);
 
   // file after opening
   auto OpenedF = O->openFileForRead("//mappedroot/a");
   ASSERT_FALSE(OpenedF.getError());
   auto OpenedS = (*OpenedF)->status();
   ASSERT_FALSE(OpenedS.getError());
   EXPECT_EQ("//root/foo/bar/a", OpenedS->getName());
-  EXPECT_TRUE(OpenedS->IsVFSMapped);
+  EXPECT_TRUE(OpenedS->ExposesExternalVFSPath);
 
   EXPECT_EQ(0, NumDiagnostics);
 }
 
 TEST_F(VFSFromYAMLTest, RemappedDirectoryOverlay) {
   IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
   Lower->addDirectory("//root/foo");
   Lower->addRegularFile("//root/foo/a");
TEST_F(VFSFromYAMLTest, ReturnsRequestedPathVFSMiss) {
   ASSERT_FALSE(OpenedF.getError());
   llvm::ErrorOr<std::string> Name = (*OpenedF)->getName();
   ASSERT_FALSE(Name.getError());
   EXPECT_EQ("a", Name.get());
 
   auto OpenedS = (*OpenedF)->status();
   ASSERT_FALSE(OpenedS.getError());
   EXPECT_EQ("a", OpenedS->getName());
-  EXPECT_FALSE(OpenedS->IsVFSMapped);
+  EXPECT_FALSE(OpenedS->ExposesExternalVFSPath);
 
   auto DirectS = RemappedFS->status("a");
   ASSERT_FALSE(DirectS.getError());
   EXPECT_EQ("a", DirectS->getName());
-  EXPECT_FALSE(DirectS->IsVFSMapped);
+  EXPECT_FALSE(DirectS->ExposesExternalVFSPath);
 
   EXPECT_EQ(0, NumDiagnostics);
 }
 
 TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) {
   IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS(
       new vfs::InMemoryFileSystem);
   BaseFS->addFile("//root/foo/realname", 0,
TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) {
   ASSERT_FALSE(OpenedF.getError());
   llvm::ErrorOr<std::string> Name = (*OpenedF)->getName();
   ASSERT_FALSE(Name.getError());
   EXPECT_EQ("realname", Name.get());
 
   auto OpenedS = (*OpenedF)->status();
   ASSERT_FALSE(OpenedS.getError());
   EXPECT_EQ("realname", OpenedS->getName());
-  EXPECT_TRUE(OpenedS->IsVFSMapped);
+  EXPECT_TRUE(OpenedS->ExposesExternalVFSPath);
 
   auto DirectS = FS->status("vfsname");
   ASSERT_FALSE(DirectS.getError());
   EXPECT_EQ("realname", DirectS->getName());
-  EXPECT_TRUE(DirectS->IsVFSMapped);
+  EXPECT_TRUE(DirectS->ExposesExternalVFSPath);
 
   EXPECT_EQ(0, NumDiagnostics);
 }
 
 TEST_F(VFSFromYAMLTest, ReturnsInternalPathVFSHit) {
   IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS(
       new vfs::InMemoryFileSystem);
   BaseFS->addFile("//root/foo/realname", 0,
TEST_F(VFSFromYAMLTest, ReturnsInternalPathVFSHit) {
   ASSERT_FALSE(OpenedF.getError());
   llvm::ErrorOr<std::string> Name = (*OpenedF)->getName();
   ASSERT_FALSE(Name.getError());
   EXPECT_EQ("vfsname", Name.get());
 
   auto OpenedS = (*OpenedF)->status();
   ASSERT_FALSE(OpenedS.getError());
   EXPECT_EQ("vfsname", OpenedS->getName());
-  EXPECT_TRUE(OpenedS->IsVFSMapped);
+  EXPECT_FALSE(OpenedS->ExposesExternalVFSPath);
 
   auto DirectS = FS->status("vfsname");
   ASSERT_FALSE(DirectS.getError());
   EXPECT_EQ("vfsname", DirectS->getName());
-  EXPECT_TRUE(DirectS->IsVFSMapped);
+  EXPECT_FALSE(DirectS->ExposesExternalVFSPath);
 
   EXPECT_EQ(0, NumDiagnostics);
 }
 
 TEST_F(VFSFromYAMLTest, CaseInsensitive) {
   IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
   Lower->addRegularFile("//root/foo/bar/a");
   IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(