[include-cleaner] Move RecordedPP::RecordedIncludes -> Includes in Types.h. NFC

[sammccall]

Summary

Requiring everything that wants to match Includes to depend on Record is weird.
This isn't lightweight enough that it feels perfect in Types, could be its own
header instead. But pragmatically it doesn't add bad deps, and is widely used.

clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h

 // but other ways are possible (for example clangd records includes separately).
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef CLANG_INCLUDE_CLEANER_RECORD_H
 #define CLANG_INCLUDE_CLEANER_RECORD_H
 
 #include "clang-include-cleaner/Types.h"
-#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/FileSystem/UniqueID.h"
 #include <memory>
 #include <vector>
 
 namespace clang {
 class ASTConsumer;

 /// when they occur in the main file (e.g. IWYU pragmas).
 struct RecordedPP {
   /// The callback (when installed into clang) tracks macros/includes in this.
   std::unique_ptr<PPCallbacks> record(const Preprocessor &PP);
 
   /// Describes where macros were used in the main file.
   std::vector<SymbolReference> MacroReferences;
 
-  /// A container for all includes present in the main file.
-  /// Supports efficiently hit-testing Headers against Includes.
-  /// FIXME: is there a more natural header for this class?
-  class RecordedIncludes {
-  public:
-    void add(const Include &);
-
-    /// All #includes seen, in the order they appear.
-    llvm::ArrayRef<Include> all() const { return All; }
-
-    /// Determine #includes that match a header (that provides a used symbol).
-    ///
-    /// Matching is based on the type of Header specified:
-    ///  - for a physical file like /path/to/foo.h, we check Resolved
-    ///  - for a logical file like <vector>, we check Spelled
-    llvm::SmallVector<const Include *> match(Header H) const;
-
-    /// Finds the include written on the specified line.
-    const Include *atLine(unsigned OneBasedIndex) const;
-
-  private:
-    std::vector<Include> All;
-    // Lookup structures for match(), values are index into All.
-    llvm::StringMap<llvm::SmallVector<unsigned>> BySpelling;
-    llvm::DenseMap<const FileEntry *, llvm::SmallVector<unsigned>> ByFile;
-    llvm::DenseMap<unsigned, unsigned> ByLine;
-  } Includes;
+  /// The include directives seen in the main file.
+  include_cleaner::Includes Includes;
 };
 
 } // namespace include_cleaner
 } // namespace clang
 
 #endif

clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef CLANG_INCLUDE_CLEANER_TYPES_H
 #define CLANG_INCLUDE_CLEANER_TYPES_H
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 #include <memory>
 #include <vector>
 
 namespace llvm {
 class raw_ostream;
 } // namespace llvm
 namespace clang {
 class Decl;
const FileEntry *Resolved = nullptr; // e.g. /path/to/c++/v1/vector
                                        // nullptr if the header was not found
   SourceLocation HashLocation;         // of hash in #include <vector>
   unsigned Line = 0;                   // 1-based line number for #include
   bool Angled = false;                 // True if spelled with <angle> quotes.
   std::string quote() const;           // e.g. <vector>
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Include &);
 
