[clang] [clang-tools-extra] [clang] Make deprecations of some `FileManager` APIs formal (PR #110014)

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 25 09:48:12 PDT 2024


https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/110014

Some `FileManager` APIs still return `{File,Directory}Entry` instead of the preferred `{File,Directory}EntryRef`. These are documented to be deprecated, but don't have the attribute that warns on their usage. This PR marks them as such with `LLVM_DEPRECATED()` and replaces their usage with the recommended counterparts. NFCI.

>From 6b413a95887bb12cf7bae647173dd7b3c6a47684 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 23 Sep 2024 17:00:48 -0700
Subject: [PATCH 1/4] [clang] Formally deprecate `FileManager::getFile()`

---
 .../clang-move/tool/ClangMove.cpp             |  2 +-
 clang-tools-extra/clangd/SourceCode.cpp       |  4 +-
 .../clangd/unittests/ParsedASTTests.cpp       |  4 +-
 .../unittests/FindHeadersTest.cpp             |  2 +-
 .../include-cleaner/unittests/RecordTest.cpp  | 84 +++++++++----------
 clang/include/clang/Basic/FileManager.h       |  2 +
 clang/lib/CodeGen/CodeGenAction.cpp           |  4 +-
 clang/lib/ExtractAPI/ExtractAPIConsumer.cpp   |  4 +-
 clang/lib/Frontend/ASTUnit.cpp                |  2 +-
 clang/lib/Frontend/CompilerInstance.cpp       |  4 +-
 .../lib/Frontend/Rewrite/FrontendActions.cpp  |  2 +-
 clang/lib/InstallAPI/Frontend.cpp             |  2 +-
 clang/lib/Lex/HeaderSearch.cpp                |  4 +-
 clang/lib/Serialization/ASTReader.cpp         |  6 +-
 clang/lib/Serialization/ModuleManager.cpp     |  8 +-
 clang/lib/Tooling/Core/Replacement.cpp        |  2 +-
 .../DependencyScanning/ModuleDepCollector.cpp |  8 +-
 clang/tools/clang-refactor/ClangRefactor.cpp  |  2 +-
 clang/tools/clang-refactor/TestSupport.cpp    |  2 +-
 clang/unittests/Basic/FileManagerTest.cpp     | 42 +++++-----
 .../Frontend/CompilerInstanceTest.cpp         |  2 +-
 21 files changed, 99 insertions(+), 93 deletions(-)

diff --git a/clang-tools-extra/clang-move/tool/ClangMove.cpp b/clang-tools-extra/clang-move/tool/ClangMove.cpp
index 1560dcaad67793..655ea81ee37d4f 100644
--- a/clang-tools-extra/clang-move/tool/ClangMove.cpp
+++ b/clang-tools-extra/clang-move/tool/ClangMove.cpp
@@ -199,7 +199,7 @@ int main(int argc, const char **argv) {
       for (auto I = Files.begin(), E = Files.end(); I != E; ++I) {
         OS << "  {\n";
         OS << "    \"FilePath\": \"" << *I << "\",\n";
-        const auto Entry = FileMgr.getFile(*I);
+        const auto Entry = FileMgr.getOptionalFileRef(*I);
         auto ID = SM.translateFile(*Entry);
         std::string Content;
         llvm::raw_string_ostream ContentStream(Content);
diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 3af99b9db056da..780aaa471dc8b6 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -814,8 +814,8 @@ llvm::SmallVector<llvm::StringRef> ancestorNamespaces(llvm::StringRef NS) {
 
 // Checks whether \p FileName is a valid spelling of main file.
 bool isMainFile(llvm::StringRef FileName, const SourceManager &SM) {
-  auto FE = SM.getFileManager().getFile(FileName);
-  return FE && *FE == SM.getFileEntryForID(SM.getMainFileID());
+  auto FE = SM.getFileManager().getOptionalFileRef(FileName);
+  return FE && FE == SM.getFileEntryRefForID(SM.getMainFileID());
 }
 
 } // namespace
diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index 4bb76cd6ab8304..6ee641caeefe3d 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -397,10 +397,10 @@ TEST(ParsedASTTest, PatchesAdditionalIncludes) {
   auto &FM = SM.getFileManager();
   // Copy so that we can use operator[] to get the children.
   IncludeStructure Includes = PatchedAST->getIncludeStructure();
-  auto MainFE = FM.getFile(testPath("foo.cpp"));
+  auto MainFE = FM.getOptionalFileRef(testPath("foo.cpp"));
   ASSERT_TRUE(MainFE);
   auto MainID = Includes.getID(*MainFE);
-  auto AuxFE = FM.getFile(testPath("sub/aux.h"));
+  auto AuxFE = FM.getOptionalFileRef(testPath("sub/aux.h"));
   ASSERT_TRUE(AuxFE);
   auto AuxID = Includes.getID(*AuxFE);
   EXPECT_THAT(Includes.IncludeChildren[*MainID], Contains(*AuxID));
diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
index c5fc465ced7a75..84e02e1d0d621b 100644
--- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -60,7 +60,7 @@ class FindHeadersTest : public testing::Test {
   llvm::SmallVector<Hinted<Header>> findHeaders(llvm::StringRef FileName) {
     return include_cleaner::findHeaders(
         AST->sourceManager().translateFileLineCol(
-            AST->fileManager().getFile(FileName).get(),
+            *AST->fileManager().getOptionalFileRef(FileName),
             /*Line=*/1, /*Col=*/1),
         AST->sourceManager(), &PI);
   }
diff --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
index 0b05c9190cb67f..b5a7b9720903eb 100644
--- a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -234,7 +234,7 @@ TEST_F(RecordPPTest, CapturesMacroRefs) {
   const auto &SM = AST.sourceManager();
 
   SourceLocation Def = SM.getComposedLoc(
-      SM.translateFile(AST.fileManager().getFile("header.h").get()),
+      SM.translateFile(*AST.fileManager().getOptionalFileRef("header.h")),
       Header.point("def"));
   ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));
   Symbol OrigX = Recorded.MacroReferences.front().Target;
@@ -368,29 +368,29 @@ TEST_F(PragmaIncludeTest, IWYUKeep) {
   TestAST Processed = build();
   auto &FM = Processed.fileManager();
 
-  EXPECT_FALSE(PI.shouldKeep(FM.getFile("normal.h").get()));
-  EXPECT_FALSE(PI.shouldKeep(FM.getFile("std/vector").get()));
+  EXPECT_FALSE(PI.shouldKeep(*FM.getOptionalFileRef("normal.h")));
+  EXPECT_FALSE(PI.shouldKeep(*FM.getOptionalFileRef("std/vector")));
 
   // Keep
-  EXPECT_TRUE(PI.shouldKeep(FM.getFile("keep1.h").get()));
-  EXPECT_TRUE(PI.shouldKeep(FM.getFile("keep2.h").get()));
-  EXPECT_TRUE(PI.shouldKeep(FM.getFile("keep3.h").get()));
-  EXPECT_TRUE(PI.shouldKeep(FM.getFile("keep4.h").get()));
-  EXPECT_TRUE(PI.shouldKeep(FM.getFile("keep5.h").get()));
-  EXPECT_TRUE(PI.shouldKeep(FM.getFile("keep6.h").get()));
-  EXPECT_TRUE(PI.shouldKeep(FM.getFile("std/map").get()));
+  EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("keep1.h")));
+  EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("keep2.h")));
+  EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("keep3.h")));
+  EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("keep4.h")));
+  EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("keep5.h")));
+  EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("keep6.h")));
+  EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("std/map")));
 
   // Exports
