FileManager: Add FileEntryRef::getDir, returning DirectoryEntryRef

[dexonsmith]

Summary

Add FileEntryRef::getDir, which returns a DirectoryEntryRef. This
includes a few changes:

• Customize OptionalStorage so that Optional<DirectoryEntryRef> is pointer-sized (like the change made to Optional<FileEntryRef>). Factored out a common class, FileMgr::MapEntryOptionalStorage, to reduce the code duplication.
• Store an Optional<DirectoryEntryRef> in FileEntryRef::MapValue. This is set if and only if MapValue has a real FileEntry.
• Change FileManager::getFileRef and getVirtualFileRef to use getDirectoryRef and store it in the StringMap for FileEntryRef.

[dexonsmith]

Rebased on top of ac49500cd0484e1b2dcf37fa4c0dade6f113c2c9.

[dexonsmith]

Rebase on top of 814141f9bd0a64bbedae05773972d140f04f654d.

[dexonsmith]

ping

[dexonsmith]

ping

[jansvoboda11]

Left tiny question inline, but LGTM anyways.

clang/unittests/Basic/FileEntryTest.cpp

95

Dead variable, do we need it?

[dexonsmith]

clang/unittests/Basic/FileEntryTest.cpp

95

Thanks, nice catch, that looks vestigial. I'll delete that line in a follow-up.

[dexonsmith]

Thanks again for the review, landed in 1b042de5b29af4869a77ecbc632029fba0313dec.

clang/unittests/Basic/FileEntryTest.cpp

95

Deleted this stray line in 94f537c6b2bb55c8e058a9989d02ab0d68a0c61a; turns out it was originally a copy/paste bug from ac49500cd0484e1b2dcf37fa4c0dade6f113c2c9.

clang/include/clang/Basic/DirectoryEntry.h

 #define LLVM_CLANG_BASIC_DIRECTORYENTRY_H
 
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorOr.h"
 
 namespace clang {
+namespace FileMgr {
+
+template <class RefTy> class MapEntryOptionalStorage;
+
+} // end namespace FileMgr
 
 /// Cached information about one directory (either on disk or in
 /// the virtual file system).
 class DirectoryEntry {
   friend class FileManager;
 
   // FIXME: We should not be storing a directory entry name here.
   StringRef Name; // Name of the directory.
 
 public:
   StringRef getName() const { return Name; }
 };
 
 /// A reference to a \c DirectoryEntry  that includes the name of the directory
 /// as it was accessed by the FileManager's client.
 class DirectoryEntryRef {
 public:
-  const DirectoryEntry &getDirEntry() const { return *Entry->getValue(); }
+  const DirectoryEntry &getDirEntry() const { return *ME->getValue(); }
+
+  StringRef getName() const { return ME->getKey(); }
+
+  using MapEntry = llvm::StringMapEntry<llvm::ErrorOr<DirectoryEntry &>>;
+
+  const MapEntry &getMapEntry() const { return *ME; }
 
-  StringRef getName() const { return Entry->getKey(); }
+  DirectoryEntryRef() = delete;
+  DirectoryEntryRef(MapEntry &ME) : ME(&ME) {}
 
 private:
-  friend class FileManager;
+  friend class FileMgr::MapEntryOptionalStorage<DirectoryEntryRef>;
+  struct optional_none_tag {};
+
+  // Private constructor for use by OptionalStorage.
+  DirectoryEntryRef(optional_none_tag) : ME(nullptr) {}
+  bool hasOptionalValue() const { return ME; }
+
+  const MapEntry *ME;
+};
+
+namespace FileMgr {
 
-  DirectoryEntryRef(
-      llvm::StringMapEntry<llvm::ErrorOr<DirectoryEntry &>> *Entry)
-      : Entry(Entry) {}
+/// Customized storage for refs derived from map entires in FileManager, using
+/// the private optional_none_tag to keep it to the size of a single pointer.
+template <class RefTy> class MapEntryOptionalStorage {
+  using optional_none_tag = typename RefTy::optional_none_tag;
+  RefTy MaybeRef;
 
-  const llvm::StringMapEntry<llvm::ErrorOr<DirectoryEntry &>> *Entry;
+public:
+  MapEntryOptionalStorage() : MaybeRef(optional_none_tag()) {}
+
+  template <class... ArgTypes>
+  explicit MapEntryOptionalStorage(llvm::optional_detail::in_place_t,
+                                   ArgTypes &&...Args)
+      : MaybeRef(std::forward<ArgTypes>(Args)...) {}
+
+  void reset() { MaybeRef = optional_none_tag(); }
+
+  bool hasValue() const { return MaybeRef.hasOptionalValue(); }
+
+  RefTy &getValue() LLVM_LVALUE_FUNCTION {
+    assert(hasValue());
+    return MaybeRef;
+  }
+  RefTy const &getValue() const LLVM_LVALUE_FUNCTION {
+    assert(hasValue());
+    return MaybeRef;
+  }
+#if LLVM_HAS_RVALUE_REFERENCE_THIS
+  RefTy &&getValue() && {
+    assert(hasValue());
+    return std::move(MaybeRef);
+  }
+#endif
+
+  template <class... Args> void emplace(Args &&...args) {
+    MaybeRef = RefTy(std::forward<Args>(args)...);
+  }
+
+  MapEntryOptionalStorage &operator=(RefTy Ref) {
+    MaybeRef = Ref;
+    return *this;
+  }
 };
 
+} // end namespace FileMgr
 } // end namespace clang
 
