dexonsmith created this revision.
dexonsmith added reviewers: arphaman, JDevlieghere, teemperor.
Herald added a subscriber: ributzka.
dexonsmith requested review of this revision.

Change the `InputFile` class to store a `MaybeFileEntryRef` instead of
`FileEntry*`. This paged in a few API changes:

- Added `FileManager::getVirtualFileRef`, and converted `getVirtualFile` to a 
wrapper of it.
- Added `{Maybe,}FileEntryRef::getMapEntry`, which gives access to the 
underlying `StringMapEntry*` pointer, to enable clients (`InputFile`) to store 
a `FileEntryRef` in a `PointerIntPair`.
- Updated `SourceManager::bypassFileContentsOverride` to take `FileEntryRef` 
and return `MaybeFileEntry` (`ASTReader::getInputFile` is the only caller).


https://reviews.llvm.org/D90053

Files:
  clang/include/clang/Basic/FileEntry.h
  clang/include/clang/Basic/FileManager.h
  clang/include/clang/Basic/SourceManager.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/unittests/Basic/FileEntryTest.cpp

Index: clang/unittests/Basic/FileEntryTest.cpp
===================================================================
--- clang/unittests/Basic/FileEntryTest.cpp
+++ clang/unittests/Basic/FileEntryTest.cpp
@@ -112,6 +112,7 @@
   FileEntryRef R1Also = addRef(Refs, "1-also", E1);
 
   EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1)));
+  EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1.getMapEntry())));
   EXPECT_FALSE(R1.isSameRef(R2));
   EXPECT_FALSE(R1.isSameRef(R1Also));
 
@@ -123,6 +124,7 @@
   EXPECT_FALSE(M0.isSameRef(M1));
   EXPECT_FALSE(M1.isSameRef(M0));
   EXPECT_TRUE(M1.isSameRef(MaybeFileEntryRef(M1)));
+  EXPECT_TRUE(M1.isSameRef(MaybeFileEntryRef(M1.getMapEntry())));
   EXPECT_FALSE(M1.isSameRef(M1Also));
   EXPECT_FALSE(M1.isSameRef(M2));
   EXPECT_TRUE(M1.isSameRef(R1));
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -2312,27 +2312,25 @@
   StringRef Filename = FI.Filename;
   uint64_t StoredContentHash = FI.ContentHash;
 
-  const FileEntry *File = nullptr;
-  if (auto FE = FileMgr.getFile(Filename, /*OpenFile=*/false))
-    File = *FE;
+  MaybeFileEntryRef File =
+      expectedToOptional(FileMgr.getFileRef(Filename, /*OpenFile=*/false));
 
   // If we didn't find the file, resolve it relative to the
   // original directory from which this AST file was created.
