benlangmuir created this revision.
Herald added subscribers: arphaman, martong.
Herald added a reviewer: shafik.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Refactor ContentCache::OrigEntry and Filename to be private and only
expose them from FileInfo. This is in preparation for removing these
fields from ContentCache.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137303

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/tools/libclang/CIndexInclusionStack.cpp

Index: clang/tools/libclang/CIndexInclusionStack.cpp
===================================================================
--- clang/tools/libclang/CIndexInclusionStack.cpp
+++ clang/tools/libclang/CIndexInclusionStack.cpp
@@ -35,7 +35,7 @@
       continue;
 
     const SrcMgr::FileInfo &FI = SL.getFile();
-    if (!FI.getContentCache().OrigEntry)
+    if (!FI.getOrigEntry())
       continue;
 
     // If this is the main file, and there is a preamble, skip this SLoc. The
@@ -60,7 +60,7 @@
     // Callback to the client.
     // FIXME: We should have a function to construct CXFiles.
     CB(static_cast<CXFile>(const_cast<FileEntry *>(
-           static_cast<const FileEntry *>(FI.getContentCache().OrigEntry))),
+           static_cast<const FileEntry *>(FI.getOrigEntry()))),
        InclusionStack.data(), InclusionStack.size(), clientData);
   }
 }
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1542,21 +1542,21 @@
       continue;
     const SrcMgr::FileInfo &File = SLoc->getFile();
     const SrcMgr::ContentCache *Cache = &File.getContentCache();
-    if (!Cache->OrigEntry)
+    if (!File.getOrigEntry())
       continue;
 
     if (isModuleMap(File.getFileCharacteristic()) &&
         !isSystem(File.getFileCharacteristic()) &&
         !AffectingClangModuleMaps.empty() &&
-        AffectingClangModuleMaps.find(Cache->OrigEntry) ==
+        AffectingClangModuleMaps.find(File.getOrigEntry()) ==
             AffectingClangModuleMaps.end()) {
-      SkippedModuleMaps.insert(Cache->OrigEntry);
+      SkippedModuleMaps.insert(File.getOrigEntry());
       // Do not emit modulemaps that do not affect current module.
       continue;
     }
 
     InputFileEntry Entry;
-    Entry.File = Cache->OrigEntry;
+    Entry.File = File.getOrigEntry();
     Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
     Entry.IsTransient = Cache->IsTransient;
     Entry.BufferOverridden = Cache->BufferOverridden;
