[clang-tools-extra] ebfcd06 - [clangd] IncludeCleaner: Mark used headers

Kirill Bobyrev via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 5 09:08:34 PDT 2021


Author: Kirill Bobyrev
Date: 2021-10-05T18:08:24+02:00
New Revision: ebfcd06d422286dcdd0e9a8c57e207a46c8fb8fb

URL: https://github.com/llvm/llvm-project/commit/ebfcd06d422286dcdd0e9a8c57e207a46c8fb8fb
DIFF: https://github.com/llvm/llvm-project/commit/ebfcd06d422286dcdd0e9a8c57e207a46c8fb8fb.diff

LOG: [clangd] IncludeCleaner: Mark used headers

Follow-up on D105426.

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D108194

Added: 
    

Modified: 
    clang-tools-extra/clangd/Headers.cpp
    clang-tools-extra/clangd/Headers.h
    clang-tools-extra/clangd/IncludeCleaner.cpp
    clang-tools-extra/clangd/IncludeCleaner.h
    clang-tools-extra/clangd/ParsedAST.cpp
    clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp
index 5946180f346d..9f5ab244aa63 100644
--- a/clang-tools-extra/clangd/Headers.cpp
+++ b/clang-tools-extra/clangd/Headers.cpp
@@ -56,6 +56,8 @@ class RecordHeaders : public PPCallbacks {
           SM.getLineNumber(SM.getFileID(HashLoc), Inc.HashOffset) - 1;
       Inc.FileKind = FileKind;
       Inc.Directive = IncludeTok.getIdentifierInfo()->getPPKeywordID();
+      if (File)
+        Inc.HeaderID = static_cast<unsigned>(Out->getOrCreateID(File));
     }
 
     // Record include graph (not just for main-file includes)

diff  --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h
index 513e21d62088..815131572eac 100644
--- a/clang-tools-extra/clangd/Headers.h
+++ b/clang-tools-extra/clangd/Headers.h
@@ -61,6 +61,7 @@ struct Inclusion {
   unsigned HashOffset = 0; // Byte offset from start of file to #.
   int HashLine = 0;        // Line number containing the directive, 0-indexed.
   SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User;
+  llvm::Optional<unsigned> HeaderID;
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Inclusion &);
 bool operator==(const Inclusion &LHS, const Inclusion &RHS);

diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index dbf19d4e08f9..d9bf600a04b8 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -98,6 +98,35 @@ class ReferencedLocationCrawler
   llvm::DenseSet<const void *> Visited;
 };
 
+// Given a set of referenced FileIDs, determines all the potentially-referenced
+// files and macros by traversing expansion/spelling locations of macro IDs.
+// This is used to map the referenced SourceLocations onto real files.
+struct ReferencedFiles {
+  ReferencedFiles(const SourceManager &SM) : SM(SM) {}
+  llvm::DenseSet<FileID> Files;
+  llvm::DenseSet<FileID> Macros;
+  const SourceManager &SM;
+
+  void add(SourceLocation Loc) { add(SM.getFileID(Loc), Loc); }
+
+  void add(FileID FID, SourceLocation Loc) {
+    if (FID.isInvalid())
+      return;
+    assert(SM.isInFileID(Loc, FID));
+    if (Loc.isFileID()) {
+      Files.insert(FID);
+      return;
+    }
+    // Don't process the same macro FID twice.
+    if (!Macros.insert(FID).second)
+      return;
+    const auto &Exp = SM.getSLocEntry(FID).getExpansion();
+    add(Exp.getSpellingLoc());
+    add(Exp.getExpansionLocStart());
+    add(Exp.getExpansionLocEnd());
+  }
+};
+
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST &AST) {
@@ -108,5 +137,65 @@ ReferencedLocations findReferencedLocations(ParsedAST &AST) {
   return Result;
 }
 
+llvm::DenseSet<FileID>
+findReferencedFiles(const llvm::DenseSet<SourceLocation> &Locs,
+                    const SourceManager &SM) {
+  std::vector<SourceLocation> Sorted{Locs.begin(), Locs.end()};
+  llvm::sort(Sorted); // Group by FileID.
+  ReferencedFiles Result(SM);
+  for (auto It = Sorted.begin(); It < Sorted.end();) {
+    FileID FID = SM.getFileID(*It);
+    Result.add(FID, *It);
+    // Cheaply skip over all the other locations from the same FileID.
+    // This avoids lots of redundant Loc->File lookups for the same file.
+    do
+      ++It;
+    while (It != Sorted.end() && SM.isInFileID(*It, FID));
+  }
+  return std::move(Result.Files);
+}
+
+std::vector<const Inclusion *>
+getUnused(const IncludeStructure &Structure,
+          const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles) {
+  std::vector<const Inclusion *> Unused;
+  for (const Inclusion &MFI : Structure.MainFileIncludes) {
+    // FIXME: Skip includes that are not self-contained.
+    assert(MFI.HeaderID);
+    auto IncludeID = static_cast<IncludeStructure::HeaderID>(*MFI.HeaderID);
+    if (!ReferencedFiles.contains(IncludeID)) {
+      Unused.push_back(&MFI);
+    }
+    dlog("{0} is {1}", MFI.Written,
+         ReferencedFiles.contains(IncludeID) ? "USED" : "UNUSED");
+  }
+  return Unused;
+}
+
+llvm::DenseSet<IncludeStructure::HeaderID>
+translateToHeaderIDs(const llvm::DenseSet<FileID> &Files,
+                     const IncludeStructure &Includes,
+                     const SourceManager &SM) {
+  llvm::DenseSet<IncludeStructure::HeaderID> TranslatedHeaderIDs;
+  TranslatedHeaderIDs.reserve(Files.size());
+  for (FileID FID : Files) {
+    const FileEntry *FE = SM.getFileEntryForID(FID);
+    assert(FE);
+    const auto File = Includes.getID(FE);
+    assert(File);
+    TranslatedHeaderIDs.insert(*File);
+  }
+  return TranslatedHeaderIDs;
+}
+
+std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST) {
+  const auto &SM = AST.getSourceManager();
+
+  auto Refs = findReferencedLocations(AST);
+  auto ReferencedFiles = translateToHeaderIDs(findReferencedFiles(Refs, SM),
+                                              AST.getIncludeStructure(), SM);
+  return getUnused(AST.getIncludeStructure(), ReferencedFiles);
+}
+
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h
index ca9f79c0f3b0..b232c09cae58 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -25,6 +25,7 @@
 #include "ParsedAST.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/DenseSet.h"