-  EXPECT_TRUE(PI.shouldKeep(FM.getFile("export1.h").get()));
-  EXPECT_TRUE(PI.shouldKeep(FM.getFile("export2.h").get()));
-  EXPECT_TRUE(PI.shouldKeep(FM.getFile("export3.h").get()));
-  EXPECT_TRUE(PI.shouldKeep(FM.getFile("std/set").get()));
+  EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("export1.h")));
+  EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("export2.h")));
+  EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("export3.h")));
+  EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("std/set")));
 }
 
 TEST_F(PragmaIncludeTest, AssociatedHeader) {
   createEmptyFiles({"foo/main.h", "bar/main.h", "bar/other.h", "std/vector"});
   auto IsKeep = [&](llvm::StringRef Name, TestAST &AST) {
-    return PI.shouldKeep(AST.fileManager().getFile(Name).get());
+    return PI.shouldKeep(*AST.fileManager().getOptionalFileRef(Name));
   };
 
   Inputs.FileName = "main.cc";
@@ -452,19 +452,19 @@ TEST_F(PragmaIncludeTest, IWYUPrivate) {
     // IWYU pragma: private
   )cpp";
   TestAST Processed = build();
-  auto PrivateFE = Processed.fileManager().getFile("private.h");
+  auto PrivateFE = Processed.fileManager().getOptionalFileRef("private.h");
   assert(PrivateFE);
-  EXPECT_TRUE(PI.isPrivate(PrivateFE.get()));
-  EXPECT_EQ(PI.getPublic(PrivateFE.get()), "\"public2.h\"");
+  EXPECT_TRUE(PI.isPrivate(*PrivateFE));
+  EXPECT_EQ(PI.getPublic(*PrivateFE), "\"public2.h\"");
 
-  auto PublicFE = Processed.fileManager().getFile("public.h");
+  auto PublicFE = Processed.fileManager().getOptionalFileRef("public.h");
   assert(PublicFE);
-  EXPECT_EQ(PI.getPublic(PublicFE.get()), ""); // no mapping.
-  EXPECT_FALSE(PI.isPrivate(PublicFE.get()));
+  EXPECT_EQ(PI.getPublic(*PublicFE), ""); // no mapping.
+  EXPECT_FALSE(PI.isPrivate(*PublicFE));
 
-  auto Private2FE = Processed.fileManager().getFile("private2.h");
+  auto Private2FE = Processed.fileManager().getOptionalFileRef("private2.h");
   assert(Private2FE);
-  EXPECT_TRUE(PI.isPrivate(Private2FE.get()));
+  EXPECT_TRUE(PI.isPrivate(*Private2FE));
 }
 
 TEST_F(PragmaIncludeTest, IWYUExport) {
@@ -486,13 +486,13 @@ TEST_F(PragmaIncludeTest, IWYUExport) {
   const auto &SM = Processed.sourceManager();
   auto &FM = Processed.fileManager();
 
-  EXPECT_THAT(PI.getExporters(FM.getFile("private.h").get(), FM),
+  EXPECT_THAT(PI.getExporters(*FM.getOptionalFileRef("private.h"), FM),
               testing::UnorderedElementsAre(FileNamed("export1.h"),
                                             FileNamed("export3.h")));
 
-  EXPECT_TRUE(PI.getExporters(FM.getFile("export1.h").get(), FM).empty());
-  EXPECT_TRUE(PI.getExporters(FM.getFile("export2.h").get(), FM).empty());
-  EXPECT_TRUE(PI.getExporters(FM.getFile("export3.h").get(), FM).empty());
+  EXPECT_TRUE(PI.getExporters(*FM.getOptionalFileRef("export1.h"), FM).empty());
+  EXPECT_TRUE(PI.getExporters(*FM.getOptionalFileRef("export2.h"), FM).empty());
+  EXPECT_TRUE(PI.getExporters(*FM.getOptionalFileRef("export3.h"), FM).empty());
   EXPECT_TRUE(
       PI.getExporters(SM.getFileEntryForID(SM.getMainFileID()), FM).empty());
 }
@@ -548,23 +548,23 @@ TEST_F(PragmaIncludeTest, IWYUExportBlock) {
     }
     return Result;
   };
-  auto Exporters = PI.getExporters(FM.getFile("private1.h").get(), FM);
+  auto Exporters = PI.getExporters(*FM.getOptionalFileRef("private1.h"), FM);
   EXPECT_THAT(Exporters, testing::UnorderedElementsAre(FileNamed("export1.h"),
                                                        FileNamed("normal.h")))
       << GetNames(Exporters);
 
-  Exporters = PI.getExporters(FM.getFile("private2.h").get(), FM);
+  Exporters = PI.getExporters(*FM.getOptionalFileRef("private2.h"), FM);
   EXPECT_THAT(Exporters, testing::UnorderedElementsAre(FileNamed("export1.h")))
       << GetNames(Exporters);
 
-  Exporters = PI.getExporters(FM.getFile("private3.h").get(), FM);
+  Exporters = PI.getExporters(*FM.getOptionalFileRef("private3.h"), FM);
   EXPECT_THAT(Exporters, testing::UnorderedElementsAre(FileNamed("export1.h")))
       << GetNames(Exporters);
 
-  Exporters = PI.getExporters(FM.getFile("foo.h").get(), FM);
+  Exporters = PI.getExporters(*FM.getOptionalFileRef("foo.h"), FM);
   EXPECT_TRUE(Exporters.empty()) << GetNames(Exporters);
 
-  Exporters = PI.getExporters(FM.getFile("bar.h").get(), FM);
+  Exporters = PI.getExporters(*FM.getOptionalFileRef("bar.h"), FM);
   EXPECT_TRUE(Exporters.empty()) << GetNames(Exporters);
 }
 