+namespace llvm {
+namespace optional_detail {
+
+/// Customize OptionalStorage<DirectoryEntryRef> to use DirectoryEntryRef and
+/// its optional_none_tag to keep it the size of a single pointer.
+template <>
+class OptionalStorage<clang::DirectoryEntryRef>
+    : public clang::FileMgr::MapEntryOptionalStorage<clang::DirectoryEntryRef> {
+  using StorageImpl =
+      clang::FileMgr::MapEntryOptionalStorage<clang::DirectoryEntryRef>;
+
+public:
+  OptionalStorage() = default;
+
+  template <class... ArgTypes>
+  explicit OptionalStorage(in_place_t, ArgTypes &&...Args)
+      : StorageImpl(in_place_t{}, std::forward<ArgTypes>(Args)...) {}
+
+  OptionalStorage &operator=(clang::DirectoryEntryRef Ref) {
+    StorageImpl::operator=(Ref);
+    return *this;
+  }
+};
+
+static_assert(sizeof(Optional<clang::DirectoryEntryRef>) ==
+                  sizeof(clang::DirectoryEntryRef),
+              "Optional<DirectoryEntryRef> must avoid size overhead");
+
+static_assert(
+    std::is_trivially_copyable<Optional<clang::DirectoryEntryRef>>::value,
+    "Optional<DirectoryEntryRef> should be trivially copyable");
+
+} // end namespace optional_detail
+} // end namespace llvm
+
 #endif // LLVM_CLANG_BASIC_DIRECTORYENTRY_H

clang/include/clang/Basic/FileEntry.h

 //===- clang/Basic/FileEntry.h - File references ----------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// Defines interfaces for clang::FileEntry and clang::FileEntryRef.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_FILEENTRY_H
 #define LLVM_CLANG_BASIC_FILEENTRY_H
 