+/// A container for all includes present in a file.
+/// Supports efficiently hit-testing Headers against Includes.
+class Includes {
+public:
+  void add(const Include &);
+
+  /// All #includes seen, in the order they appear.
+  llvm::ArrayRef<Include> all() const { return All; }
+
+  /// Determine #includes that match a header (that provides a used symbol).
+  ///
+  /// Matching is based on the type of Header specified:
+  ///  - for a physical file like /path/to/foo.h, we check Resolved
+  ///  - for a logical file like <vector>, we check Spelled
+  llvm::SmallVector<const Include *> match(Header H) const;
+
+  /// Finds the include written on the specified line.
+  const Include *atLine(unsigned OneBasedIndex) const;
+
+private:
+  std::vector<Include> All;
+  // Lookup structures for match(), values are index into All.
+  llvm::StringMap<llvm::SmallVector<unsigned>> BySpelling;
+  llvm::DenseMap<const FileEntry *, llvm::SmallVector<unsigned>> ByFile;
+  llvm::DenseMap<unsigned, unsigned> ByLine;
+};
+
 } // namespace include_cleaner
 } // namespace clang
 
 namespace llvm {
 
 template <> struct DenseMapInfo<clang::include_cleaner::Symbol> {
   using Outer = clang::include_cleaner::Symbol;
   using Base = DenseMapInfo<decltype(Outer::Storage)>;

clang-tools-extra/include-cleaner/lib/AnalysisInternal.h

 
 /// Finds the headers that provide the symbol location.
 // FIXME: expose signals
 llvm::SmallVector<Header> findHeaders(const SymbolLocation &Loc,
                                       const SourceManager &SM,
                                       const PragmaIncludes *PI);
 
 /// Write an HTML summary of the analysis to the given stream.
-void writeHTMLReport(FileID File, const RecordedPP::RecordedIncludes &Includes,
+void writeHTMLReport(FileID File, const Includes &,
                      llvm::ArrayRef<Decl *> Roots,
                      llvm::ArrayRef<SymbolReference> MacroRefs, ASTContext &Ctx,
                      HeaderSearch &HS, PragmaIncludes *PI,
                      llvm::raw_ostream &OS);
 
 } // namespace include_cleaner
 } // namespace clang
 
 #endif

clang-tools-extra/include-cleaner/lib/HTMLReport.cpp

llvm::StringRef refType(RefType T) {
   llvm_unreachable("unhandled RefType enum");
 }
 
 class Reporter {
   llvm::raw_ostream &OS;
   const ASTContext &Ctx;
   const SourceManager &SM;
   HeaderSearch &HS;
-  const RecordedPP::RecordedIncludes &Includes;
+  const include_cleaner::Includes &Includes;
   const PragmaIncludes *PI;
   FileID MainFile;
   const FileEntry *MainFE;
 
   // Points within the main file that reference a Symbol.
   // Implicit refs will be marked with a symbol just before the token.
   struct Ref {
     unsigned Offset;
void fillTarget(Ref &R) {
 
     if (!R.Headers.empty())
       // FIXME: library should tell us which header to use.
       R.Insert = spellHeader(R.Headers.front());
   }
 
 public:
   Reporter(llvm::raw_ostream &OS, ASTContext &Ctx, HeaderSearch &HS,
-           const RecordedPP::RecordedIncludes &Includes,
+           const include_cleaner::Includes &Includes,
            const PragmaIncludes *PI, FileID MainFile)
       : OS(OS), Ctx(Ctx), SM(Ctx.getSourceManager()), HS(HS),
         Includes(Includes), PI(PI), MainFile(MainFile),
         MainFE(SM.getFileEntryForID(MainFile)) {}
 
   void addRef(const SymbolReference &SR) {
     auto [File, Offset] = SM.getDecomposedLoc(SM.getFileLoc(SR.RefLocation));
     if (File != this->MainFile) {
void writeCode() {
     }
     EndLine();
     OS << "</pre>\n";
   }
 };
 
 } // namespace
 
-void writeHTMLReport(FileID File, const RecordedPP::RecordedIncludes &Includes,
+void writeHTMLReport(FileID File, const include_cleaner::Includes &Includes,
                      llvm::ArrayRef<Decl *> Roots,
                      llvm::ArrayRef<SymbolReference> MacroRefs, ASTContext &Ctx,
                      HeaderSearch &HS, PragmaIncludes *PI,
                      llvm::raw_ostream &OS) {
   Reporter R(OS, Ctx, HS, Includes, PI, File);
   for (Decl *Root : Roots)
     walkAST(*Root, [&](SourceLocation Loc, const NamedDecl &D, RefType T) {
       R.addRef(SymbolReference{Loc, D, T});
     });
   for (const SymbolReference &Ref : MacroRefs)
     R.addRef(Ref);
   R.write();
 }
 
 } // namespace clang::include_cleaner

clang-tools-extra/include-cleaner/lib/Record.cpp

bool HandleTopLevelDecl(DeclGroupRef DG) override {
       }
       return ASTConsumer::HandleTopLevelDecl(DG);
     }
   };
 
   return std::make_unique<Recorder>(this);
 }
 
