[clang] NFCI: Use FileEntryRef for FileID creation (#67838)

This patch removes the `SourceManager` APIs that create `FileID` from a `const FileEntry *` in favor of APIs that take `FileEntryRef`. This also removes a misleading documentation that claims `nullptr` file entry represents stdin. I don't think that's right, since we just try to dereference that pointer anyways.
llvm · Oct 3, 2023 · 27254ae · 27254ae
1 parent c6fed74
commit 27254ae
Show file tree

Hide file tree

Showing 11 changed files with 20 additions and 40 deletions.
diff --git a/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp b/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp
@@ -152,8 +152,8 @@ int main(int argc, const char **argv) {
       for (auto I = ChangedFiles.begin(), E = ChangedFiles.end(); I != E; ++I) {
         OS << "  {\n";
         OS << "    \"FilePath\": \"" << *I << "\",\n";
-        const auto Entry = FileMgr.getFile(*I);
-        auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
+        auto Entry = llvm::cantFail(FileMgr.getFileRef(*I));
+        auto ID = Sources.getOrCreateFileID(Entry, SrcMgr::C_User);
         std::string Content;
         llvm::raw_string_ostream ContentStream(Content);
         Rewrite.getEditBuffer(ID).write(ContentStream);
@@ -170,9 +170,9 @@ int main(int argc, const char **argv) {
   }
 
   for (const auto &File : ChangedFiles) {
-    const auto Entry = FileMgr.getFile(File);
+    auto Entry = llvm::cantFail(FileMgr.getFileRef(File));
 
-    auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
+    auto ID = Sources.getOrCreateFileID(Entry, SrcMgr::C_User);
     outs() << "============== " << File << " ==============\n";
     Rewrite.getEditBuffer(ID).write(llvm::outs());
     outs() << "\n============================================\n";

diff --git a/clang-tools-extra/clang-move/Move.cpp b/clang-tools-extra/clang-move/Move.cpp
@@ -843,7 +843,7 @@ void ClangMoveTool::moveDeclsToNewFiles() {
 // Move all contents from OldFile to NewFile.
 void ClangMoveTool::moveAll(SourceManager &SM, StringRef OldFile,
                             StringRef NewFile) {
-  auto FE = SM.getFileManager().getFile(makeAbsolutePath(OldFile));
+  auto FE = SM.getFileManager().getOptionalFileRef(makeAbsolutePath(OldFile));
   if (!FE) {
     llvm::errs() << "Failed to get file: " << OldFile << "\n";
     return;

diff --git a/clang-tools-extra/clang-reorder-fields/tool/ClangReorderFields.cpp b/clang-tools-extra/clang-reorder-fields/tool/ClangReorderFields.cpp
@@ -84,8 +84,8 @@ int main(int argc, const char **argv) {
   Tool.applyAllReplacements(Rewrite);
 
   for (const auto &File : Files) {
-    auto Entry = FileMgr.getFile(File);
-    const auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
+    auto Entry = llvm::cantFail(FileMgr.getFileRef(File));
+    const auto ID = Sources.getOrCreateFileID(Entry, SrcMgr::C_User);
     Rewrite.getEditBuffer(ID).write(outs());
   }
 

diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -243,7 +243,7 @@ class ErrorReporter {
     if (FilePath.empty())
       return {};
 
-    auto File = SourceMgr.getFileManager().getFile(FilePath);
+    auto File = SourceMgr.getFileManager().getOptionalFileRef(FilePath);
     if (!File)
       return {};
 

diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -194,9 +194,9 @@ DiagnosticBuilder ClangTidyContext::diag(
 
 DiagnosticBuilder ClangTidyContext::diag(const tooling::Diagnostic &Error) {
   SourceManager &SM = DiagEngine->getSourceManager();
-  llvm::ErrorOr<const FileEntry *> File =
-      SM.getFileManager().getFile(Error.Message.FilePath);
-  FileID ID = SM.getOrCreateFileID(*File, SrcMgr::C_User);
+  FileManager &FM = SM.getFileManager();
+  FileEntryRef File = llvm::cantFail(FM.getFileRef(Error.Message.FilePath));
+  FileID ID = SM.getOrCreateFileID(File, SrcMgr::C_User);
   SourceLocation FileStartLoc = SM.getLocForStartOfFile(ID);
   SourceLocation Loc = FileStartLoc.getLocWithOffset(
       static_cast<SourceLocation::IntTy>(Error.Message.FileOffset));

diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h
@@ -877,13 +877,6 @@ class SourceManager : public RefCountedBase<SourceManager> {
 
   /// Create a new FileID that represents the specified file
   /// being \#included from the specified IncludePosition.
-  ///
-  /// This translates NULL into standard input.
-  FileID createFileID(const FileEntry *SourceFile, SourceLocation IncludePos,
-                      SrcMgr::CharacteristicKind FileCharacter,
-                      int LoadedID = 0,
-                      SourceLocation::UIntTy LoadedOffset = 0);
-
   FileID createFileID(FileEntryRef SourceFile, SourceLocation IncludePos,
                       SrcMgr::CharacteristicKind FileCharacter,
                       int LoadedID = 0,
@@ -909,7 +902,7 @@ class SourceManager : public RefCountedBase<SourceManager> {
 
   /// Get the FileID for \p SourceFile if it exists. Otherwise, create a
   /// new FileID for the \p SourceFile.
-  FileID getOrCreateFileID(const FileEntry *SourceFile,
+  FileID getOrCreateFileID(FileEntryRef SourceFile,
                            SrcMgr::CharacteristicKind FileCharacter);
 
   /// Creates an expansion SLocEntry for the substitution of an argument into a

diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
@@ -527,17 +527,6 @@ FileID SourceManager::getNextFileID(FileID FID) const {
 
 /// Create a new FileID that represents the specified file
 /// being \#included from the specified IncludePosition.
-///
-/// This translates NULL into standard input.
-FileID SourceManager::createFileID(const FileEntry *SourceFile,
-                                   SourceLocation IncludePos,
-                                   SrcMgr::CharacteristicKind FileCharacter,
-                                   int LoadedID,
-                                   SourceLocation::UIntTy LoadedOffset) {
-  return createFileID(SourceFile->getLastRef(), IncludePos, FileCharacter,
-                      LoadedID, LoadedOffset);
-}
-
 FileID SourceManager::createFileID(FileEntryRef SourceFile,
                                    SourceLocation IncludePos,
                                    SrcMgr::CharacteristicKind FileCharacter,
@@ -585,7 +574,7 @@ FileID SourceManager::createFileID(const llvm::MemoryBufferRef &Buffer,
 /// Get the FileID for \p SourceFile if it exists. Otherwise, create a
 /// new FileID for the \p SourceFile.
 FileID
-SourceManager::getOrCreateFileID(const FileEntry *SourceFile,
+SourceManager::getOrCreateFileID(FileEntryRef SourceFile,
                                  SrcMgr::CharacteristicKind FileCharacter) {
   FileID ID = translateFile(SourceFile);
   return ID.isValid() ? ID : createFileID(SourceFile, SourceLocation(),
@@ -2375,8 +2364,9 @@ SourceManagerForFile::SourceManagerForFile(StringRef FileName,
       IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs),
       new DiagnosticOptions);
   SourceMgr = std::make_unique<SourceManager>(*Diagnostics, *FileMgr);
-  FileID ID = SourceMgr->createFileID(*FileMgr->getFile(FileName),
-                                      SourceLocation(), clang::SrcMgr::C_User);
+  FileEntryRef FE = llvm::cantFail(FileMgr->getFileRef(FileName));
+  FileID ID =
+      SourceMgr->createFileID(FE, SourceLocation(), clang::SrcMgr::C_User);
   assert(ID.isValid());
   SourceMgr->setMainFileID(ID);
 }
diff --git a/clang/lib/Tooling/Core/Replacement.cpp b/clang/lib/Tooling/Core/Replacement.cpp
@@ -67,7 +67,7 @@ bool Replacement::isApplicable() const {
 
 bool Replacement::apply(Rewriter &Rewrite) const {
   SourceManager &SM = Rewrite.getSourceMgr();
-  auto Entry = SM.getFileManager().getFile(FilePath);
+  auto Entry = SM.getFileManager().getOptionalFileRef(FilePath);
   if (!Entry)
     return false;
 

diff --git a/clang/lib/Tooling/Refactoring.cpp b/clang/lib/Tooling/Refactoring.cpp
@@ -78,10 +78,7 @@ bool formatAndApplyAllReplacements(
     const std::string &FilePath = FileAndReplaces.first;
     auto &CurReplaces = FileAndReplaces.second;
 
-    const FileEntry *Entry = nullptr;
-    if (auto File = Files.getFile(FilePath))
-      Entry = *File;
-
+    FileEntryRef Entry = llvm::cantFail(Files.getFileRef(FilePath));
     FileID ID = SM.getOrCreateFileID(Entry, SrcMgr::C_User);
     StringRef Code = SM.getBufferData(ID);
 

diff --git a/clang/tools/clang-rename/ClangRename.cpp b/clang/tools/clang-rename/ClangRename.cpp
@@ -228,7 +228,7 @@ int main(int argc, const char **argv) {
 
     Tool.applyAllReplacements(Rewrite);
     for (const auto &File : Files) {
-      auto Entry = FileMgr.getFile(File);
+      auto Entry = FileMgr.getOptionalFileRef(File);
       if (!Entry) {
         errs() << "clang-rename: " << File << " does not exist.\n";
         return 1;

diff --git a/clang/unittests/Analysis/UnsafeBufferUsageTest.cpp b/clang/unittests/Analysis/UnsafeBufferUsageTest.cpp
@@ -25,7 +25,7 @@ class UnsafeBufferUsageTest : public ::testing::Test {
 } // namespace
 
 TEST_F(UnsafeBufferUsageTest, FixItHintsConflict) {
-  const FileEntry *DummyFile = FileMgr.getVirtualFile("<virtual>", 100, 0);
+  FileEntryRef DummyFile = FileMgr.getVirtualFileRef("<virtual>", 100, 0);
   FileID DummyFileID = SourceMgr.getOrCreateFileID(DummyFile, SrcMgr::C_User);
   SourceLocation StartLoc = SourceMgr.getLocForStartOfFile(DummyFileID);