@@ -2052,8 +2052,7 @@
     // Figure out which record code to use.
     unsigned Code;
     if (SLoc->isFile()) {
-      const SrcMgr::ContentCache *Cache = &SLoc->getFile().getContentCache();
-      if (Cache->OrigEntry) {
+      if (SLoc->getFile().getOrigEntry()) {
         Code = SM_SLOC_FILE_ENTRY;
       } else
         Code = SM_SLOC_BUFFER_ENTRY;
@@ -2066,10 +2065,10 @@
     Record.push_back(SLoc->getOffset() - 2);
     if (SLoc->isFile()) {
       const SrcMgr::FileInfo &File = SLoc->getFile();
+      const FileEntry *OrigEntry = File.getOrigEntry();
       const SrcMgr::ContentCache *Content = &File.getContentCache();
-      if (Content->OrigEntry && !SkippedModuleMaps.empty() &&
-          SkippedModuleMaps.find(Content->OrigEntry) !=
-              SkippedModuleMaps.end()) {
+      if (OrigEntry && !SkippedModuleMaps.empty() &&
+          SkippedModuleMaps.find(OrigEntry) != SkippedModuleMaps.end()) {
         // Do not emit files that were not listed as inputs.
         continue;
       }
@@ -2078,13 +2077,13 @@
       Record.push_back(File.hasLineDirectives());
 
       bool EmitBlob = false;
-      if (Content->OrigEntry) {
-        assert(Content->OrigEntry == Content->ContentsEntry &&
+      if (OrigEntry) {
+        assert(OrigEntry == Content->ContentsEntry &&
                "Writing to AST an overridden file is not supported");
 
         // The source location entry is a file. Emit input file ID.
-        assert(InputFileIDs[Content->OrigEntry] != 0 && "Missed file entry");
-        Record.push_back(InputFileIDs[Content->OrigEntry]);
+        assert(InputFileIDs[OrigEntry] != 0 && "Missed file entry");
+        Record.push_back(InputFileIDs[OrigEntry]);
 
         Record.push_back(File.NumCreatedFIDs);
 
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1553,7 +1553,7 @@
     const SrcMgr::ContentCache &ContentCache =
         SourceMgr.getOrCreateContentCache(*File, isSystem(FileCharacter));
     if (OverriddenBuffer && !ContentCache.BufferOverridden &&
-        ContentCache.ContentsEntry == ContentCache.OrigEntry &&
+        ContentCache.ContentsEntry == FileInfo.getOrigEntry() &&
         !ContentCache.getBufferIfLoaded()) {
       auto Buffer = ReadBuffer(SLocEntryCursor, File->getName());
       if (!Buffer)
Index: clang/lib/Basic/SourceManager.cpp
===================================================================
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -358,18 +358,23 @@
   return false;
 }
 
+ContentCache ContentCache::createUnownedCopy(const ContentCache &Other) {
+  ContentCache Clone;
+  Clone.OrigEntry = Other.OrigEntry;
+  Clone.ContentsEntry = Other.ContentsEntry;
+  Clone.BufferOverridden = Other.BufferOverridden;
+  Clone.IsFileVolatile = Other.IsFileVolatile;
+  Clone.IsTransient = Other.IsTransient;
+  Clone.setUnownedBuffer(Other.getBufferIfLoaded());
+  return Clone;
+}
+
 void SourceManager::initializeForReplay(const SourceManager &Old) {
   assert(MainFileID.isInvalid() && "expected uninitialized SourceManager");
 
   auto CloneContentCache = [&](const ContentCache *Cache) -> ContentCache * {
-    auto *Clone = new (ContentCacheAlloc.Allocate<ContentCache>()) ContentCache;
-    Clone->OrigEntry = Cache->OrigEntry;
-    Clone->ContentsEntry = Cache->ContentsEntry;
-    Clone->BufferOverridden = Cache->BufferOverridden;
-    Clone->IsFileVolatile = Cache->IsFileVolatile;
-    Clone->IsTransient = Cache->IsTransient;
-    Clone->setUnownedBuffer(Cache->getBufferIfLoaded());
-    return Clone;
+    return new (ContentCacheAlloc.Allocate<ContentCache>())
+        ContentCache(ContentCache::createUnownedCopy(*Cache));
   };
 
   // Ensure all SLocEntries are loaded from the external source.
@@ -730,7 +735,7 @@
 Optional<StringRef>
 SourceManager::getNonBuiltinFilenameForID(FileID FID) const {
   if (const SrcMgr::SLocEntry *Entry = getSLocEntryForFile(FID))
-    if (Entry->getFile().getContentCache().OrigEntry)
+    if (Entry->getFile().getOrigEntry())
       return Entry->getFile().getName();
   return None;
 }
@@ -1535,8 +1540,8 @@
   // MemBuffer.
   FileID FID = LocInfo.first;
   StringRef Filename;
-  if (C->OrigEntry)
-    Filename = C->OrigEntry->getName();
+  if (Optional<FileEntryRef> E = FI.getOrigEntry())
+    Filename = E->getName();
   else if (auto Buffer = C->getBufferOrNone(Diag, getFileManager()))
     Filename = Buffer->getBufferIdentifier();
 
@@ -1666,7 +1671,7 @@
       return FileID();
 
     if (MainSLoc.isFile()) {
-      if (MainSLoc.getFile().getContentCache().OrigEntry == SourceFile)
+      if (MainSLoc.getFile().getOrigEntry() == SourceFile)
         return MainFileID;
     }
   }
@@ -1675,16 +1680,14 @@
   // through all of the local source locations.
   for (unsigned I = 0, N = local_sloc_entry_size(); I != N; ++I) {
     const SLocEntry &SLoc = getLocalSLocEntry(I);
-    if (SLoc.isFile() &&
-        SLoc.getFile().getContentCache().OrigEntry == SourceFile)
+    if (SLoc.isFile() && SLoc.getFile().getOrigEntry() == SourceFile)
       return FileID::get(I);
   }
 
   // If that still didn't help, try the modules.
   for (unsigned I = 0, N = loaded_sloc_entry_size(); I != N; ++I) {
     const SLocEntry &SLoc = getLoadedSLocEntry(I);
-    if (SLoc.isFile() &&
-        SLoc.getFile().getContentCache().OrigEntry == SourceFile)
+    if (SLoc.isFile() && SLoc.getFile().getOrigEntry() == SourceFile)
       return FileID::get(-int(I) - 2);
   }
 
@@ -2193,11 +2196,12 @@
       if (FI.getIncludeLoc().isValid())
         out << "  included from " << FI.getIncludeLoc().getOffset() << "\n";
       auto &CC = FI.getContentCache();
-      out << "  for " << (CC.OrigEntry ? CC.OrigEntry->getName() : "<none>")
+      out << "  for "
+          << (FI.getOrigEntry() ? FI.getOrigEntry()->getName() : "<none>")
           << "\n";
       if (CC.BufferOverridden)
         out << "  contents overridden\n";
-      if (CC.ContentsEntry != CC.OrigEntry) {
+      if (CC.ContentsEntry != FI.getOrigEntry()) {
         out << "  contents from "
             << (CC.ContentsEntry ? CC.ContentsEntry->getName() : "<none>")
             << "\n";
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -9512,7 +9512,8 @@
     }
     ToID = ToSM.getFileID(MLoc);
   } else {
-    const SrcMgr::ContentCache *Cache = &FromSLoc.getFile().getContentCache();
+    const SrcMgr::FileInfo &FI = FromSLoc.getFile();
+    const SrcMgr::ContentCache *Cache = &FI.getContentCache();
 
     if (!IsBuiltin && !Cache->BufferOverridden) {
       // Include location of this file.
@@ -9529,13 +9530,13 @@
       if (FromID == FromSM.getMainFileID())
         ToIncludeLocOrFakeLoc = ToSM.getLocForStartOfFile(ToSM.getMainFileID());
 
-      if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
+      if (FI.getOrigEntry() && FI.getOrigEntry()->getDir()) {
         // FIXME: We probably want to use getVirtualFile(), so we don't hit the
         // disk again
         // FIXME: We definitely want to re-use the existing MemoryBuffer, rather
         // than mmap the files several times.
         auto Entry =
-            ToFileManager.getOptionalFileRef(Cache->OrigEntry->getName());
+            ToFileManager.getOptionalFileRef(FI.getOrigEntry()->getName());
         // FIXME: The filename may be a virtual name that does probably not
         // point to a valid file and we get no Entry here. In this case try with
         // the memory buffer below.
Index: clang/include/clang/Basic/SourceManager.h
===================================================================
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -131,7 +131,8 @@
   /// file.
   mutable std::unique_ptr<llvm::MemoryBuffer> Buffer;
 
-public:
+  friend class FileInfo;
+
   /// Reference to the file entry representing this ContentCache.
   ///
   /// This reference does not own the FileEntry object.
@@ -143,17 +144,18 @@
   /// Filename and use \c OrigEntry.getNameAsRequested() instead.
   OptionalFileEntryRefDegradesToFileEntryPtr OrigEntry;
 
+  /// The filename that is used to access OrigEntry.
+  ///
+  /// FIXME: Remove this once OrigEntry is a FileEntryRef with a stable name.
+  StringRef Filename;
+
+public:
   /// References the file which the contents were actually loaded from.
   ///
   /// Can be different from 'Entry' if we overridden the contents of one file
   /// with the contents of another file.
   const FileEntry *ContentsEntry;
 
-  /// The filename that is used to access OrigEntry.
-  ///
-  /// FIXME: Remove this once OrigEntry is a FileEntryRef with a stable name.
-  StringRef Filename;
-
   /// A bump pointer allocated array of offsets for each source line.
   ///
   /// This is lazily computed.  The lines are owned by the SourceManager
@@ -188,19 +190,12 @@
       : OrigEntry(Ent), ContentsEntry(contentEnt), BufferOverridden(false),
         IsFileVolatile(false), IsTransient(false), IsBufferInvalid(false) {}
 
-  /// The copy ctor does not allow copies where source object has either
-  /// a non-NULL Buffer or SourceLineCache.  Ownership of allocated memory
-  /// is not transferred, so this is a logical error.
-  ContentCache(const ContentCache &RHS)
-      : BufferOverridden(false), IsFileVolatile(false), IsTransient(false),
-        IsBufferInvalid(false) {
-    OrigEntry = RHS.OrigEntry;
-    ContentsEntry = RHS.ContentsEntry;
-
-    assert(!RHS.Buffer && !RHS.SourceLineCache &&
-           "Passed ContentCache object cannot own a buffer.");
-  }
+  /// Create a copy of \p Other with an unowned reference to its buffer as if
+  /// set using \c setUnownedBuffer.
+  static ContentCache createUnownedCopy(const ContentCache &Other);
 
+  ContentCache(ContentCache &&RHS) = default;
+  ContentCache(const ContentCache &RHS) = delete;
   ContentCache &operator=(const ContentCache &RHS) = delete;
 
   /// Returns the memory buffer for the associated content.
@@ -345,6 +340,10 @@
   /// Returns the name of the file that was used when the file was loaded from
   /// the underlying file system.
   StringRef getName() const { return getContentCache().Filename; }
+
+  OptionalFileEntryRefDegradesToFileEntryPtr getOrigEntry() const {
+    return getContentCache().OrigEntry;
+  }
 };
 
 /// Each ExpansionInfo encodes the expansion location - where
@@ -1042,14 +1041,14 @@
   /// Returns the FileEntry record for the provided FileID.
   const FileEntry *getFileEntryForID(FileID FID) const {
     if (auto *Entry = getSLocEntryForFile(FID))
-      return Entry->getFile().getContentCache().OrigEntry;
+      return Entry->getFile().getOrigEntry();
     return nullptr;
   }
 
   /// Returns the FileEntryRef for the provided FileID.
   Optional<FileEntryRef> getFileEntryRefForID(FileID FID) const {
     if (auto *Entry = getSLocEntryForFile(FID))
-      return Entry->getFile().getContentCache().OrigEntry;
+      return Entry->getFile().getOrigEntry();
     return None;
   }
 
@@ -1062,7 +1061,7 @@
   /// Returns the FileEntry record for the provided SLocEntry.
   const FileEntry *getFileEntryForSLocEntry(const SrcMgr::SLocEntry &sloc) const
   {
-    return sloc.getFile().getContentCache().OrigEntry;
+    return sloc.getFile().getOrigEntry();
   }
 
   /// Return a StringRef to the source buffer data for the
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to