-void RecordedPP::RecordedIncludes::add(const Include &I) {
-  unsigned Index = All.size();
-  All.push_back(I);
-  auto BySpellingIt = BySpelling.try_emplace(I.Spelled).first;
-  All.back().Spelled = BySpellingIt->first(); // Now we own the backing string.
-
-  BySpellingIt->second.push_back(Index);
-  if (I.Resolved)
-    ByFile[I.Resolved].push_back(Index);
-  ByLine[I.Line] = Index;
-}
-
-const Include *
-RecordedPP::RecordedIncludes::atLine(unsigned OneBasedIndex) const {
-  auto It = ByLine.find(OneBasedIndex);
-  return (It == ByLine.end()) ? nullptr : &All[It->second];
-}
-
-llvm::SmallVector<const Include *>
-RecordedPP::RecordedIncludes::match(Header H) const {
-  llvm::SmallVector<const Include *> Result;
-  switch (H.kind()) {
-  case Header::Physical:
-    for (unsigned I : ByFile.lookup(H.physical()))
-      Result.push_back(&All[I]);
-    break;
-  case Header::Standard:
-    for (unsigned I : BySpelling.lookup(H.standard().name().trim("<>")))
-      Result.push_back(&All[I]);
-    break;
-  case Header::Verbatim:
-    for (unsigned I : BySpelling.lookup(H.verbatim().trim("\"<>")))
-      Result.push_back(&All[I]);
-    break;
-  }
-  return Result;
-}
-
 std::unique_ptr<PPCallbacks> RecordedPP::record(const Preprocessor &PP) {
   return std::make_unique<PPRecorder>(*this, PP);
 }
 
 } // namespace clang::include_cleaner

clang-tools-extra/include-cleaner/lib/Types.cpp

llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, RefType T) {
   llvm_unreachable("Unexpected RefType");
 }
 
 std::string Include::quote() const {
   return (llvm::StringRef(Angled ? "<" : "\"") + Spelled +
           (Angled ? ">" : "\""))
       .str();
 }
+
+void Includes::add(const Include &I) {
+  unsigned Index = All.size();
+  All.push_back(I);
+  auto BySpellingIt = BySpelling.try_emplace(I.Spelled).first;
+  All.back().Spelled = BySpellingIt->first(); // Now we own the backing string.
+
+  BySpellingIt->second.push_back(Index);
+  if (I.Resolved)
+    ByFile[I.Resolved].push_back(Index);
+  ByLine[I.Line] = Index;
+}
+
+const Include *Includes::atLine(unsigned OneBasedIndex) const {
+  auto It = ByLine.find(OneBasedIndex);
+  return (It == ByLine.end()) ? nullptr : &All[It->second];
+}
+
+llvm::SmallVector<const Include *> Includes::match(Header H) const {
+  llvm::SmallVector<const Include *> Result;
+  switch (H.kind()) {
+  case Header::Physical:
+    for (unsigned I : ByFile.lookup(H.physical()))
+      Result.push_back(&All[I]);
+    break;
+  case Header::Standard:
+    for (unsigned I : BySpelling.lookup(H.standard().name().trim("<>")))
+      Result.push_back(&All[I]);
+    break;
+  case Header::Verbatim:
+    for (unsigned I : BySpelling.lookup(H.verbatim().trim("\"<>")))
+      Result.push_back(&All[I]);
+    break;
+  }
+  return Result;
+}
+
 } // namespace clang::include_cleaner