+#include <vector>
 
 namespace clang {
 namespace clangd {
@@ -46,6 +47,22 @@ using ReferencedLocations = llvm::DenseSet<SourceLocation>;
 /// - err on the side of reporting all possible locations
 ReferencedLocations findReferencedLocations(ParsedAST &AST);
 
+/// Retrieves IDs of all files containing SourceLocations from \p Locs.
+llvm::DenseSet<FileID> findReferencedFiles(const ReferencedLocations &Locs,
+                                           const SourceManager &SM);
+
+/// Maps FileIDs to the internal IncludeStructure representation (HeaderIDs).
+llvm::DenseSet<IncludeStructure::HeaderID>
+translateToHeaderIDs(const llvm::DenseSet<FileID> &Files,
+                     const IncludeStructure &Includes, const SourceManager &SM);
+
+/// Retrieves headers that are referenced from the main file but not used.
+std::vector<const Inclusion *>
+getUnused(const IncludeStructure &Includes,
+          const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles);
+
+std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST);
+
 } // namespace clangd
 } // namespace clang
 

diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 21a494aa462f..53bd01b1fddf 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -18,6 +18,7 @@
 #include "FeatureModule.h"
 #include "Headers.h"
 #include "HeuristicResolver.h"
+#include "IncludeCleaner.h"
 #include "IncludeFixer.h"
 #include "Preamble.h"
 #include "SourceCode.h"
@@ -624,5 +625,6 @@ llvm::Optional<llvm::StringRef> ParsedAST::preambleVersion() const {
     return llvm::None;
   return llvm::StringRef(Preamble->Version);
 }
+
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 718076adc762..c6e816731961 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -16,6 +16,8 @@ namespace clang {
 namespace clangd {
 namespace {
 
+using ::testing::UnorderedElementsAre;
+
 TEST(IncludeCleaner, ReferencedLocations) {
   struct TestCase {
     std::string HeaderCode;
@@ -131,6 +133,40 @@ TEST(IncludeCleaner, ReferencedLocations) {
   }
 }
 
+TEST(IncludeCleaner, GetUnusedHeaders) {
+  llvm::StringLiteral MainFile = R"cpp(
+    #include "a.h"
+    #include "b.h"
+    #include "dir/c.h"
+    #include "dir/unused.h"
+    #include "unused.h"
+    void foo() {
+      a();
+      b();
+      c();
+    })cpp";
+  // Build expected ast with symbols coming from headers.
+  TestTU TU;
+  TU.Filename = "foo.cpp";
+  TU.AdditionalFiles["foo.h"] = "void foo();";
+  TU.AdditionalFiles["a.h"] = "void a();";
+  TU.AdditionalFiles["b.h"] = "void b();";
+  TU.AdditionalFiles["dir/c.h"] = "void c();";
+  TU.AdditionalFiles["unused.h"] = "void unused();";
+  TU.AdditionalFiles["dir/unused.h"] = "void dirUnused();";
+  TU.AdditionalFiles["not_included.h"] = "void notIncluded();";
+  TU.ExtraArgs = {"-I" + testPath("dir")};
+  TU.Code = MainFile.str();
+  ParsedAST AST = TU.build();
+  auto UnusedIncludes = computeUnusedIncludes(AST);
+  std::vector<std::string> UnusedHeaders;
+  UnusedHeaders.reserve(UnusedIncludes.size());
+  for (const auto &Include : UnusedIncludes)
+    UnusedHeaders.push_back(Include->Written);
+  EXPECT_THAT(UnusedHeaders,
+              UnorderedElementsAre("\"unused.h\"", "\"dir/unused.h\""));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


        


More information about the cfe-commits mailing list