-  if (File == nullptr && !F.OriginalDir.empty() && !F.BaseDirectory.empty() &&
+  if (!File && !F.OriginalDir.empty() && !F.BaseDirectory.empty() &&
       F.OriginalDir != F.BaseDirectory) {
     std::string Resolved = resolveFileRelativeToOriginalDir(
         std::string(Filename), F.OriginalDir, F.BaseDirectory);
     if (!Resolved.empty())
-      if (auto FE = FileMgr.getFile(Resolved))
-        File = *FE;
+      File = expectedToOptional(FileMgr.getFileRef(Resolved));
   }
 
   // For an overridden file, create a virtual file with the stored
   // size/timestamp.
-  if ((Overridden || Transient) && File == nullptr)
-    File = FileMgr.getVirtualFile(Filename, StoredSize, StoredTime);
+  if ((Overridden || Transient) && !File)
+    File = FileMgr.getVirtualFileRef(Filename, StoredSize, StoredTime);
 
-  if (File == nullptr) {
+  if (!File) {
     if (Complain) {
       std::string ErrorStr = "could not find file '";
       ErrorStr += Filename;
@@ -2443,7 +2441,7 @@
   // FIXME: If the file is overridden and we've already opened it,
   // issue an error (or split it into a separate FileEntry).
 
-  InputFile IF = InputFile(File, Overridden || Transient, IsOutOfDate);
+  InputFile IF = InputFile(*File, Overridden || Transient, IsOutOfDate);
 
   // Note that we've loaded this input file.
   F.InputFilesLoaded[ID-1] = IF;
@@ -9195,7 +9193,7 @@
     InputFileInfo IFI = readInputFileInfo(MF, I + 1);
     if (IFI.TopLevelModuleMap)
       // FIXME: This unnecessarily re-reads the InputFileInfo.
-      if (auto *FE = getInputFile(MF, I + 1).getFile())
+      if (auto FE = getInputFile(MF, I + 1).getFile())
         Visitor(FE);
   }
 }
Index: clang/lib/Basic/SourceManager.cpp
===================================================================
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -703,19 +703,16 @@
   getOverriddenFilesInfo().OverriddenFiles[SourceFile] = NewFile;
 }
 
-const FileEntry *
-SourceManager::bypassFileContentsOverride(const FileEntry &File) {
-  assert(isFileOverridden(&File));
-  llvm::Optional<FileEntryRef> BypassFile =
-      FileMgr.getBypassFile(File.getLastRef());
+MaybeFileEntryRef SourceManager::bypassFileContentsOverride(FileEntryRef File) {
+  assert(isFileOverridden(&File.getFileEntry()));
+  llvm::Optional<FileEntryRef> BypassFile = FileMgr.getBypassFile(File);
 
   // If the file can't be found in the FS, give up.
   if (!BypassFile)
-    return nullptr;
+    return None;
 
-  const FileEntry *FE = &BypassFile->getFileEntry();
-  (void)getOrCreateContentCache(FE);
-  return FE;
+  (void)getOrCreateContentCache(&BypassFile->getFileEntry());
+  return BypassFile;
 }
 
 void SourceManager::setFileIsTransient(const FileEntry *File) {
Index: clang/lib/Basic/FileManager.cpp
===================================================================
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -329,9 +329,13 @@
   return *(UFE.LastRef = FileEntryRef(*NamedFileEnt));
 }
 
-const FileEntry *
-FileManager::getVirtualFile(StringRef Filename, off_t Size,
-                            time_t ModificationTime) {
+const FileEntry *FileManager::getVirtualFile(StringRef Filename, off_t Size,
+                                             time_t ModificationTime) {
+  return &getVirtualFileRef(Filename, Size, ModificationTime).getFileEntry();
+}
+
+FileEntryRef FileManager::getVirtualFileRef(StringRef Filename, off_t Size,
+                                            time_t ModificationTime) {
   ++NumFileLookups;
 
   // See if there is already an entry in the map for an existing file.
@@ -339,11 +343,9 @@
       {Filename, std::errc::no_such_file_or_directory}).first;
   if (NamedFileEnt.second) {
     FileEntryRef::MapValue Value = *NamedFileEnt.second;
-    FileEntry *FE;
-    if (LLVM_LIKELY(FE = Value.V.dyn_cast<FileEntry *>()))
-      return FE;
-    return &FileEntryRef(*Value.V.get<const FileEntryRef::MapEntry *>())
-                .getFileEntry();
+    if (LLVM_LIKELY(Value.V.is<FileEntry *>()))
+      return FileEntryRef(NamedFileEnt);
+    return FileEntryRef(*Value.V.get<const FileEntryRef::MapEntry *>());
   }
 
   // We've not seen this before, or the file is cached as non-existent.
@@ -382,7 +384,7 @@
     // FIXME: Surely this should add a reference by the new name, and return
     // it instead...
     if (UFE->isValid())
-      return UFE;
+      return FileEntryRef(NamedFileEnt);
 
     UFE->UniqueID = Status.getUniqueID();
     UFE->IsNamedPipe = Status.getType() == llvm::sys::fs::file_type::fifo_file;
@@ -400,7 +402,7 @@
   UFE->UID     = NextFileUID++;
   UFE->IsValid = true;
   UFE->File.reset();
-  return UFE;
+  return FileEntryRef(NamedFileEnt);
 }
 
 llvm::Optional<FileEntryRef> FileManager::getBypassFile(FileEntryRef VF) {
Index: clang/include/clang/Serialization/ModuleFile.h
===================================================================
--- clang/include/clang/Serialization/ModuleFile.h
+++ clang/include/clang/Serialization/ModuleFile.h
@@ -67,13 +67,13 @@
     OutOfDate = 2,
     NotFound = 3
   };
-  llvm::PointerIntPair<const FileEntry *, 2, unsigned> Val;
+  llvm::PointerIntPair<const FileEntryRef::MapEntry *, 2, unsigned> Val;
 
 public:
   InputFile() = default;
 
-  InputFile(const FileEntry *File,
-            bool isOverridden = false, bool isOutOfDate = false) {
+  InputFile(FileEntryRef File, bool isOverridden = false,
+            bool isOutOfDate = false) {
     assert(!(isOverridden && isOutOfDate) &&
            "an overridden cannot be out-of-date");
     unsigned intVal = 0;
@@ -81,7 +81,7 @@
       intVal = Overridden;
     else if (isOutOfDate)
       intVal = OutOfDate;
-    Val.setPointerAndInt(File, intVal);
+    Val.setPointerAndInt(&File.getMapEntry(), intVal);
   }
 
   static InputFile getNotFound() {
@@ -90,7 +90,9 @@
     return File;
   }
 
-  const FileEntry *getFile() const { return Val.getPointer(); }
+  MaybeFileEntryRef getFile() const {
+    return MaybeFileEntryRef(Val.getPointer());
+  }
   bool isOverridden() const { return Val.getInt() == Overridden; }
   bool isOutOfDate() const { return Val.getInt() == OutOfDate; }
   bool isNotFound() const { return Val.getInt() == NotFound; }
Index: clang/include/clang/Basic/SourceManager.h
===================================================================
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -35,6 +35,7 @@
 #define LLVM_CLANG_BASIC_SOURCEMANAGER_H
 
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitVector.h"
@@ -60,8 +61,6 @@
 class ASTReader;
 class ASTWriter;
 class FileManager;
-class FileEntry;
-class FileEntryRef;
 class LineTableInfo;
 class SourceManager;
 
@@ -969,11 +968,11 @@
   }
 
   /// Bypass the overridden contents of a file.  This creates a new FileEntry
-  /// and initializes the content cache for it.  Returns nullptr if there is no
+  /// and initializes the content cache for it.  Returns None if there is no
   /// such file in the filesystem.
   ///
   /// This should be called before parsing has begun.
-  const FileEntry *bypassFileContentsOverride(const FileEntry &File);
+  MaybeFileEntryRef bypassFileContentsOverride(FileEntryRef File);
 
   /// Specify that a file is transient.
   void setFileIsTransient(const FileEntry *SourceFile);
Index: clang/include/clang/Basic/FileManager.h
===================================================================
--- clang/include/clang/Basic/FileManager.h
+++ clang/include/clang/Basic/FileManager.h
@@ -268,6 +268,9 @@
   /// if there were a file with the given name on disk.
   ///
   /// The file itself is not accessed.
+  FileEntryRef getVirtualFileRef(StringRef Filename, off_t Size,
+                                 time_t ModificationTime);
+
   const FileEntry *getVirtualFile(StringRef Filename, off_t Size,
                                   time_t ModificationTime);
 
Index: clang/include/clang/Basic/FileEntry.h
===================================================================
--- clang/include/clang/Basic/FileEntry.h
+++ clang/include/clang/Basic/FileEntry.h
@@ -124,6 +124,8 @@
   inline time_t getModificationTime() const;
   inline void closeFile() const;
 
+  const MapEntry &getMapEntry() const { return *ME; }
+
   explicit FileEntryRef(const MapEntry &ME) : FileEntryRefBase(&ME) {}
 };
 
@@ -166,6 +168,8 @@
   MaybeFileEntryRef(Optional<FileEntryRef> F)
       : FileEntryRefBase(F ? F->ME : nullptr) {}
 
+  const MapEntry *getMapEntry() const { return ME; }
+
   explicit MaybeFileEntryRef(const MapEntry *ME) : FileEntryRefBase(ME) {}
   MaybeFileEntryRef(llvm::NoneType = None) : FileEntryRefBase(nullptr) {}
   MaybeFileEntryRef(FileEntryRef F) : FileEntryRefBase(F) {}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D90053: Se... Duncan P. N. Exon Smith via Phabricator via cfe-commits

Reply via email to