@@ -580,8 +580,8 @@ TEST_F(PragmaIncludeTest, SelfContained) {
   Inputs.ExtraFiles["unguarded.h"] = "";
   TestAST Processed = build();
   auto &FM = Processed.fileManager();
-  EXPECT_TRUE(PI.isSelfContained(FM.getFile("guarded.h").get()));
-  EXPECT_FALSE(PI.isSelfContained(FM.getFile("unguarded.h").get()));
+  EXPECT_TRUE(PI.isSelfContained(*FM.getOptionalFileRef("guarded.h")));
+  EXPECT_FALSE(PI.isSelfContained(*FM.getOptionalFileRef("unguarded.h")));
 }
 
 TEST_F(PragmaIncludeTest, AlwaysKeep) {
@@ -596,8 +596,8 @@ TEST_F(PragmaIncludeTest, AlwaysKeep) {
   Inputs.ExtraFiles["usual.h"] = "#pragma once";
   TestAST Processed = build();
   auto &FM = Processed.fileManager();
-  EXPECT_TRUE(PI.shouldKeep(FM.getFile("always_keep.h").get()));
-  EXPECT_FALSE(PI.shouldKeep(FM.getFile("usual.h").get()));
+  EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("always_keep.h")));
+  EXPECT_FALSE(PI.shouldKeep(*FM.getOptionalFileRef("usual.h")));
 }
 
 TEST_F(PragmaIncludeTest, ExportInUnnamedBuffer) {
@@ -653,13 +653,13 @@ TEST_F(PragmaIncludeTest, OutlivesFMAndSM) {
   // Now this build gives us a new File&Source Manager.
   TestAST Processed = build(/*ResetPragmaIncludes=*/false);
   auto &FM = Processed.fileManager();
-  auto PrivateFE = FM.getFile("private.h");
+  auto PrivateFE = FM.getOptionalFileRef("private.h");
   assert(PrivateFE);
-  EXPECT_EQ(PI.getPublic(PrivateFE.get()), "\"public.h\"");
+  EXPECT_EQ(PI.getPublic(*PrivateFE), "\"public.h\"");
 
-  auto Private2FE = FM.getFile("private2.h");
+  auto Private2FE = FM.getOptionalFileRef("private2.h");
   assert(Private2FE);
-  EXPECT_THAT(PI.getExporters(Private2FE.get(), FM),
+  EXPECT_THAT(PI.getExporters(*Private2FE, FM),
               testing::ElementsAre(llvm::cantFail(FM.getFileRef("public.h"))));
 }
 
@@ -676,8 +676,8 @@ TEST_F(PragmaIncludeTest, CanRecordManyTimes) {
 
   TestAST Processed = build();
   auto &FM = Processed.fileManager();
-  auto PrivateFE = FM.getFile("private.h");
-  llvm::StringRef Public = PI.getPublic(PrivateFE.get());
+  auto PrivateFE = FM.getOptionalFileRef("private.h");
+  llvm::StringRef Public = PI.getPublic(*PrivateFE);
   EXPECT_EQ(Public, "\"public.h\"");
 
   // This build populates same PI during build, but this time we don't have
diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h
index 74029a91d1a6d0..1846feb23716db 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -207,6 +207,8 @@ class FileManager : public RefCountedBase<FileManager> {
   ///
   /// \param CacheFailure If true and the file does not exist, we'll cache
   /// the failure to find this file.
+  LLVM_DEPRECATED("Functions returning FileEntry are deprecated.",
+                  "getOptionalFileRef()")
   llvm::ErrorOr<const FileEntry *>
   getFile(StringRef Filename, bool OpenFile = false, bool CacheFailure = true);
 
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 883333f0924ddb..c9f9b688d0d8a2 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -586,9 +586,9 @@ const FullSourceLoc BackendConsumer::getBestLocationFromDebugLoc(
   if (D.isLocationAvailable()) {
     D.getLocation(Filename, Line, Column);
     if (Line > 0) {
-      auto FE = FileMgr.getFile(Filename);
+      auto FE = FileMgr.getOptionalFileRef(Filename);
       if (!FE)
-        FE = FileMgr.getFile(D.getAbsolutePath());
+        FE = FileMgr.getOptionalFileRef(D.getAbsolutePath());
       if (FE) {
         // If -gcolumn-info was not used, Column will be 0. This upsets the
         // source manager, so pass 1 if Column is not set.
diff --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
index 75c2dec22400b9..6f42b36bd36a4b 100644
--- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -217,8 +217,8 @@ struct LocationFileChecker {
                       SmallVector<std::pair<SmallString<32>, bool>> &KnownFiles)
       : CI(CI), KnownFiles(KnownFiles), ExternalFileEntries() {
     for (const auto &KnownFile : KnownFiles)
-      if (auto FileEntry = CI.getFileManager().getFile(KnownFile.first))
-        KnownFileEntries.insert(*FileEntry);
+      if (auto FE = CI.getFileManager().getOptionalFileRef(KnownFile.first))
+        KnownFileEntries.insert(*FE);
   }
 
 private:
diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 93836ec5402faa..bffff0d27af3ab 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -2395,7 +2395,7 @@ void ASTUnit::TranslateStoredDiagnostics(
     // Rebuild the StoredDiagnostic.
     if (SD.Filename.empty())
       continue;
-    auto FE = FileMgr.getFile(SD.Filename);
+    auto FE = FileMgr.getOptionalFileRef(SD.Filename);
     if (!FE)
       continue;
     SourceLocation FileLoc;
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 5f2a9637e3ea46..08905a8debb1b8 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1925,8 +1925,8 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
     M = HS.lookupModule(ModuleName, ImportLoc, true, !IsInclusionDirective);
 
     // Check whether M refers to the file in the prebuilt module path.
-    if (M && M->getASTFile())
-      if (auto ModuleFile = FileMgr->getFile(ModuleFilename))
+    if (M && M->getASTFile() && M->getASTFile())
+      if (auto ModuleFile = FileMgr->getOptionalFileRef(ModuleFilename))
         if (*ModuleFile == M->getASTFile())
           return M;
 
diff --git a/clang/lib/Frontend/Rewrite/FrontendActions.cpp b/clang/lib/Frontend/Rewrite/FrontendActions.cpp
index cf5a9437e89e6c..6e1f949f543a51 100644
--- a/clang/lib/Frontend/Rewrite/FrontendActions.cpp
+++ b/clang/lib/Frontend/Rewrite/FrontendActions.cpp
@@ -213,7 +213,7 @@ class RewriteIncludesAction::RewriteImportsListener : public ASTReaderListener {
 
   void visitModuleFile(StringRef Filename,
                        serialization::ModuleKind Kind) override {
-    auto File = CI.getFileManager().getFile(Filename);
+    auto File = CI.getFileManager().getOptionalFileRef(Filename);
     assert(File && "missing file for loaded module?");
 
     // Only rewrite each module file once.
diff --git a/clang/lib/InstallAPI/Frontend.cpp b/clang/lib/InstallAPI/Frontend.cpp
index 04d06f46d26520..2ebe72bf021cf9 100644
--- a/clang/lib/InstallAPI/Frontend.cpp
+++ b/clang/lib/InstallAPI/Frontend.cpp
@@ -107,7 +107,7 @@ InstallAPIContext::findAndRecordFile(const FileEntry *FE,
 }
 
 void InstallAPIContext::addKnownHeader(const HeaderFile &H) {
-  auto FE = FM->getFile(H.getPath());
+  auto FE = FM->getOptionalFileRef(H.getPath());
   if (!FE)
     return; // File does not exist.
   KnownFiles[*FE] = H.getType();
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 4914c10e62d0c5..5f7e9533b58c89 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -227,7 +227,7 @@ std::string HeaderSearch::getPrebuiltModuleFileName(StringRef ModuleName,
                                           ".pcm");
     else
       llvm::sys::path::append(Result, ModuleName + ".pcm");
-    if (getFileMgr().getFile(Result.str()))
+    if (getFileMgr().getOptionalFileRef(Result))
       return std::string(Result);
   }
 
@@ -246,7 +246,7 @@ std::string HeaderSearch::getPrebuiltImplicitModuleFileName(Module *Module) {
     llvm::sys::path::append(CachePath, ModuleCacheHash);
     std::string FileName =
         getCachedModuleFileNameImpl(ModuleName, ModuleMapPath, CachePath);
-    if (!FileName.empty() && getFileMgr().getFile(FileName))
+    if (!FileName.empty() && getFileMgr().getOptionalFileRef(FileName))
       return FileName;
   }
   return {};
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index a369ad0be47954..1f7946e61d1757 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2044,14 +2044,14 @@ ASTReader::getGlobalPreprocessedEntityID(ModuleFile &M,
 const FileEntry *HeaderFileInfoTrait::getFile(const internal_key_type &Key) {
   FileManager &FileMgr = Reader.getFileManager();
   if (!Key.Imported) {
-    if (auto File = FileMgr.getFile(Key.Filename))
+    if (auto File = FileMgr.getOptionalFileRef(Key.Filename))
       return *File;
     return nullptr;
   }
 
   std::string Resolved = std::string(Key.Filename);
   Reader.ResolveImportedPath(M, Resolved);
-  if (auto File = FileMgr.getFile(Resolved))
+  if (auto File = FileMgr.getOptionalFileRef(Resolved))
     return *File;
   return nullptr;
 }
@@ -4217,7 +4217,7 @@ ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F,
     assert(M && M->Name == F.ModuleName && "found module with different name");
 
     // Check the primary module map file.
-    auto StoredModMap = FileMgr.getFile(F.ModuleMapPath);
+    auto StoredModMap = FileMgr.getOptionalFileRef(F.ModuleMapPath);
     if (!StoredModMap || *StoredModMap != ModMap) {
       assert(ModMap && "found module is missing module map file");
       assert((ImportedBy || F.Kind == MK_ImplicitModule) &&
diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp
index 51b6429412960e..fa19f68ae7e367 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/clang/lib/Serialization/ModuleManager.cpp
@@ -42,8 +42,8 @@ using namespace clang;
 using namespace serialization;
 
 ModuleFile *ModuleManager::lookupByFileName(StringRef Name) const {
-  auto Entry = FileMgr.getFile(Name, /*OpenFile=*/false,
-                               /*CacheFailure=*/false);
+  auto Entry = FileMgr.getOptionalFileRef(Name, /*OpenFile=*/false,
+                                          /*CacheFailure=*/false);
   if (Entry)
     return lookup(*Entry);
 
@@ -64,8 +64,8 @@ ModuleFile *ModuleManager::lookup(const FileEntry *File) const {
 
 std::unique_ptr<llvm::MemoryBuffer>
 ModuleManager::lookupBuffer(StringRef Name) {
-  auto Entry = FileMgr.getFile(Name, /*OpenFile=*/false,
-                               /*CacheFailure=*/false);
+  auto Entry = FileMgr.getOptionalFileRef(Name, /*OpenFile=*/false,
+                                          /*CacheFailure=*/false);
   if (!Entry)
     return nullptr;
   return std::move(InMemoryBuffers[*Entry]);
diff --git a/clang/lib/Tooling/Core/Replacement.cpp b/clang/lib/Tooling/Core/Replacement.cpp
index 89a5b152442740..92e9859ca206e5 100644
--- a/clang/lib/Tooling/Core/Replacement.cpp
+++ b/clang/lib/Tooling/Core/Replacement.cpp
@@ -614,7 +614,7 @@ std::map<std::string, Replacements> groupReplacementsByFile(
   std::map<std::string, Replacements> Result;
   llvm::SmallPtrSet<const FileEntry *, 16> ProcessedFileEntries;
   for (const auto &Entry : FileToReplaces) {
-    auto FE = FileMgr.getFile(Entry.first);
+    auto FE = FileMgr.getOptionalFileRef(Entry.first);
     if (!FE)
       llvm::errs() << "File path " << Entry.first << " is invalid.\n";
     else if (ProcessedFileEntries.insert(*FE).second)
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index c775adc0ddd73c..677f426590ab9e 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -241,7 +241,7 @@ ModuleDepCollector::getInvocationAdjustedForModuleBuildWithoutOutputs(
                                               ModuleMapInputKind);
 
   auto CurrentModuleMapEntry =
-      ScanInstance.getFileManager().getFile(Deps.ClangModuleMapFile);
+      ScanInstance.getFileManager().getOptionalFileRef(Deps.ClangModuleMapFile);
   assert(CurrentModuleMapEntry && "module map file entry not found");
 
   // Remove directly passed modulemap files. They will get added back if they
@@ -251,7 +251,8 @@ ModuleDepCollector::getInvocationAdjustedForModuleBuildWithoutOutputs(
   auto DepModuleMapFiles = collectModuleMapFiles(Deps.ClangModuleDeps);
   for (StringRef ModuleMapFile : Deps.ModuleMapFileDeps) {
     // TODO: Track these as `FileEntryRef` to simplify the equality check below.
-    auto ModuleMapEntry = ScanInstance.getFileManager().getFile(ModuleMapFile);
+    auto ModuleMapEntry =
+        ScanInstance.getFileManager().getOptionalFileRef(ModuleMapFile);
     assert(ModuleMapEntry && "module map file entry not found");
 
     // Don't report module maps describing eagerly-loaded dependency. This
@@ -299,7 +300,8 @@ llvm::DenseSet<const FileEntry *> ModuleDepCollector::collectModuleMapFiles(
     ModuleDeps *MD = ModuleDepsByID.lookup(MID);
     assert(MD && "Inconsistent dependency info");
     // TODO: Track ClangModuleMapFile as `FileEntryRef`.
-    auto FE = ScanInstance.getFileManager().getFile(MD->ClangModuleMapFile);
+    auto FE = ScanInstance.getFileManager().getOptionalFileRef(
+        MD->ClangModuleMapFile);
     assert(FE && "Missing module map file that was previously found");
     ModuleMapFiles.insert(*FE);
   }
diff --git a/clang/tools/clang-refactor/ClangRefactor.cpp b/clang/tools/clang-refactor/ClangRefactor.cpp
index 9310263c446aea..968f0594085d40 100644
--- a/clang/tools/clang-refactor/ClangRefactor.cpp
+++ b/clang/tools/clang-refactor/ClangRefactor.cpp
@@ -117,7 +117,7 @@ class SourceRangeSelectionArgument final : public SourceSelectionArgument {
 
   bool forAllRanges(const SourceManager &SM,
                     llvm::function_ref<void(SourceRange R)> Callback) override {
-    auto FE = SM.getFileManager().getFile(Range.FileName);
+    auto FE = SM.getFileManager().getOptionalFileRef(Range.FileName);
     FileID FID = FE ? SM.translateFile(*FE) : FileID();
     if (!FE || FID.isInvalid()) {
       llvm::errs() << "error: -selection=" << Range.FileName
diff --git a/clang/tools/clang-refactor/TestSupport.cpp b/clang/tools/clang-refactor/TestSupport.cpp
index 3fae18c2109a69..8b6e250b3632d0 100644
--- a/clang/tools/clang-refactor/TestSupport.cpp
+++ b/clang/tools/clang-refactor/TestSupport.cpp
@@ -43,7 +43,7 @@ void TestSelectionRangesInFile::dump(raw_ostream &OS) const {
 bool TestSelectionRangesInFile::foreachRange(
     const SourceManager &SM,
     llvm::function_ref<void(SourceRange)> Callback) const {
-  auto FE = SM.getFileManager().getFile(Filename);
+  auto FE = SM.getFileManager().getOptionalFileRef(Filename);
   FileID FID = FE ? SM.translateFile(*FE) : FileID();
   if (!FE || FID.isInvalid()) {
     llvm::errs() << "error: -selection=test:" << Filename
diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp
index d32036d975ce9c..47d7c5725fbdc3 100644
--- a/clang/unittests/Basic/FileManagerTest.cpp
+++ b/clang/unittests/Basic/FileManagerTest.cpp
@@ -190,11 +190,11 @@ TEST_F(FileManagerTest, getFileReturnsDifferentFileEntriesForDifferentFiles) {
   statCache->InjectFile("bar.cpp", 43);
   manager.setStatCache(std::move(statCache));
 
-  auto fileFoo = manager.getFile("foo.cpp");
-  auto fileBar = manager.getFile("bar.cpp");
+  auto fileFoo = manager.getOptionalFileRef("foo.cpp");
+  auto fileBar = manager.getOptionalFileRef("bar.cpp");
   ASSERT_TRUE(fileFoo);
   ASSERT_TRUE(fileBar);
-  EXPECT_NE(*fileFoo, *fileBar);
+  EXPECT_NE(&fileFoo->getFileEntry(), &fileBar->getFileEntry());
 }
 
 // getFile() returns an error if neither a real file nor a virtual file
@@ -210,13 +210,15 @@ TEST_F(FileManagerTest, getFileReturnsErrorForNonexistentFile) {
   // Create a virtual bar.cpp file.
   manager.getVirtualFile("bar.cpp", 200, 0);
 
-  auto file = manager.getFile("xyz.txt");
+  auto file = manager.getFileRef("xyz.txt");
   ASSERT_FALSE(file);
-  ASSERT_EQ(file.getError(), std::errc::no_such_file_or_directory);
+  ASSERT_EQ(llvm::errorToErrorCode(file.takeError()),
+            std::make_error_code(std::errc::no_such_file_or_directory));
 
-  auto readingDirAsFile = manager.getFile("MyDirectory");
+  auto readingDirAsFile = manager.getFileRef("MyDirectory");
   ASSERT_FALSE(readingDirAsFile);
-  ASSERT_EQ(readingDirAsFile.getError(), std::errc::is_a_directory);
+  ASSERT_EQ(llvm::errorToErrorCode(readingDirAsFile.takeError()),
+            std::make_error_code(std::errc::is_a_directory));
 
   auto readingFileAsDir = manager.getDirectory("foo.cpp");
   ASSERT_FALSE(readingFileAsDir);
@@ -236,11 +238,11 @@ TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedRealFiles) {
   statCache->InjectFile("abc/bar.cpp", 42);
   manager.setStatCache(std::move(statCache));
 
-  auto f1 = manager.getFile("abc/foo.cpp");
-  auto f2 = manager.getFile("abc/bar.cpp");
+  auto f1 = manager.getOptionalFileRef("abc/foo.cpp");
+  auto f2 = manager.getOptionalFileRef("abc/bar.cpp");
 
-  EXPECT_EQ(f1 ? *f1 : nullptr,
-            f2 ? *f2 : nullptr);
+  EXPECT_EQ(f1 ? &f1->getFileEntry() : nullptr,
+            f2 ? &f2->getFileEntry() : nullptr);
 
   // Check that getFileRef also does the right thing.
   auto r1 = manager.getFileRef("abc/foo.cpp");
@@ -338,11 +340,11 @@ TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedVirtualFiles) {
   statCache->InjectFile("abc/bar.cpp", 42);
   manager.setStatCache(std::move(statCache));
 
-  auto f1 = manager.getFile("abc/foo.cpp");
-  auto f2 = manager.getFile("abc/bar.cpp");
+  auto f1 = manager.getOptionalFileRef("abc/foo.cpp");
+  auto f2 = manager.getOptionalFileRef("abc/bar.cpp");
 
-  EXPECT_EQ(f1 ? *f1 : nullptr,
-            f2 ? *f2 : nullptr);
+  EXPECT_EQ(f1 ? &f1->getFileEntry() : nullptr,
+            f2 ? &f2->getFileEntry() : nullptr);
 }
 
 TEST_F(FileManagerTest, getFileRefEquality) {
@@ -426,14 +428,14 @@ TEST_F(FileManagerTest, getVirtualFileWithDifferentName) {
   EXPECT_EQ(123, file1->getSize());
 
   // Lookup the virtual file with a different name:
-  auto file2 = manager.getFile("c:/tmp/test", 100, 1);
+  auto file2 = manager.getOptionalFileRef("c:/tmp/test", 100, 1);
   ASSERT_TRUE(file2);
   // Check that it's the same UFE:
   EXPECT_EQ(file1, *file2);
-  EXPECT_EQ(43U, (*file2)->getUniqueID().getFile());
+  EXPECT_EQ(43U, file2->getUniqueID().getFile());
   // Check that the contents of the UFE are not overwritten by the entry in the
   // filesystem:
-  EXPECT_EQ(123, (*file2)->getSize());
+  EXPECT_EQ(123, file2->getSize());
 }
 
 #endif  // !_WIN32
@@ -514,12 +516,12 @@ TEST_F(FileManagerTest, getFileDontOpenRealPath) {
   Manager.setStatCache(std::move(statCache));
 
   // Check for real path.
-  auto file = Manager.getFile("/tmp/test", /*OpenFile=*/false);
+  auto file = Manager.getOptionalFileRef("/tmp/test", /*OpenFile=*/false);
   ASSERT_TRUE(file);
   SmallString<64> ExpectedResult = CustomWorkingDir;
 
   llvm::sys::path::append(ExpectedResult, "tmp", "test");
-  EXPECT_EQ((*file)->tryGetRealPathName(), ExpectedResult);
+  EXPECT_EQ(file->getFileEntry().tryGetRealPathName(), ExpectedResult);
 }
 
 TEST_F(FileManagerTest, getBypassFile) {
diff --git a/clang/unittests/Frontend/CompilerInstanceTest.cpp b/clang/unittests/Frontend/CompilerInstanceTest.cpp
index 8bc705dd219936..5cf548e913cc10 100644
--- a/clang/unittests/Frontend/CompilerInstanceTest.cpp
+++ b/clang/unittests/Frontend/CompilerInstanceTest.cpp
@@ -71,7 +71,7 @@ TEST(CompilerInstance, DefaultVFSOverlayFromInvocation) {
 
   // Check if the virtual file exists which means that our VFS is used by the
   // CompilerInstance.
-  ASSERT_TRUE(Instance.getFileManager().getFile("vfs-virtual.file"));
+  ASSERT_TRUE(Instance.getFileManager().getOptionalFileRef("vfs-virtual.file"));
 }
 
 TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {

>From cd3dc21c10d7aeb965351d29a94d9e5d133c07bd Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 23 Sep 2024 17:47:01 -0700
Subject: [PATCH 2/4] [clang] Formally deprecate
 `FileManager::getVirtualFile()`

---
 .../include/common/VirtualFileHelper.h         |  2 +-
 .../clang/Basic/DiagnosticFrontendKinds.td     |  2 --
 clang/include/clang/Basic/FileManager.h        |  2 ++
 clang/lib/AST/ASTImporter.cpp                  |  4 ++--
 clang/lib/Frontend/CompilerInstance.cpp        |  8 ++------
 clang/lib/Serialization/ModuleManager.cpp      |  4 ++--
 clang/unittests/Basic/FileManagerTest.cpp      | 18 ++++++++----------
 clang/unittests/Basic/SourceManagerTest.cpp    |  2 +-
 8 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/clang-tools-extra/unittests/include/common/VirtualFileHelper.h b/clang-tools-extra/unittests/include/common/VirtualFileHelper.h
index 18b98d2796e679..abe10674956949 100644
--- a/clang-tools-extra/unittests/include/common/VirtualFileHelper.h
+++ b/clang-tools-extra/unittests/include/common/VirtualFileHelper.h
@@ -60,7 +60,7 @@ class VirtualFileHelper {
          I != E; ++I) {
       std::unique_ptr<llvm::MemoryBuffer> Buf =
           llvm::MemoryBuffer::getMemBuffer(I->Code);
-      const FileEntry *Entry = SM.getFileManager().getVirtualFile(
+      FileEntryRef Entry = SM.getFileManager().getVirtualFileRef(
           I->FileName, Buf->getBufferSize(), /*ModificationTime=*/0);
       SM.overrideFileContents(Entry, std::move(Buf));
     }
diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 292e4af1b3b303..a6b17ccb6799d2 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -109,8 +109,6 @@ def err_fe_expected_clang_command : Error<
     "expected a clang compiler command">;
 def err_fe_remap_missing_to_file : Error<
     "could not remap file '%0' to the contents of file '%1'">, DefaultFatal;
-def err_fe_remap_missing_from_file : Error<
-    "could not remap from missing file '%0'">, DefaultFatal;
 def err_fe_unable_to_load_pch : Error<
     "unable to load PCH file">;
 def err_fe_unable_to_load_plugin : Error<
diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h
index 1846feb23716db..29583910fa1871 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -271,6 +271,8 @@ class FileManager : public RefCountedBase<FileManager> {
   FileEntryRef getVirtualFileRef(StringRef Filename, off_t Size,
                                  time_t ModificationTime);
 
+  LLVM_DEPRECATED("Functions returning FileEntry are deprecated.",
+                  "getVirtualFileRef()")
   const FileEntry *getVirtualFile(StringRef Filename, off_t Size,
                                   time_t ModificationTime);
 
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index bba97e289da2e1..60175f1ccb342a 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -10020,8 +10020,8 @@ Expected<FileID> ASTImporter::Import(FileID FromID, bool IsBuiltin) {
         ToIncludeLocOrFakeLoc = ToSM.getLocForStartOfFile(ToSM.getMainFileID());
 
       if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-        // FIXME: We probably want to use getVirtualFile(), so we don't hit the
-        // disk again
+        // FIXME: We probably want to use getVirtualFileRef(), 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 =
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 08905a8debb1b8..0089786379d3ea 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -427,12 +427,8 @@ static void InitializeFileRemapping(DiagnosticsEngine &Diags,
     }
 
     // Create the file entry for the file that we're mapping from.
-    const FileEntry *FromFile =
-        FileMgr.getVirtualFile(RF.first, ToFile->getSize(), 0);
-    if (!FromFile) {
-      Diags.Report(diag::err_fe_remap_missing_from_file) << RF.first;
-      continue;
-    }
+    FileEntryRef FromFile =
+        FileMgr.getVirtualFileRef(RF.first, ToFile->getSize(), 0);
 
     // Override the contents of the "from" file with the contents of
     // the "to" file.
diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp
index fa19f68ae7e367..e74a16b6368028 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/clang/lib/Serialization/ModuleManager.cpp
@@ -279,8 +279,8 @@ void ModuleManager::removeModules(ModuleIterator First) {
 void
 ModuleManager::addInMemoryBuffer(StringRef FileName,
                                  std::unique_ptr<llvm::MemoryBuffer> Buffer) {
-  const FileEntry *Entry =
-      FileMgr.getVirtualFile(FileName, Buffer->getBufferSize(), 0);
+  FileEntryRef Entry =
+      FileMgr.getVirtualFileRef(FileName, Buffer->getBufferSize(), 0);
   InMemoryBuffers[Entry] = std::move(Buffer);
 }
 
diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp
index 47d7c5725fbdc3..1f911227c897df 100644
--- a/clang/unittests/Basic/FileManagerTest.cpp
+++ b/clang/unittests/Basic/FileManagerTest.cpp
@@ -126,7 +126,7 @@ TEST_F(FileManagerTest, getVirtualFileCreatesDirectoryEntriesForAncestors) {
   // Fake an empty real file system.
   manager.setStatCache(std::make_unique<FakeStatCache>());
 
-  manager.getVirtualFile("virtual/dir/bar.h", 100, 0);
+  manager.getVirtualFileRef("virtual/dir/bar.h", 100, 0);
   ASSERT_FALSE(manager.getDirectory("virtual/dir/foo"));
 
   auto dir = manager.getDirectoryRef("virtual/dir");
@@ -172,7 +172,7 @@ TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingVirtualFile) {
   // Fake an empty real file system.
   manager.setStatCache(std::make_unique<FakeStatCache>());
 
-  manager.getVirtualFile("virtual/dir/bar.h", 100, 0);
+  manager.getVirtualFileRef("virtual/dir/bar.h", 100, 0);
   auto file = manager.getFileRef("virtual/dir/bar.h");
   ASSERT_THAT_EXPECTED(file, llvm::Succeeded());
   EXPECT_EQ("virtual/dir/bar.h", file->getName());
@@ -208,7 +208,7 @@ TEST_F(FileManagerTest, getFileReturnsErrorForNonexistentFile) {
   manager.setStatCache(std::move(statCache));
 
   // Create a virtual bar.cpp file.
-  manager.getVirtualFile("bar.cpp", 200, 0);
+  manager.getVirtualFileRef("bar.cpp", 200, 0);
 
   auto file = manager.getFileRef("xyz.txt");
   ASSERT_FALSE(file);
@@ -422,10 +422,9 @@ TEST_F(FileManagerTest, getVirtualFileWithDifferentName) {
   manager.setStatCache(std::move(statCache));
 
   // Inject the virtual file:
-  const FileEntry *file1 = manager.getVirtualFile("c:\\tmp\\test", 123, 1);
-  ASSERT_TRUE(file1 != nullptr);
-  EXPECT_EQ(43U, file1->getUniqueID().getFile());
-  EXPECT_EQ(123, file1->getSize());
+  FileEntryRef file1 = manager.getVirtualFileRef("c:\\tmp\\test", 123, 1);
+  EXPECT_EQ(43U, file1.getUniqueID().getFile());
+  EXPECT_EQ(123, file1.getSize());
 
   // Lookup the virtual file with a different name:
   auto file2 = manager.getOptionalFileRef("c:/tmp/test", 100, 1);
@@ -489,12 +488,11 @@ TEST_F(FileManagerTest, getVirtualFileFillsRealPathName) {
   Manager.setStatCache(std::move(statCache));
 
   // Check for real path.
-  const FileEntry *file = Manager.getVirtualFile("/tmp/test", 123, 1);
-  ASSERT_TRUE(file != nullptr);
+  FileEntryRef file = Manager.getVirtualFileRef("/tmp/test", 123, 1);
   SmallString<64> ExpectedResult = CustomWorkingDir;
 
   llvm::sys::path::append(ExpectedResult, "tmp", "test");
-  EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
+  EXPECT_EQ(file.getFileEntry().tryGetRealPathName(), ExpectedResult);
 }
 
 TEST_F(FileManagerTest, getFileDontOpenRealPath) {
diff --git a/clang/unittests/Basic/SourceManagerTest.cpp b/clang/unittests/Basic/SourceManagerTest.cpp
index 0f2476bd8b0612..2b3fce9128ba99 100644
--- a/clang/unittests/Basic/SourceManagerTest.cpp
+++ b/clang/unittests/Basic/SourceManagerTest.cpp
@@ -549,7 +549,7 @@ TEST_F(SourceManagerTest, getMacroArgExpandedLocation) {
   // These are different than normal includes since predefines buffer doesn't
   // have a valid insertion location.
   PP.setPredefines("#include \"/implicit-header.h\"");
-  FileMgr.getVirtualFile("/implicit-header.h", 0, 0);
+  FileMgr.getVirtualFileRef("/implicit-header.h", 0, 0);
   PP.Initialize(*Target);
   PP.EnterMainSourceFile();
 

>From f3d5c29011110bdb58ee4355733d77eef1891e58 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 23 Sep 2024 18:18:18 -0700
Subject: [PATCH 3/4] [clang] Formally deprecate `FileManager::getDirectory()`

---
 clang/include/clang/Basic/FileManager.h   |  2 ++
 clang/lib/Lex/HeaderSearch.cpp            |  4 ++--
 clang/lib/Lex/ModuleMap.cpp               |  3 ++-
 clang/lib/Lex/PPLexerChange.cpp           |  2 +-
 clang/tools/clang-installapi/Options.cpp  |  2 +-
 clang/unittests/Basic/FileManagerTest.cpp | 17 ++++++++++-------
 6 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h
index 29583910fa1871..29df66860f8f7b 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -190,6 +190,8 @@ class FileManager : public RefCountedBase<FileManager> {
   ///
   /// \param CacheFailure If true and the file does not exist, we'll cache
   /// the failure to find this file.
+  LLVM_DEPRECATED("Functions returning DirectoryEntry are deprecated.",
+                  "getOptionalDirectoryRef()")
   llvm::ErrorOr<const DirectoryEntry *>
   getDirectory(StringRef DirName, bool CacheFailure = true);
 
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 5f7e9533b58c89..8826ab449df493 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -655,7 +655,7 @@ OptionalFileEntryRef DirectoryLookup::DoFrameworkLookup(
     ++NumFrameworkLookups;
 
     // If the framework dir doesn't exist, we fail.
-    auto Dir = FileMgr.getDirectory(FrameworkName);
+    auto Dir = FileMgr.getOptionalDirectoryRef(FrameworkName);
     if (!Dir)
       return std::nullopt;
 
@@ -718,7 +718,7 @@ OptionalFileEntryRef DirectoryLookup::DoFrameworkLookup(
     bool FoundFramework = false;
     do {
       // Determine whether this directory exists.
-      auto Dir = FileMgr.getDirectory(FrameworkPath);
+      auto Dir = FileMgr.getOptionalDirectoryRef(FrameworkPath);
       if (!Dir)
         break;
 
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index eed7eca2e73562..2aada51c71c503 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -1144,7 +1144,8 @@ Module *ModuleMap::inferFrameworkModule(DirectoryEntryRef FrameworkDir,
         if (SubframeworkDirName.empty())
           break;
 
-        if (auto SubDir = FileMgr.getDirectory(SubframeworkDirName)) {
+        if (auto SubDir =
+                FileMgr.getOptionalDirectoryRef(SubframeworkDirName)) {
           if (*SubDir == FrameworkDir) {
             FoundParent = true;
             break;
diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp
index 8221db46e06acc..1a71f03b182367 100644
--- a/clang/lib/Lex/PPLexerChange.cpp
+++ b/clang/lib/Lex/PPLexerChange.cpp
@@ -229,7 +229,7 @@ static void computeRelativePath(FileManager &FM, const DirectoryEntry *Dir,
   StringRef FilePath = File.getDir().getName();
   StringRef Path = FilePath;
   while (!Path.empty()) {
-    if (auto CurDir = FM.getDirectory(Path)) {
+    if (auto CurDir = FM.getOptionalDirectoryRef(Path)) {
       if (*CurDir == Dir) {
         Result = FilePath.substr(Path.size());
         llvm::sys::path::append(Result,
diff --git a/clang/tools/clang-installapi/Options.cpp b/clang/tools/clang-installapi/Options.cpp
index 1ca1d583d5ccdf..3fa79636de5d75 100644
--- a/clang/tools/clang-installapi/Options.cpp
+++ b/clang/tools/clang-installapi/Options.cpp
@@ -554,7 +554,7 @@ bool Options::processFrontendOptions(InputArgList &Args) {
 bool Options::addFilePaths(InputArgList &Args, PathSeq &Headers,
                            OptSpecifier ID) {
   for (const StringRef Path : Args.getAllArgValues(ID)) {
-    if ((bool)FM->getDirectory(Path, /*CacheFailure=*/false)) {
+    if ((bool)FM->getOptionalDirectoryRef(Path, /*CacheFailure=*/false)) {
       auto InputHeadersOrErr = enumerateFiles(*FM, Path);
       if (!InputHeadersOrErr) {
         Diags->Report(diag::err_cannot_open_file)
diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp
index 1f911227c897df..53897322f61609 100644
--- a/clang/unittests/Basic/FileManagerTest.cpp
+++ b/clang/unittests/Basic/FileManagerTest.cpp
@@ -116,9 +116,9 @@ TEST_F(FileManagerTest, NoVirtualDirectoryExistsBeforeAVirtualFileIsAdded) {
   // by what's in the real file system.
   manager.setStatCache(std::make_unique<FakeStatCache>());
 
-  ASSERT_FALSE(manager.getDirectory("virtual/dir/foo"));
-  ASSERT_FALSE(manager.getDirectory("virtual/dir"));
-  ASSERT_FALSE(manager.getDirectory("virtual"));
+  ASSERT_FALSE(manager.getOptionalDirectoryRef("virtual/dir/foo"));
+  ASSERT_FALSE(manager.getOptionalDirectoryRef("virtual/dir"));
+  ASSERT_FALSE(manager.getOptionalDirectoryRef("virtual"));
 }
 
 // When a virtual file is added, all of its ancestors should be created.
@@ -127,9 +127,11 @@ TEST_F(FileManagerTest, getVirtualFileCreatesDirectoryEntriesForAncestors) {
   manager.setStatCache(std::make_unique<FakeStatCache>());
 
   manager.getVirtualFileRef("virtual/dir/bar.h", 100, 0);
-  ASSERT_FALSE(manager.getDirectory("virtual/dir/foo"));
 
-  auto dir = manager.getDirectoryRef("virtual/dir");
+  auto dir = manager.getDirectoryRef("virtual/dir/foo");
+  ASSERT_THAT_EXPECTED(dir, llvm::Failed());
+
+  dir = manager.getDirectoryRef("virtual/dir");
   ASSERT_THAT_EXPECTED(dir, llvm::Succeeded());
   EXPECT_EQ("virtual/dir", dir->getName());
 
@@ -220,9 +222,10 @@ TEST_F(FileManagerTest, getFileReturnsErrorForNonexistentFile) {
   ASSERT_EQ(llvm::errorToErrorCode(readingDirAsFile.takeError()),
             std::make_error_code(std::errc::is_a_directory));
 
-  auto readingFileAsDir = manager.getDirectory("foo.cpp");
+  auto readingFileAsDir = manager.getDirectoryRef("foo.cpp");
   ASSERT_FALSE(readingFileAsDir);
-  ASSERT_EQ(readingFileAsDir.getError(), std::errc::not_a_directory);
+  ASSERT_EQ(llvm::errorToErrorCode(readingFileAsDir.takeError()),
+            std::make_error_code(std::errc::not_a_directory));
 }
 
 // The following tests apply to Unix-like system only.

>From 269ec5c44424ec01a7fca3c084156da851deaa5a Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 23 Sep 2024 18:20:55 -0700
Subject: [PATCH 4/4] [clang] Fix formatting in `FileManager.h`

---
 clang/include/clang/Basic/FileManager.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h
index 29df66860f8f7b..ce4e8c1fbe16eb 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -84,7 +84,7 @@ class FileManager : public RefCountedBase<FileManager> {
   /// VirtualDirectoryEntries/VirtualFileEntries above.
   ///
   llvm::StringMap<llvm::ErrorOr<DirectoryEntry &>, llvm::BumpPtrAllocator>
-  SeenDirEntries;
+      SeenDirEntries;
 
   /// A cache that maps paths to file entries (either real or
   /// virtual) we have looked up, or an error that occurred when we looked up



More information about the cfe-commits mailing list