[clang] [clang-tools-extra] [clang] NFC: Remove `OptionalFileEntryRefDegradesToFileEntryPtr` (PR #74899)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-tidy

Author: Jan Svoboda (jansvoboda11)

<details>
<summary>Changes</summary>



---
Full diff: https://github.com/llvm/llvm-project/pull/74899.diff


18 Files Affected:

- (modified) clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp (+1-1) 
- (modified) clang/include/clang/Basic/FileEntry.h (-66) 
- (modified) clang/include/clang/Basic/Module.h (+1-1) 
- (modified) clang/include/clang/Basic/SourceManager.h (+8-6) 
- (modified) clang/include/clang/Lex/PreprocessorLexer.h (+1-1) 
- (modified) clang/include/clang/Serialization/ModuleFile.h (+1-1) 
- (modified) clang/lib/Frontend/CompilerInstance.cpp (+1-1) 
- (modified) clang/lib/Lex/ModuleMap.cpp (+1-3) 
- (modified) clang/lib/Lex/PPDirectives.cpp (+2-1) 
- (modified) clang/lib/Lex/Pragma.cpp (+1-1) 
- (modified) clang/lib/Lex/PreprocessorLexer.cpp (+1-2) 
- (modified) clang/lib/Serialization/ASTReader.cpp (+3-3) 
- (modified) clang/lib/Serialization/ASTWriter.cpp (+3-3) 
- (modified) clang/lib/Serialization/ModuleManager.cpp (+2-2) 
- (modified) clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp (+1-1) 
- (modified) clang/tools/libclang/CXIndexDataConsumer.cpp (+3-3) 
- (modified) clang/tools/libclang/CXIndexDataConsumer.h (+1-1) 
- (modified) clang/unittests/Basic/FileEntryTest.cpp (-25) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
index 52cc2e6569b052..0b1e9f59e1a70c 100644
--- a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -171,7 +171,7 @@ void ExpandModularHeadersPPCallbacks::InclusionDirective(
   if (Imported) {
     serialization::ModuleFile *MF =
         Compiler.getASTReader()->getModuleManager().lookup(
-            Imported->getASTFile());
+            *Imported->getASTFile());
     handleModuleFile(MF);
   }
   parseToLocation(DirectiveLoc);
diff --git a/clang/include/clang/Basic/FileEntry.h b/clang/include/clang/Basic/FileEntry.h
index 6351aeae92e2c4..35efa147950f06 100644
--- a/clang/include/clang/Basic/FileEntry.h
+++ b/clang/include/clang/Basic/FileEntry.h
@@ -279,72 +279,6 @@ template <> struct DenseMapInfo<clang::FileEntryRef> {
 
 namespace clang {
 
-/// Wrapper around OptionalFileEntryRef that degrades to 'const FileEntry*',
-/// facilitating incremental patches to propagate FileEntryRef.
-///
-/// This class can be used as return value or field where it's convenient for
-/// an OptionalFileEntryRef to degrade to a 'const FileEntry*'. The purpose
-/// is to avoid code churn due to dances like the following:
-/// \code
-/// // Old code.
-/// lvalue = rvalue;
-///
-/// // Temporary code from an incremental patch.
-/// OptionalFileEntryRef MaybeF = rvalue;
-/// lvalue = MaybeF ? &MaybeF.getFileEntry() : nullptr;
-///
-/// // Final code.
-/// lvalue = rvalue;
-/// \endcode
-///
-/// FIXME: Once FileEntryRef is "everywhere" and FileEntry::LastRef and
-/// FileEntry::getName have been deleted, delete this class and replace
-/// instances with OptionalFileEntryRef.
-class OptionalFileEntryRefDegradesToFileEntryPtr : public OptionalFileEntryRef {
-public:
-  OptionalFileEntryRefDegradesToFileEntryPtr() = default;
-  OptionalFileEntryRefDegradesToFileEntryPtr(
-      OptionalFileEntryRefDegradesToFileEntryPtr &&) = default;
-  OptionalFileEntryRefDegradesToFileEntryPtr(
-      const OptionalFileEntryRefDegradesToFileEntryPtr &) = default;
-  OptionalFileEntryRefDegradesToFileEntryPtr &
-  operator=(OptionalFileEntryRefDegradesToFileEntryPtr &&) = default;
-  OptionalFileEntryRefDegradesToFileEntryPtr &
-  operator=(const OptionalFileEntryRefDegradesToFileEntryPtr &) = default;
-
-  OptionalFileEntryRefDegradesToFileEntryPtr(std::nullopt_t) {}
-  OptionalFileEntryRefDegradesToFileEntryPtr(FileEntryRef Ref)
-      : OptionalFileEntryRef(Ref) {}
-  OptionalFileEntryRefDegradesToFileEntryPtr(OptionalFileEntryRef MaybeRef)
-      : OptionalFileEntryRef(MaybeRef) {}
-
-  OptionalFileEntryRefDegradesToFileEntryPtr &operator=(std::nullopt_t) {
-    OptionalFileEntryRef::operator=(std::nullopt);
-    return *this;
-  }
-  OptionalFileEntryRefDegradesToFileEntryPtr &operator=(FileEntryRef Ref) {
-    OptionalFileEntryRef::operator=(Ref);
-    return *this;
-  }
-  OptionalFileEntryRefDegradesToFileEntryPtr &
-  operator=(OptionalFileEntryRef MaybeRef) {
-    OptionalFileEntryRef::operator=(MaybeRef);
-    return *this;
-  }
-
-  /// Degrade to 'const FileEntry *' to allow  FileEntry::LastRef and
-  /// FileEntry::getName have been deleted, delete this class and replace
-  /// instances with OptionalFileEntryRef
-  operator const FileEntry *() const {
-    return has_value() ? &(*this)->getFileEntry() : nullptr;
-  }
-};
-
-static_assert(
-    std::is_trivially_copyable<
-        OptionalFileEntryRefDegradesToFileEntryPtr>::value,
-    "OptionalFileEntryRefDegradesToFileEntryPtr should be trivially copyable");
-
 inline bool operator==(const FileEntry *LHS, const OptionalFileEntryRef &RHS) {
   return LHS == (RHS ? &RHS->getFileEntry() : nullptr);
 }
diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index d29cc0b45d583e..23f263c89511a6 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -672,7 +672,7 @@ class alignas(8) Module {
   }
 
   /// The serialized AST file for this module, if one was created.
-  OptionalFileEntryRefDegradesToFileEntryPtr getASTFile() const {
+  OptionalFileEntryRef getASTFile() const {
     return getTopLevelModule()->ASTFile;
   }
 
diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h
index 985ea6354b8219..d2ece14da0b11a 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -143,7 +143,7 @@ class alignas(8) ContentCache {
   ///
   /// FIXME: Make non-optional using a virtual file as needed, remove \c
   /// Filename and use \c OrigEntry.getNameAsRequested() instead.
-  OptionalFileEntryRefDegradesToFileEntryPtr OrigEntry;
+  OptionalFileEntryRef OrigEntry;
 
   /// References the file which the contents were actually loaded from.
   ///
@@ -1064,8 +1064,8 @@ class SourceManager : public RefCountedBase<SourceManager> {
 
   /// Returns the FileEntry record for the provided FileID.
   const FileEntry *getFileEntryForID(FileID FID) const {
-    if (auto *Entry = getSLocEntryForFile(FID))
-      return Entry->getFile().getContentCache().OrigEntry;
+    if (auto FE = getFileEntryRefForID(FID))
+      return *FE;
     return nullptr;
   }
 
@@ -1083,9 +1083,11 @@ class SourceManager : public RefCountedBase<SourceManager> {
   std::optional<StringRef> getNonBuiltinFilenameForID(FileID FID) const;
 
   /// Returns the FileEntry record for the provided SLocEntry.
-  const FileEntry *getFileEntryForSLocEntry(const SrcMgr::SLocEntry &sloc) const
-  {
-    return sloc.getFile().getContentCache().OrigEntry;
+  const FileEntry *
+  getFileEntryForSLocEntry(const SrcMgr::SLocEntry &SLocEntry) const {
+    if (auto FE = SLocEntry.getFile().getContentCache().OrigEntry)
+      return *FE;
+    return nullptr;
   }
 
   /// Return a StringRef to the source buffer data for the
diff --git a/clang/include/clang/Lex/PreprocessorLexer.h b/clang/include/clang/Lex/PreprocessorLexer.h
index eebaad7d50db3b..d71fe708ab20a2 100644
--- a/clang/include/clang/Lex/PreprocessorLexer.h
+++ b/clang/include/clang/Lex/PreprocessorLexer.h
@@ -157,7 +157,7 @@ class PreprocessorLexer {
 
   /// getFileEntry - Return the FileEntry corresponding to this FileID.  Like
   /// getFileID(), this only works for lexers with attached preprocessors.
-  OptionalFileEntryRefDegradesToFileEntryPtr getFileEntry() const;
+  OptionalFileEntryRef getFileEntry() const;
 
   /// Iterator that traverses the current stack of preprocessor
   /// conditional directives (\#if/\#ifdef/\#ifndef).
diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h
index 70ab61dec8b6bf..9a14129d72ff33 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -104,7 +104,7 @@ class InputFile {
     return File;
   }
 
-  OptionalFileEntryRefDegradesToFileEntryPtr getFile() const {
+  OptionalFileEntryRef getFile() const {
     if (auto *P = Val.getPointer())
       return FileEntryRef(*P);
     return std::nullopt;
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index e5f8c0746a99dd..56bbef9697b650 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -2260,7 +2260,7 @@ GlobalModuleIndex *CompilerInstance::loadGlobalModuleIndex(
     for (ModuleMap::module_iterator I = MMap.module_begin(),
         E = MMap.module_end(); I != E; ++I) {
       Module *TheModule = I->second;
-      const FileEntry *Entry = TheModule->getASTFile();
+      OptionalFileEntryRef Entry = TheModule->getASTFile();
       if (!Entry) {
         SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Path;
         Path.push_back(std::make_pair(
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 1d67e275cb4775..268b72c966ab81 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -1067,9 +1067,7 @@ Module *ModuleMap::inferFrameworkModule(DirectoryEntryRef FrameworkDir,
     if (!canInfer)
       return nullptr;
   } else {
-    OptionalFileEntryRefDegradesToFileEntryPtr ModuleMapRef =
-        getModuleMapFileForUniquing(Parent);
-    ModuleMapFile = ModuleMapRef;
+    ModuleMapFile = getModuleMapFileForUniquing(Parent);
   }
 
   // Look for an umbrella header.
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 956e2276f25b71..14003480d7fa2e 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -1934,7 +1934,8 @@ Preprocessor::getIncludeNextStart(const Token &IncludeNextTok) const {
     // Start looking up in the directory *after* the one in which the current
     // file would be found, if any.
     assert(CurPPLexer && "#include_next directive in macro?");
-    LookupFromFile = CurPPLexer->getFileEntry();
+    if (auto FE = CurPPLexer->getFileEntry())
+      LookupFromFile = *FE;
     Lookup = nullptr;
   } else if (!Lookup) {
     // The current file was not found by walking the include path. Either it
diff --git a/clang/lib/Lex/Pragma.cpp b/clang/lib/Lex/Pragma.cpp
index 35ab42cb6b5ef8..499813f8ab7df0 100644
--- a/clang/lib/Lex/Pragma.cpp
+++ b/clang/lib/Lex/Pragma.cpp
@@ -548,7 +548,7 @@ void Preprocessor::HandlePragmaDependency(Token &DependencyTok) {
     return;
   }
 
-  const FileEntry *CurFile = getCurrentFileLexer()->getFileEntry();
+  OptionalFileEntryRef CurFile = getCurrentFileLexer()->getFileEntry();
 
   // If this file is older than the file it depends on, emit a diagnostic.
   if (CurFile && CurFile->getModificationTime() < File->getModificationTime()) {
diff --git a/clang/lib/Lex/PreprocessorLexer.cpp b/clang/lib/Lex/PreprocessorLexer.cpp
index 23c80d375214c6..7551ba235fe9b8 100644
--- a/clang/lib/Lex/PreprocessorLexer.cpp
+++ b/clang/lib/Lex/PreprocessorLexer.cpp
@@ -47,7 +47,6 @@ void PreprocessorLexer::LexIncludeFilename(Token &FilenameTok) {
 
 /// getFileEntry - Return the FileEntry corresponding to this FileID.  Like
 /// getFileID(), this only works for lexers with attached preprocessors.
-OptionalFileEntryRefDegradesToFileEntryPtr
-PreprocessorLexer::getFileEntry() const {
+OptionalFileEntryRef PreprocessorLexer::getFileEntry() const {
   return PP->getSourceManager().getFileEntryRefForID(getFileID());
 }
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 49f25d6648c801..ac9bddb5c36b99 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2531,8 +2531,7 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) {
     Overridden = false;
   }
 
-  OptionalFileEntryRefDegradesToFileEntryPtr File = OptionalFileEntryRef(
-      expectedToOptional(FileMgr.getFileRef(Filename, /*OpenFile=*/false)));
+  auto File = FileMgr.getOptionalFileRef(Filename, /*OpenFile=*/false);
 
   // For an overridden file, create a virtual file with the stored
   // size/timestamp.
@@ -2559,7 +2558,8 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) {
   // PCH.
   SourceManager &SM = getSourceManager();
   // FIXME: Reject if the overrides are different.
-  if ((!Overridden && !Transient) && !SkipChecks && SM.isFileOverridden(File)) {
+  if ((!Overridden && !Transient) && !SkipChecks &&
+      SM.isFileOverridden(*File)) {
     if (Complain)
       Error(diag::err_fe_pch_file_overridden, Filename);
 
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 38e8b8ccbe058d..91eb2af8f8ad6a 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -2182,8 +2182,8 @@ void ASTWriter::WriteSourceManagerBlock(SourceManager &SourceMgr,
                "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[*Content->OrigEntry] != 0 && "Missed file entry");
+        Record.push_back(InputFileIDs[*Content->OrigEntry]);
 
         Record.push_back(getAdjustedNumCreatedFIDs(FID));
 
@@ -4695,7 +4695,7 @@ void ASTWriter::collectNonAffectingInputFiles() {
 
     if (!isModuleMap(File.getFileCharacteristic()) ||
         AffectingModuleMaps.empty() ||
-        AffectingModuleMaps.find(Cache->OrigEntry) != AffectingModuleMaps.end())
+        llvm::is_contained(AffectingModuleMaps, *Cache->OrigEntry))
       continue;
 
     IsSLocAffecting[I] = false;
diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp
index d1ded6cd8ee282..51b6429412960e 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/clang/lib/Serialization/ModuleManager.cpp
@@ -52,8 +52,8 @@ ModuleFile *ModuleManager::lookupByFileName(StringRef Name) const {
 
 ModuleFile *ModuleManager::lookupByModuleName(StringRef Name) const {
   if (const Module *Mod = HeaderSearchInfo.getModuleMap().findModule(Name))
-    if (const FileEntry *File = Mod->getASTFile())
-      return lookup(File);
+    if (OptionalFileEntryRef File = Mod->getASTFile())
+      return lookup(*File);
 
   return nullptr;
 }
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 1058ddb8254cde..f65da413bb87c3 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -521,7 +521,7 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
 
   serialization::ModuleFile *MF =
       MDC.ScanInstance.getASTReader()->getModuleManager().lookup(
-          M->getASTFile());
+          *M->getASTFile());
   MDC.ScanInstance.getASTReader()->visitInputFileInfos(
       *MF, /*IncludeSystem=*/true,
       [&](const serialization::InputFileInfo &IFI, bool IsSystem) {
diff --git a/clang/tools/libclang/CXIndexDataConsumer.cpp b/clang/tools/libclang/CXIndexDataConsumer.cpp
index 5ca484fbc8cd82..a327d39ee05e1b 100644
--- a/clang/tools/libclang/CXIndexDataConsumer.cpp
+++ b/clang/tools/libclang/CXIndexDataConsumer.cpp
@@ -1074,8 +1074,8 @@ CXIndexDataConsumer::getClientContainerForDC(const DeclContext *DC) const {
   return DC ? ContainerMap.lookup(DC) : nullptr;
 }
 
-CXIdxClientFile CXIndexDataConsumer::getIndexFile(const FileEntry *File) {
-  return File ? FileMap.lookup(File) : nullptr;
+CXIdxClientFile CXIndexDataConsumer::getIndexFile(OptionalFileEntryRef File) {
+  return File ? FileMap.lookup(*File) : nullptr;
 }
 
 CXIdxLoc CXIndexDataConsumer::getIndexLoc(SourceLocation Loc) const {
@@ -1105,7 +1105,7 @@ void CXIndexDataConsumer::translateLoc(SourceLocation Loc,
   if (FID.isInvalid())
     return;
   
-  OptionalFileEntryRefDegradesToFileEntryPtr FE = SM.getFileEntryRefForID(FID);
+  OptionalFileEntryRef FE = SM.getFileEntryRefForID(FID);
   if (indexFile)
     *indexFile = getIndexFile(FE);
   if (file)
diff --git a/clang/tools/libclang/CXIndexDataConsumer.h b/clang/tools/libclang/CXIndexDataConsumer.h
index afa2239ed653f9..eb9b2eaae22513 100644
--- a/clang/tools/libclang/CXIndexDataConsumer.h
+++ b/clang/tools/libclang/CXIndexDataConsumer.h
@@ -460,7 +460,7 @@ class CXIndexDataConsumer : public index::IndexDataConsumer {
 
   const DeclContext *getEntityContainer(const Decl *D) const;
 
-  CXIdxClientFile getIndexFile(const FileEntry *File);
+  CXIdxClientFile getIndexFile(OptionalFileEntryRef File);
   
   CXIdxLoc getIndexLoc(SourceLocation Loc) const;
 
diff --git a/clang/unittests/Basic/FileEntryTest.cpp b/clang/unittests/Basic/FileEntryTest.cpp
index dcd196417da731..f8a0b4a4edcdaf 100644
--- a/clang/unittests/Basic/FileEntryTest.cpp
+++ b/clang/unittests/Basic/FileEntryTest.cpp
@@ -92,24 +92,6 @@ TEST(FileEntryTest, FileEntryRef) {
   EXPECT_EQ(CE1, &R1.getFileEntry());
 }
 
-TEST(FileEntryTest, OptionalFileEntryRefDegradesToFileEntryPtr) {
-  FileEntryTestHelper Refs;
-  OptionalFileEntryRefDegradesToFileEntryPtr M0;
-  OptionalFileEntryRefDegradesToFileEntryPtr M1 = Refs.addFile("1");
-  OptionalFileEntryRefDegradesToFileEntryPtr M2 = Refs.addFile("2");
-  OptionalFileEntryRefDegradesToFileEntryPtr M0Also = std::nullopt;
-  OptionalFileEntryRefDegradesToFileEntryPtr M1Also =
-      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());
-
-  const FileEntry *CE1 = M1;
-  EXPECT_EQ(CE1, &M1->getFileEntry());
-}
-
 TEST(FileEntryTest, equals) {
   FileEntryTestHelper Refs;
   FileEntryRef R1 = Refs.addFile("1");
@@ -126,13 +108,6 @@ TEST(FileEntryTest, equals) {
   EXPECT_NE(R1, R2);
   EXPECT_EQ(R1, R1Redirect);
   EXPECT_EQ(R1, R1Redirect2);
-
-  OptionalFileEntryRefDegradesToFileEntryPtr M1 = R1;
-
-  EXPECT_EQ(M1, &R1.getFileEntry());
-  EXPECT_EQ(&R1.getFileEntry(), M1);
-  EXPECT_NE(M1, &R2.getFileEntry());
-  EXPECT_NE(&R2.getFileEntry(), M1);
 }
 
 TEST(FileEntryTest, isSameRef) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/74899