[clang] Make deprecations of some FileManager APIs formal (#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.

Author: jansvoboda11

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);

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

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));

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);
   }

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

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));
     }

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<

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
@@ -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);
 
@@ -207,6 +209,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);
 
@@ -269,6 +273,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);
 

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 =

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.