[clang][tools] Use FileEntryRef in include_cleaner::Header

jansvoboda11 · jansvoboda11 · commit 98e6deb62cef · 2023-09-09T09:54:13.000-07:00
diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -101,7 +101,7 @@ bool IncludeCleanerCheck::shouldIgnore(const include_cleaner::Header &H) {
     case include_cleaner::Header::Verbatim:
       return R.match(H.verbatim());
     case include_cleaner::Header::Physical:
-      return R.match(H.physical()->tryGetRealPathName());
+      return R.match(H.physical().getFileEntry().tryGetRealPathName());
     }
     llvm_unreachable("Unknown Header kind.");
   });
@@ -145,7 +145,7 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
              for (const include_cleaner::Header &H : Providers) {
                if (H.kind() == include_cleaner::Header::Physical &&
                    (H.physical() == MainFile ||
-                    H.physical()->getDir() == ResourceDir)) {
+                    H.physical().getDir() == ResourceDir)) {
                  Satisfied = true;
                  continue;
                }
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -410,7 +410,7 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
         for (const auto &H : Providers) {
           if (H.kind() == include_cleaner::Header::Physical &&
               (H.physical() == MainFile || H.physical() == PreamblePatch ||
-               H.physical()->getLastRef().getDir() == ResourceDir)) {
+               H.physical().getDir() == ResourceDir)) {
             Satisfied = true;
             continue;
           }
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -167,7 +167,8 @@ TEST(IncludeCleaner, ComputeMissingHeaders) {
   size_t End = llvm::cantFail(positionToOffset(MainFile.code(), Range.end));
   syntax::FileRange BRange{SM.getMainFileID(), static_cast<unsigned int>(Start),
                            static_cast<unsigned int>(End)};
-  include_cleaner::Header Header{*SM.getFileManager().getFile("b.h")};
+  include_cleaner::Header Header{
+      *SM.getFileManager().getOptionalFileRef("b.h")};
   MissingIncludeDiagInfo BInfo{B, BRange, {Header}};
   EXPECT_THAT(Findings.MissingIncludes, ElementsAre(BInfo));
 }