+#include "clang/Basic/DirectoryEntry.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem/UniqueID.h"
 
 namespace llvm {

 template <>
 class OptionalStorage<clang::FileEntryRef, /*is_trivially_copyable*/ true>;
 
 } // namespace optional_detail
 } // namespace llvm
 
 namespace clang {
 
-class DirectoryEntry;
 class FileEntry;
 
 /// A reference to a \c FileEntry that includes the name of the file as it was
 /// accessed by the FileManager's client.
 class FileEntryRef {
 public:
   StringRef getName() const { return ME->first(); }
   const FileEntry &getFileEntry() const {
     return *ME->second->V.get<FileEntry *>();
   }
+  DirectoryEntryRef getDir() const { return *ME->second->Dir; }
 
   inline bool isValid() const;
   inline off_t getSize() const;
   inline unsigned getUID() const;
   inline const llvm::sys::fs::UniqueID &getUniqueID() const;
   inline time_t getModificationTime() const;
   inline void closeFile() const;
 
struct MapValue {
     /// VFSs that use external names. In that case, the \c FileEntryRef
     /// returned by the \c FileManager will have the external name, and not the
     /// name that was used to lookup the file.
     ///
     /// The second type is really a `const MapEntry *`, but that confuses
     /// gcc5.3.  Once that's no longer supported, change this back.
     llvm::PointerUnion<FileEntry *, const void *> V;
 
+    /// Directory the file was found in. Set if and only if V is a FileEntry.
+    Optional<DirectoryEntryRef> Dir;
+
     MapValue() = delete;
-    MapValue(FileEntry &FE) : V(&FE) {}
+    MapValue(FileEntry &FE, DirectoryEntryRef Dir) : V(&FE), Dir(Dir) {}
     MapValue(MapEntry &ME) : V(&ME) {}
   };
 
   /// Check if RHS referenced the file in exactly the same way.
   bool isSameRef(const FileEntryRef &RHS) const { return ME == RHS.ME; }
 
   /// Allow FileEntryRef to degrade into 'const FileEntry*' to facilitate
   /// incremental adoption.
explicit FileEntryRef(const MapEntry &ME) : ME(&ME) {
     assert(ME.second->V.is<FileEntry *>() && "Expected FileEntry");
   }
 
   /// Expose the underlying MapEntry to simplify packing in a PointerIntPair or
   /// PointerUnion and allow construction in Optional.
   const clang::FileEntryRef::MapEntry &getMapEntry() const { return *ME; }
 
 private:
-  friend class llvm::optional_detail::OptionalStorage<
-      FileEntryRef, /*is_trivially_copyable*/ true>;
+  friend class FileMgr::MapEntryOptionalStorage<FileEntryRef>;
   struct optional_none_tag {};
 
   // Private constructor for use by OptionalStorage.
   FileEntryRef(optional_none_tag) : ME(nullptr) {}
   bool hasOptionalValue() const { return ME; }
 
   const MapEntry *ME;
 };
 
 static_assert(sizeof(FileEntryRef) == sizeof(const FileEntry *),
               "FileEntryRef must avoid size overhead");
 
 static_assert(std::is_trivially_copyable<FileEntryRef>::value,
               "FileEntryRef must be trivially copyable");
 
 } // end namespace clang
 
 namespace llvm {
 namespace optional_detail {
 
 /// Customize OptionalStorage<FileEntryRef> to use FileEntryRef and its
 /// optional_none_tag to keep it the size of a single pointer.
-template <> class OptionalStorage<clang::FileEntryRef> {
-  clang::FileEntryRef MaybeRef;
+template <>
+class OptionalStorage<clang::FileEntryRef>
+    : public clang::FileMgr::MapEntryOptionalStorage<clang::FileEntryRef> {
+  using StorageImpl =
+      clang::FileMgr::MapEntryOptionalStorage<clang::FileEntryRef>;
 
 public:
-  ~OptionalStorage() = default;
-  OptionalStorage() : MaybeRef(clang::FileEntryRef::optional_none_tag()) {}
-  OptionalStorage(OptionalStorage const &Other) = default;
-  OptionalStorage(OptionalStorage &&Other) = default;
-  OptionalStorage &operator=(OptionalStorage const &Other) = default;
-  OptionalStorage &operator=(OptionalStorage &&Other) = default;
+  OptionalStorage() = default;
 
   template <class... ArgTypes>
   explicit OptionalStorage(in_place_t, ArgTypes &&...Args)
-      : MaybeRef(std::forward<ArgTypes>(Args)...) {}
-
-  void reset() { MaybeRef = clang::FileEntryRef::optional_none_tag(); }
-
-  bool hasValue() const { return MaybeRef.hasOptionalValue(); }
-
-  clang::FileEntryRef &getValue() LLVM_LVALUE_FUNCTION {
-    assert(hasValue());
-    return MaybeRef;
-  }
-  clang::FileEntryRef const &getValue() const LLVM_LVALUE_FUNCTION {
-    assert(hasValue());
-    return MaybeRef;
-  }
-#if LLVM_HAS_RVALUE_REFERENCE_THIS
-  clang::FileEntryRef &&getValue() && {
-    assert(hasValue());
-    return std::move(MaybeRef);
-  }
-#endif
-
-  template <class... Args> void emplace(Args &&...args) {
-    MaybeRef = clang::FileEntryRef(std::forward<Args>(args)...);
-  }
+      : StorageImpl(in_place_t{}, std::forward<ArgTypes>(Args)...) {}
 
   OptionalStorage &operator=(clang::FileEntryRef Ref) {
-    MaybeRef = Ref;
+    StorageImpl::operator=(Ref);
     return *this;
   }
 };
 
 static_assert(sizeof(Optional<clang::FileEntryRef>) ==
                   sizeof(clang::FileEntryRef),
               "Optional<FileEntryRef> must avoid size overhead");
 

clang/lib/Basic/FileManager.cpp

void FileManager::setStatCache(std::unique_ptr<FileSystemStatCache> statCache) {
   assert(statCache && "No stat cache provided?");
   StatCache = std::move(statCache);
 }
 
 void FileManager::clearStatCache() { StatCache.reset(); }
 
 /// Retrieve the directory that the given file name resides in.
 /// Filename can point to either a real file or a virtual file.
-static llvm::ErrorOr<const DirectoryEntry *>
+static llvm::Expected<DirectoryEntryRef>
 getDirectoryFromFile(FileManager &FileMgr, StringRef Filename,
                      bool CacheFailure) {
   if (Filename.empty())
-    return std::errc::no_such_file_or_directory;
+    return llvm::errorCodeToError(
+        make_error_code(std::errc::no_such_file_or_directory));
 
   if (llvm::sys::path::is_separator(Filename[Filename.size() - 1]))
-    return std::errc::is_a_directory;
+    return llvm::errorCodeToError(make_error_code(std::errc::is_a_directory));
 
   StringRef DirName = llvm::sys::path::parent_path(Filename);
   // Use the current directory if file has no path component.
   if (DirName.empty())
     DirName = ".";
 
-  return FileMgr.getDirectory(DirName, CacheFailure);
+  return FileMgr.getDirectoryRef(DirName, CacheFailure);
 }
 
 /// Add all ancestors of the given path (pointing to either a file or
 /// a directory) as virtual directories.
 void FileManager::addAncestorsAsVirtualDirs(StringRef Path) {
   StringRef DirName = llvm::sys::path::parent_path(Path);
   if (DirName.empty())
     DirName = ".";
#endif
   ++NumDirLookups;
 
   // See if there was already an entry in the map.  Note that the map
   // contains both virtual and real directories.
   auto SeenDirInsertResult =
       SeenDirEntries.insert({DirName, std::errc::no_such_file_or_directory});
   if (!SeenDirInsertResult.second) {
     if (SeenDirInsertResult.first->second)
-      return DirectoryEntryRef(&*SeenDirInsertResult.first);
+      return DirectoryEntryRef(*SeenDirInsertResult.first);
     return llvm::errorCodeToError(SeenDirInsertResult.first->second.getError());
   }
 
   // We've not seen this before. Fill it in.
   ++NumDirCacheMisses;
   auto &NamedDirEnt = *SeenDirInsertResult.first;
   assert(!NamedDirEnt.second && "should be newly-created");
 
#endif
 
   NamedDirEnt.second = UDE;
   if (UDE.getName().empty()) {
     // We don't have this directory yet, add it.  We use the string
     // key from the SeenDirEntries map as the string.
     UDE.Name  = InterndDirName;
   }
 
-  return DirectoryEntryRef(&NamedDirEnt);
+  return DirectoryEntryRef(NamedDirEnt);
 }
 
 llvm::ErrorOr<const DirectoryEntry *>
 FileManager::getDirectory(StringRef DirName, bool CacheFailure) {
   auto Result = getDirectoryRef(DirName, CacheFailure);
   if (Result)
     return &Result->getDirEntry();
   return llvm::errorToErrorCode(Result.takeError());
FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
 
   // Look up the directory for the file.  When looking up something like
   // sys/foo.h we'll discover all of the search directories that have a 'sys'
   // subdirectory.  This will let us avoid having to waste time on known-to-fail
   // searches when we go to find sys/bar.h, because all the search directories
   // without a 'sys' subdir will get a cached failure result.
   auto DirInfoOrErr = getDirectoryFromFile(*this, Filename, CacheFailure);
   if (!DirInfoOrErr) { // Directory doesn't exist, file can't exist.
+    std::error_code Err = errorToErrorCode(DirInfoOrErr.takeError());
     if (CacheFailure)
-      NamedFileEnt->second = DirInfoOrErr.getError();
+      NamedFileEnt->second = Err;
     else
       SeenFileEntries.erase(Filename);
 
-    return llvm::errorCodeToError(DirInfoOrErr.getError());
+    return llvm::errorCodeToError(Err);
   }
-  const DirectoryEntry *DirInfo = *DirInfoOrErr;
+  DirectoryEntryRef DirInfo = *DirInfoOrErr;
 
   // FIXME: Use the directory info to prune this, before doing the stat syscall.
   // FIXME: This will reduce the # syscalls.
 
   // Check to see if the file exists.
   std::unique_ptr<llvm::vfs::File> F;
   llvm::vfs::Status Status;
   auto statError = getStatValue(InterndFileName, Status, true,
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()];
 
   if (Status.getName() == Filename) {
     // The name matches. Set the FileEntry.
-    NamedFileEnt->second = FileEntryRef::MapValue(UFE);
+    NamedFileEnt->second = FileEntryRef::MapValue(UFE, DirInfo);
   } else {
     // Name mismatch. We need a redirect. First grab the actual entry we want
     // to return.
     auto &Redirection =
-        *SeenFileEntries.insert({Status.getName(), FileEntryRef::MapValue(UFE)})
+        *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 != UFE.Dir && Status.IsVFSMapped)
-      UFE.Dir = DirInfo;
+    if (&DirInfo.getDirEntry() != UFE.Dir && Status.IsVFSMapped)
+      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
     // FileEntryRef.
     UFE.LastRef = ReturnedRef;
 
     return ReturnedRef;
   }
 
   // Otherwise, we don't have this file yet, add it.
   UFE.LastRef = ReturnedRef;
   UFE.Size    = Status.getSize();
   UFE.ModTime = llvm::sys::toTimeT(Status.getLastModificationTime());
-  UFE.Dir     = DirInfo;
+  UFE.Dir     = &DirInfo.getDirEntry();
   UFE.UID     = NextFileUID++;
   UFE.UniqueID = Status.getUniqueID();
   UFE.IsNamedPipe = Status.getType() == llvm::sys::fs::file_type::fifo_file;
   UFE.File = std::move(F);
   UFE.IsValid = true;
 
   if (UFE.File) {
     if (auto PathName = UFE.File->getName())
FileEntryRef FileManager::getVirtualFileRef(StringRef Filename, off_t Size,
   // We've not seen this before, or the file is cached as non-existent.
   ++NumFileCacheMisses;
   addAncestorsAsVirtualDirs(Filename);
   FileEntry *UFE = nullptr;
 
   // Now that all ancestors of Filename are in the cache, the
   // following call is guaranteed to find the DirectoryEntry from the
   // cache.
-  auto DirInfo = getDirectoryFromFile(*this, Filename, /*CacheFailure=*/true);
+  auto DirInfo = expectedToOptional(
+      getDirectoryFromFile(*this, Filename, /*CacheFailure=*/true));
   assert(DirInfo &&
          "The directory of a virtual file should already be in the cache.");
 
   // Check to see if the file exists. If so, drop the virtual file
   llvm::vfs::Status Status;
   const char *InterndFileName = NamedFileEnt.first().data();
   if (!getStatValue(InterndFileName, Status, true, nullptr)) {
     UFE = &UniqueRealFiles[Status.getUniqueID()];
     Status = llvm::vfs::Status(
       Status.getName(), Status.getUniqueID(),
       llvm::sys::toTimePoint(ModificationTime),
       Status.getUser(), Status.getGroup(), Size,
       Status.getType(), Status.getPermissions());
 
-    NamedFileEnt.second = FileEntryRef::MapValue(*UFE);
+    NamedFileEnt.second = FileEntryRef::MapValue(*UFE, *DirInfo);
 
     // If we had already opened this file, close it now so we don't
     // leak the descriptor. We're not going to use the file
     // descriptor anyway, since this is a virtual file.
     if (UFE->File)
       UFE->closeFile();
 
     // If we already have an entry with this inode, return it.
     //
     // FIXME: Surely this should add a reference by the new name, and return
     // it instead...
     if (UFE->isValid())
       return FileEntryRef(NamedFileEnt);
 
     UFE->UniqueID = Status.getUniqueID();
     UFE->IsNamedPipe = Status.getType() == llvm::sys::fs::file_type::fifo_file;
     fillRealPathName(UFE, Status.getName());
   } else {
     VirtualFileEntries.push_back(std::make_unique<FileEntry>());
     UFE = VirtualFileEntries.back().get();
-    NamedFileEnt.second = FileEntryRef::MapValue(*UFE);
+    NamedFileEnt.second = FileEntryRef::MapValue(*UFE, *DirInfo);
   }
 
   UFE->LastRef = FileEntryRef(NamedFileEnt);
   UFE->Size    = Size;
   UFE->ModTime = ModificationTime;
-  UFE->Dir     = *DirInfo;
+  UFE->Dir     = &DirInfo->getDirEntry();
   UFE->UID     = NextFileUID++;
   UFE->IsValid = true;
   UFE->File.reset();
   return FileEntryRef(NamedFileEnt);
 }
 
 llvm::Optional<FileEntryRef> FileManager::getBypassFile(FileEntryRef VF) {
   // Stat of the file and return nullptr if it doesn't exist.
auto Insertion = SeenBypassFileEntries->insert(
       {VF.getName(), std::errc::no_such_file_or_directory});
   if (!Insertion.second)
     return FileEntryRef(*Insertion.first);
 
   // Fill in the new entry from the stat.
   BypassFileEntries.push_back(std::make_unique<FileEntry>());
   const FileEntry &VFE = VF.getFileEntry();
   FileEntry &BFE = *BypassFileEntries.back();
-  Insertion.first->second = FileEntryRef::MapValue(BFE);
+  Insertion.first->second = FileEntryRef::MapValue(BFE, VF.getDir());
   BFE.LastRef = FileEntryRef(*Insertion.first);
   BFE.Size = Status.getSize();
   BFE.Dir = VFE.Dir;
   BFE.ModTime = llvm::sys::toTimeT(Status.getLastModificationTime());
   BFE.UID = NextFileUID++;
   BFE.IsValid = true;
 
   // Save the entry in the bypass table and return.

clang/unittests/Basic/FileEntryTest.cpp

 #include "llvm/ADT/StringMap.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 using namespace clang;
 
 namespace {
 
-using MapEntry = FileEntryRef::MapEntry;
-using MapValue = FileEntryRef::MapValue;
-using MapType = StringMap<llvm::ErrorOr<MapValue>>;
+using FileMap = StringMap<llvm::ErrorOr<FileEntryRef::MapValue>>;
+using DirMap = StringMap<llvm::ErrorOr<DirectoryEntry &>>;
 
-FileEntryRef addRef(MapType &M, StringRef Name, FileEntry &E) {
-  return FileEntryRef(*M.insert({Name, MapValue(E)}).first);
+struct RefMaps {
+  FileMap Files;
+  DirMap Dirs;
+
+  DirectoryEntry D;
+  DirectoryEntryRef DR;
+  SmallVector<std::unique_ptr<FileEntry>, 5> FEs;
+
+  RefMaps() : DR(*Dirs.insert({"dir", D}).first) {}
+
+  FileEntryRef addFile(StringRef Name) {
+    FEs.push_back(std::make_unique<FileEntry>());
+    return FileEntryRef(
+        *Files.insert({Name, FileEntryRef::MapValue(*FEs.back().get(), DR)})
+             .first);
   }
+  FileEntryRef addFileAlias(StringRef Name, FileEntryRef Base) {
+    return FileEntryRef(
+        *Files
+             .insert(
+                 {Name, FileEntryRef::MapValue(
+                            const_cast<FileEntry &>(Base.getFileEntry()), DR)})
+             .first);
+  }
+};
 
 TEST(FileEntryTest, FileEntryRef) {
-  MapType Refs;
-  FileEntry E1, E2;
-  FileEntryRef R1 = addRef(Refs, "1", E1);
-  FileEntryRef R2 = addRef(Refs, "2", E2);
-  FileEntryRef R1Also = addRef(Refs, "1-also", E1);
+  RefMaps Refs;
+  FileEntryRef R1 = Refs.addFile("1");
+  FileEntryRef R2 = Refs.addFile("2");
+  FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
 
   EXPECT_EQ("1", R1.getName());
   EXPECT_EQ("2", R2.getName());
   EXPECT_EQ("1-also", R1Also.getName());
 
-  EXPECT_EQ(&E1, &R1.getFileEntry());
-  EXPECT_EQ(&E2, &R2.getFileEntry());
-  EXPECT_EQ(&E1, &R1Also.getFileEntry());
+  EXPECT_NE(&R1.getFileEntry(), &R2.getFileEntry());
+  EXPECT_EQ(&R1.getFileEntry(), &R1Also.getFileEntry());
 
   const FileEntry *CE1 = R1;
-  EXPECT_EQ(CE1, &E1);
+  EXPECT_EQ(CE1, &R1.getFileEntry());
 }
 
 TEST(FileEntryTest, OptionalFileEntryRefDegradesToFileEntryPtr) {
-  MapType Refs;
-  FileEntry E1, E2;
+  RefMaps Refs;
   OptionalFileEntryRefDegradesToFileEntryPtr M0;
-  OptionalFileEntryRefDegradesToFileEntryPtr M1 = addRef(Refs, "1", E1);
-  OptionalFileEntryRefDegradesToFileEntryPtr M2 = addRef(Refs, "2", E2);
+  OptionalFileEntryRefDegradesToFileEntryPtr M1 = Refs.addFile("1");
+  OptionalFileEntryRefDegradesToFileEntryPtr M2 = Refs.addFile("2");
   OptionalFileEntryRefDegradesToFileEntryPtr M0Also = None;
   OptionalFileEntryRefDegradesToFileEntryPtr M1Also =
-      addRef(Refs, "1-also", E1);
+      Refs.addFileAlias("1-also", *M1);
 
   EXPECT_EQ(M0, M0Also);
   EXPECT_EQ(StringRef("1"), M1->getName());
   EXPECT_EQ(StringRef("2"), M2->getName());
   EXPECT_EQ(StringRef("1-also"), M1Also->getName());
 
-  EXPECT_EQ(&E1, &M1->getFileEntry());
-  EXPECT_EQ(&E2, &M2->getFileEntry());
-  EXPECT_EQ(&E1, &M1Also->getFileEntry());
-
   const FileEntry *CE1 = M1;
-  EXPECT_EQ(CE1, &E1);
+  EXPECT_EQ(CE1, &M1->getFileEntry());
 }
 
 TEST(FileEntryTest, equals) {
-  MapType Refs;
-  FileEntry E1, E2;
-  FileEntryRef R1 = addRef(Refs, "1", E1);
-  FileEntryRef R2 = addRef(Refs, "2", E2);
-  FileEntryRef R1Also = addRef(Refs, "1-also", E1);
+  RefMaps Refs;
+  FileEntryRef R1 = Refs.addFile("1");
+  FileEntryRef R2 = Refs.addFile("2");
+  FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
 
-  EXPECT_EQ(R1, &E1);
-  EXPECT_EQ(&E1, R1);
+  EXPECT_EQ(R1, &R1.getFileEntry());
+  EXPECT_EQ(&R1.getFileEntry(), R1);
   EXPECT_EQ(R1, R1Also);
-  EXPECT_NE(R1, &E2);
-  EXPECT_NE(&E2, R1);
+  EXPECT_NE(R1, &R2.getFileEntry());
+  EXPECT_NE(&R2.getFileEntry(), R1);
   EXPECT_NE(R1, R2);
 
   OptionalFileEntryRefDegradesToFileEntryPtr M0;
   OptionalFileEntryRefDegradesToFileEntryPtr M1 = R1;
 
-  EXPECT_EQ(M1, &E1);
-  EXPECT_EQ(&E1, M1);
-  EXPECT_NE(M1, &E2);
-  EXPECT_NE(&E2, M1);
+  EXPECT_EQ(M1, &R1.getFileEntry());
+  EXPECT_EQ(&R1.getFileEntry(), M1);
+  EXPECT_NE(M1, &R2.getFileEntry());
+  EXPECT_NE(&R2.getFileEntry(), M1);
 }
 
 TEST(FileEntryTest, isSameRef) {
-  MapType Refs;
-  FileEntry E1, E2;
-  FileEntryRef R1 = addRef(Refs, "1", E1);
-  FileEntryRef R2 = addRef(Refs, "2", E2);
-  FileEntryRef R1Also = addRef(Refs, "1-also", E1);
+  RefMaps Refs;
+  FileEntryRef R1 = Refs.addFile("1");
+  FileEntryRef R2 = Refs.addFile("2");
+  FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
 
   EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1)));
   EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1.getMapEntry())));
   EXPECT_FALSE(R1.isSameRef(R2));
   EXPECT_FALSE(R1.isSameRef(R1Also));
 }
 
 } // end namespace