clang-tools-extra/include-cleaner/unittests/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   TestingSupport
   )
 
 add_custom_target(ClangIncludeCleanerUnitTests)
 add_unittest(ClangIncludeCleanerUnitTests ClangIncludeCleanerTests
   AnalysisTest.cpp
   FindHeadersTest.cpp
   RecordTest.cpp
+  TypesTest.cpp
   WalkASTTest.cpp
 )
 
 target_include_directories(ClangIncludeCleanerTests
   PRIVATE
   ${CMAKE_CURRENT_SOURCE_DIR}/../lib)
 
 clang_target_link_libraries(ClangIncludeCleanerTests

clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

 //===-- RecordTest.cpp ----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang-include-cleaner/Record.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
-#include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/ADT/ArrayRef.h"
-#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang::include_cleaner {
 namespace {
-using testing::ElementsAre;
 using testing::ElementsAreArray;
 using testing::IsEmpty;
 
 // Matches a Decl* if it is a NamedDecl with the given name.
 MATCHER_P(named, N, "") {
   if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(arg)) {
     if (N == ND->getNameAsString())
       return true;
for (const auto &Ref : Recorded.MacroReferences) {
     ASSERT_EQ(FID, SM.getMainFileID());
     EXPECT_EQ(Ref.RT, RefType::Ambiguous);
     EXPECT_EQ("X", Ref.Target.macro().Name->getName());
     RefOffsets.push_back(Off);
   }
   EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points()));
 }
 
-// Matches an Include* on the specified line;
-MATCHER_P(line, N, "") { return arg->Line == (unsigned)N; }
-
-TEST(RecordedIncludesTest, Match) {
-  // We're using synthetic data, but need a FileManager to obtain FileEntry*s.
-  // Ensure it doesn't do any actual IO.
-  auto FS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
-  FileManager FM(FileSystemOptions{});
-  const FileEntry *A = FM.getVirtualFile("/path/a", /*Size=*/0, time_t{});
-  const FileEntry *B = FM.getVirtualFile("/path/b", /*Size=*/0, time_t{});
-
-  RecordedPP::RecordedIncludes Includes;
-  Includes.add(Include{"a", A, SourceLocation(), 1});
-  Includes.add(Include{"a2", A, SourceLocation(), 2});
-  Includes.add(Include{"b", B, SourceLocation(), 3});
-  Includes.add(Include{"vector", B, SourceLocation(), 4});
-  Includes.add(Include{"vector", B, SourceLocation(), 5});
-  Includes.add(Include{"missing", nullptr, SourceLocation(), 6});
-
-  EXPECT_THAT(Includes.match(A), ElementsAre(line(1), line(2)));
-  EXPECT_THAT(Includes.match(B), ElementsAre(line(3), line(4), line(5)));
-  EXPECT_THAT(Includes.match(*tooling::stdlib::Header::named("<vector>")),
-              ElementsAre(line(4), line(5)));
-}
-
 class PragmaIncludeTest : public ::testing::Test {
 protected:
   // We don't build an AST, we just run a preprocessor action!
   TestInputs Inputs;
   PragmaIncludes PI;
 
   PragmaIncludeTest() {
     Inputs.MakeAction = [this] {

clang-tools-extra/include-cleaner/unittests/TypesTest.cpp

+//===-- RecordTest.cpp ----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang-include-cleaner/Types.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Tooling/Inclusions/StandardLibrary.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang::include_cleaner {
+namespace {
+using testing::ElementsAre;
+
+// Matches an Include* on the specified line;
+MATCHER_P(line, N, "") { return arg->Line == (unsigned)N; }
+
+TEST(RecordedIncludesTest, Match) {
+  // We're using synthetic data, but need a FileManager to obtain FileEntry*s.
+  // Ensure it doesn't do any actual IO.
+  auto FS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+  FileManager FM(FileSystemOptions{});
+  const FileEntry *A = FM.getVirtualFile("/path/a", /*Size=*/0, time_t{});
+  const FileEntry *B = FM.getVirtualFile("/path/b", /*Size=*/0, time_t{});
+
+  Includes Inc;
+  Inc.add(Include{"a", A, SourceLocation(), 1});
+  Inc.add(Include{"a2", A, SourceLocation(), 2});
+  Inc.add(Include{"b", B, SourceLocation(), 3});
+  Inc.add(Include{"vector", B, SourceLocation(), 4});
+  Inc.add(Include{"vector", B, SourceLocation(), 5});
+  Inc.add(Include{"missing", nullptr, SourceLocation(), 6});
+
+  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)));
+}
+
+} // namespace
+} // namespace clang::include_cleaner