@@ -474,8 +475,8 @@ TEST(IncludeCleaner, IsPreferredProvider) {
   auto &IncludeDef2 = AST.getIncludeStructure().MainFileIncludes[2];
 
   auto &FM = AST.getSourceManager().getFileManager();
-  auto *DeclH = &FM.getOptionalFileRef("decl.h")->getFileEntry();
-  auto *DefH = &FM.getOptionalFileRef("def.h")->getFileEntry();
+  auto DeclH = *FM.getOptionalFileRef("decl.h");
+  auto DefH = *FM.getOptionalFileRef("def.h");
 
   auto Includes = convertIncludes(AST);
   std::vector<include_cleaner::Header> Providers = {
diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
@@ -67,10 +67,10 @@ class PragmaIncludes {
 
   /// Returns all direct exporter headers for the given header file.
   /// Returns empty if there is none.
-  llvm::SmallVector<const FileEntry *> getExporters(const FileEntry *File,
-                                                    FileManager &FM) const;
-  llvm::SmallVector<const FileEntry *> getExporters(tooling::stdlib::Header,
-                                                    FileManager &FM) const;
+  llvm::SmallVector<FileEntryRef> getExporters(const FileEntry *File,
+                                               FileManager &FM) const;
+  llvm::SmallVector<FileEntryRef> getExporters(tooling::stdlib::Header,
+                                               FileManager &FM) const;
 
   /// Returns true if the given file is a self-contained file.
   bool isSelfContained(const FileEntry *File) const;
diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
@@ -122,15 +122,15 @@ struct Header {
     Verbatim,
   };
 
-  Header(const FileEntry *FE) : Storage(FE) {}
+  Header(FileEntryRef FE) : Storage(FE) {}
   Header(tooling::stdlib::Header H) : Storage(H) {}
   Header(StringRef VerbatimSpelling) : Storage(VerbatimSpelling) {}
 
   Kind kind() const { return static_cast<Kind>(Storage.index()); }
   bool operator==(const Header &RHS) const { return Storage == RHS.Storage; }
   bool operator<(const Header &RHS) const;
 
-  const FileEntry *physical() const { return std::get<Physical>(Storage); }
+  FileEntryRef physical() const { return std::get<Physical>(Storage); }
   tooling::stdlib::Header standard() const {
     return std::get<Standard>(Storage);
   }
@@ -142,7 +142,7 @@ struct Header {
 
 private:
   // Order must match Kind enum!
-  std::variant<const FileEntry *, tooling::stdlib::Header, StringRef> Storage;
+  std::variant<FileEntryRef, tooling::stdlib::Header, StringRef> Storage;
 
   // Disambiguation tag to make sure we can call the right constructor from
   // DenseMapInfo methods.
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -95,7 +95,7 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
              for (const Header &H : Providers) {
                if (H.kind() == Header::Physical &&
                    (H.physical() == MainFile ||
-                    H.physical()->getDir() == ResourceDir)) {
+                    H.physical().getDir() == ResourceDir)) {
                  Satisfied = true;
                }
                for (const Include *I : Inc.match(H)) {
diff --git a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -63,7 +63,7 @@ llvm::StringRef basename(llvm::StringRef Header) {
 bool nameMatch(llvm::StringRef DeclName, Header H) {
   switch (H.kind()) {
   case Header::Physical:
-    return basename(H.physical()->getName()).equals_insensitive(DeclName);
+    return basename(H.physical().getName()).equals_insensitive(DeclName);
   case Header::Standard:
     return basename(H.standard().name()).equals_insensitive(DeclName);
   case Header::Verbatim:
@@ -101,7 +101,7 @@ hintedHeadersForStdHeaders(llvm::ArrayRef<tooling::stdlib::Header> Headers,
     Results.emplace_back(H, Hints::PublicHeader | Hints::OriginHeader);
     if (!PI)
       continue;
-    for (const auto *Export : PI->getExporters(H, SM.getFileManager()))
+    for (FileEntryRef Export : PI->getExporters(H, SM.getFileManager()))
       Results.emplace_back(Header(Export), isPublicHeader(Export, *PI));
   }
   // StandardLibrary returns headers in preference order, so only mark the
@@ -186,31 +186,31 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const SymbolLocation &Loc,
   switch (Loc.kind()) {
   case SymbolLocation::Physical: {
     FileID FID = SM.getFileID(SM.getExpansionLoc(Loc.physical()));
-    const FileEntry *FE = SM.getFileEntryForID(FID);
+    OptionalFileEntryRef FE = SM.getFileEntryRefForID(FID);
     if (!FE)
       return {};
     if (!PI)
-      return {{FE, Hints::PublicHeader | Hints::OriginHeader}};
+      return {{*FE, Hints::PublicHeader | Hints::OriginHeader}};
     bool IsOrigin = true;
-    std::queue<const FileEntry *> Exporters;
+    std::queue<FileEntryRef> Exporters;
     while (FE) {
-      Results.emplace_back(FE,
-                           isPublicHeader(FE, *PI) |
+      Results.emplace_back(*FE,
+                           isPublicHeader(*FE, *PI) |
                                (IsOrigin ? Hints::OriginHeader : Hints::None));
-      for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
+      for (FileEntryRef Export : PI->getExporters(*FE, SM.getFileManager()))
         Exporters.push(Export);
 
-      if (auto Verbatim = PI->getPublic(FE); !Verbatim.empty()) {
+      if (auto Verbatim = PI->getPublic(*FE); !Verbatim.empty()) {
         Results.emplace_back(Verbatim,
                              Hints::PublicHeader | Hints::PreferredHeader);
         break;
       }
-      if (PI->isSelfContained(FE) || FID == SM.getMainFileID())
+      if (PI->isSelfContained(*FE) || FID == SM.getMainFileID())
         break;
 
       // Walkup the include stack for non self-contained headers.
       FID = SM.getDecomposedIncludedLoc(FID).first;
-      FE = SM.getFileEntryForID(FID);
+      FE = SM.getFileEntryRefForID(FID);
       IsOrigin = false;
     }
     // Now traverse provider trees rooted at exporters.
@@ -219,12 +219,12 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const SymbolLocation &Loc,
     // being exported in this header.
     std::set<const FileEntry *> SeenExports;
     while (!Exporters.empty()) {
-      auto *Export = Exporters.front();
+      FileEntryRef Export = Exporters.front();
       Exporters.pop();
       if (!SeenExports.insert(Export).second) // In case of cyclic exports
         continue;
       Results.emplace_back(Export, isPublicHeader(Export, *PI));
-      for (const auto *Export : PI->getExporters(Export, SM.getFileManager()))
+      for (FileEntryRef Export : PI->getExporters(Export, SM.getFileManager()))
         Exporters.push(Export);
     }
     return Results;
diff --git a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -390,7 +390,7 @@ class Reporter {
       OS << "<tr><th>Header</th><td>";
       switch (H.kind()) {
       case Header::Physical:
-        printFilename(H.physical()->getName());
+        printFilename(H.physical().getName());
         break;
       case Header::Standard:
         OS << "stdlib " << H.standard().name();
diff --git a/clang-tools-extra/include-cleaner/lib/Record.cpp b/clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -224,7 +224,7 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler {
         IncludedHeader = *StandardHeader;
       }
     if (!IncludedHeader && File)
-      IncludedHeader = &File->getFileEntry();
+      IncludedHeader = *File;
     checkForExport(HashFID, HashLine, std::move(IncludedHeader), File);
     checkForKeep(HashLine, File);
   }
@@ -243,7 +243,7 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler {
       if (IncludedHeader) {
         switch (IncludedHeader->kind()) {
         case Header::Physical:
-          Out->IWYUExportBy[IncludedHeader->physical()->getUniqueID()]
+          Out->IWYUExportBy[IncludedHeader->physical().getUniqueID()]
               .push_back(Top.Path);
           break;
         case Header::Standard:
@@ -393,26 +393,26 @@ llvm::StringRef PragmaIncludes::getPublic(const FileEntry *F) const {
   return It->getSecond();
 }
 
-static llvm::SmallVector<const FileEntry *>
+static llvm::SmallVector<FileEntryRef>
 toFileEntries(llvm::ArrayRef<StringRef> FileNames, FileManager &FM) {
-  llvm::SmallVector<const FileEntry *> Results;
+  llvm::SmallVector<FileEntryRef> Results;
 
   for (auto FName : FileNames) {
     // FIMXE: log the failing cases?
-    if (auto FE = expectedToOptional(FM.getFileRef(FName)))
+    if (auto FE = FM.getOptionalFileRef(FName))
       Results.push_back(*FE);
   }
   return Results;
 }
-llvm::SmallVector<const FileEntry *>
+llvm::SmallVector<FileEntryRef>
 PragmaIncludes::getExporters(const FileEntry *File, FileManager &FM) const {
   auto It = IWYUExportBy.find(File->getUniqueID());
   if (It == IWYUExportBy.end())
     return {};
 
   return toFileEntries(It->getSecond(), FM);
 }
-llvm::SmallVector<const FileEntry *>
+llvm::SmallVector<FileEntryRef>
 PragmaIncludes::getExporters(tooling::stdlib::Header StdHeader,
                              FileManager &FM) const {
   auto It = StdIWYUExportBy.find(StdHeader);
diff --git a/clang-tools-extra/include-cleaner/lib/Types.cpp b/clang-tools-extra/include-cleaner/lib/Types.cpp
@@ -48,7 +48,7 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S) {
 llvm::StringRef Header::resolvedPath() const {
   switch (kind()) {
   case include_cleaner::Header::Physical:
-    return physical()->tryGetRealPathName();
+    return physical().getFileEntry().tryGetRealPathName();
   case include_cleaner::Header::Standard:
     return standard().name().trim("<>\"");
   case include_cleaner::Header::Verbatim:
@@ -60,7 +60,7 @@ llvm::StringRef Header::resolvedPath() const {
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Header &H) {
   switch (H.kind()) {
   case Header::Physical:
-    return OS << H.physical()->getName();
+    return OS << H.physical().getName();
   case Header::Standard:
     return OS << H.standard().name();
   case Header::Verbatim:
@@ -198,7 +198,7 @@ bool Header::operator<(const Header &RHS) const {
     return kind() < RHS.kind();
   switch (kind()) {
   case Header::Physical:
-    return physical()->getName() < RHS.physical()->getName();
+    return physical().getName() < RHS.physical().getName();
   case Header::Standard:
     return standard().name() < RHS.standard().name();
   case Header::Verbatim:
diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -112,10 +112,10 @@ TEST_F(WalkUsedTest, Basic) {
 
   TestAST AST(Inputs);
   auto &SM = AST.sourceManager();
-  auto HeaderFile = Header(AST.fileManager().getFile("header.h").get());
-  auto PrivateFile = Header(AST.fileManager().getFile("private.h").get());
+  auto HeaderFile = Header(*AST.fileManager().getOptionalFileRef("header.h"));
+  auto PrivateFile = Header(*AST.fileManager().getOptionalFileRef("private.h"));
   auto PublicFile = Header("\"path/public.h\"");
-  auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+  auto MainFile = Header(*SM.getFileEntryRefForID(SM.getMainFileID()));
   auto VectorSTL = Header(*tooling::stdlib::Header::named("<vector>"));
   auto UtilitySTL = Header(*tooling::stdlib::Header::named("<utility>"));
   EXPECT_THAT(
@@ -152,9 +152,9 @@ TEST_F(WalkUsedTest, MultipleProviders) {
 
   TestAST AST(Inputs);
   auto &SM = AST.sourceManager();
-  auto HeaderFile1 = Header(AST.fileManager().getFile("header1.h").get());
-  auto HeaderFile2 = Header(AST.fileManager().getFile("header2.h").get());
-  auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+  auto HeaderFile1 = Header(*AST.fileManager().getOptionalFileRef("header1.h"));
+  auto HeaderFile2 = Header(*AST.fileManager().getOptionalFileRef("header2.h"));
+  auto MainFile = Header(*SM.getFileEntryRefForID(SM.getMainFileID()));
   EXPECT_THAT(
       offsetToProviders(AST),
       Contains(Pair(Code.point("foo"),
@@ -173,8 +173,8 @@ TEST_F(WalkUsedTest, MacroRefs) {
   TestAST AST(Inputs);
   auto &SM = AST.sourceManager();
   auto &PP = AST.preprocessor();
-  const auto *HdrFile = SM.getFileManager().getFile("hdr.h").get();
-  auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+  auto HdrFile = *SM.getFileManager().getOptionalFileRef("hdr.h");
+  auto MainFile = Header(*SM.getFileEntryRefForID(SM.getMainFileID()));
 
   auto HdrID = SM.translateFile(HdrFile);
 
@@ -490,9 +490,9 @@ TEST_F(WalkUsedTest, TemplateDecls) {
       guard("template<typename T> struct Foo<T*> {};");
   TestAST AST(Inputs);
   auto &SM = AST.sourceManager();
-  const auto *Fwd = SM.getFileManager().getFile("fwd.h").get();
-  const auto *Def = SM.getFileManager().getFile("def.h").get();
-  const auto *Partial = SM.getFileManager().getFile("partial.h").get();
+  auto Fwd = *SM.getFileManager().getOptionalFileRef("fwd.h");
+  auto Def = *SM.getFileManager().getOptionalFileRef("def.h");
+  auto Partial = *SM.getFileManager().getOptionalFileRef("partial.h");
 
   EXPECT_THAT(
       offsetToProviders(AST),
@@ -524,7 +524,7 @@ TEST_F(WalkUsedTest, IgnoresIdentityMacros) {
 
   TestAST AST(Inputs);
   auto &SM = AST.sourceManager();
-  auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+  auto MainFile = Header(*SM.getFileEntryRefForID(SM.getMainFileID()));
   EXPECT_THAT(offsetToProviders(AST),
               UnorderedElementsAre(
                   // FIXME: we should have a reference from stdin to header.h
diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -63,8 +63,8 @@ class FindHeadersTest : public testing::Test {
             /*Line=*/1, /*Col=*/1),
         AST->sourceManager(), &PI);
   }
-  const FileEntry *physicalHeader(llvm::StringRef FileName) {
-    return AST->fileManager().getFile(FileName).get();
+  FileEntryRef physicalHeader(llvm::StringRef FileName) {
+    return *AST->fileManager().getOptionalFileRef(FileName);
   };
 };
 
@@ -409,9 +409,10 @@ TEST_F(HeadersForSymbolTest, MainFile) {
   buildAST();
   auto &SM = AST->sourceManager();
   // FIXME: Symbols provided by main file should be treated specially.
-  EXPECT_THAT(headersForFoo(),
-              ElementsAre(physicalHeader("public_complete.h"),
-                          Header(SM.getFileEntryForID(SM.getMainFileID()))));
+  EXPECT_THAT(
+      headersForFoo(),
+      ElementsAre(physicalHeader("public_complete.h"),
+                  Header(*SM.getFileEntryRefForID(SM.getMainFileID()))));
 }
 
 TEST_F(HeadersForSymbolTest, PreferExporterOfPrivate) {
diff --git a/clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp b/clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp
@@ -47,7 +47,8 @@ class DummyIncludeSpeller : public IncludeSpeller {
       return "<bits/stdc++.h>";
     if (Input.H.kind() != Header::Physical)
       return "";
-    llvm::StringRef AbsolutePath = Input.H.physical()->tryGetRealPathName();
+    llvm::StringRef AbsolutePath =
+        Input.H.physical().getFileEntry().tryGetRealPathName();
     std::string RootWithSeparator{testRoot()};
     RootWithSeparator += llvm::sys::path::get_separator();
     if (!AbsolutePath.consume_front(llvm::StringRef{RootWithSeparator}))
@@ -70,10 +71,12 @@ TEST(IncludeSpeller, IsRelativeToTestRoot) {
   const auto *MainFile = AST.sourceManager().getFileEntryForID(
       AST.sourceManager().getMainFileID());
 
-  EXPECT_EQ("\"foo.h\"", spellHeader({Header{*FM.getFile(testPath("foo.h"))},
-                                      HS, MainFile}));
+  EXPECT_EQ("\"foo.h\"",
+            spellHeader({Header{*FM.getOptionalFileRef(testPath("foo.h"))}, HS,
+                         MainFile}));
   EXPECT_EQ("<header.h>",
-            spellHeader({Header{*FM.getFile("dir/header.h")}, HS, MainFile}));
+            spellHeader({Header{*FM.getOptionalFileRef("dir/header.h")}, HS,
+                         MainFile}));
 }
 
 TEST(IncludeSpeller, CanOverrideSystemHeaders) {
diff --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -53,9 +53,9 @@ MATCHER_P(named, N, "") {
 }
 
 MATCHER_P(FileNamed, N, "") {
-  if (arg->tryGetRealPathName() == N)
+  if (arg.getFileEntry().tryGetRealPathName() == N)
     return true;
-  *result_listener << arg->tryGetRealPathName().str();
+  *result_listener << arg.getFileEntry().tryGetRealPathName().str();
   return false;
 }
 
diff --git a/clang-tools-extra/include-cleaner/unittests/TypesTest.cpp b/clang-tools-extra/include-cleaner/unittests/TypesTest.cpp
@@ -40,9 +40,8 @@ TEST(RecordedIncludesTest, Match) {
   Inc.add(Include{"vector", B, SourceLocation(), 5});
   Inc.add(Include{"missing", std::nullopt, SourceLocation(), 6});
 
-  EXPECT_THAT(Inc.match(&A.getFileEntry()), ElementsAre(line(1), line(2)));
-  EXPECT_THAT(Inc.match(&B.getFileEntry()),
-              ElementsAre(line(3), line(4), line(5)));
+  EXPECT_THAT(Inc.match(A), ElementsAre(line(1), line(2)));
+  EXPECT_THAT(Inc.match(B), ElementsAre(line(3), line(4), line(5)));
   EXPECT_THAT(Inc.match(*tooling::stdlib::Header::named("<vector>")),
               ElementsAre(line(4), line(5)));
 }

Original file line number	Diff line number	Diff line change
`@@ -410,7 +410,7 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {`
`410`	`410`	`for (const auto &H : Providers) {`
`411`	`411`	`if (H.kind() == include_cleaner::Header::Physical &&`
`412`	`412`	`(H.physical() == MainFile \|\| H.physical() == PreamblePatch \|\|`
`413`		`- H.physical()->getLastRef().getDir() == ResourceDir)) {`
	`413`	`+ H.physical().getDir() == ResourceDir)) {`
`414`	`414`	`Satisfied = true;`
`415`	`415`	`continue;`
`416`	`416`	`}`
Original file line number	Diff line number	Diff line change
`@@ -95,7 +95,7 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,`
`95`	`95`	`for (const Header &H : Providers) {`
`96`	`96`	`if (H.kind() == Header::Physical &&`
`97`	`97`	`(H.physical() == MainFile \|\|`
`98`		`- H.physical()->getDir() == ResourceDir)) {`
	`98`	`+ H.physical().getDir() == ResourceDir)) {`
`99`	`99`	`Satisfied = true;`
`100`	`100`	`}`
`101`	`101`	`for (const Include *I : Inc.match(H)) {`