kbobyrev updated this revision to Diff 376431.
kbobyrev added a comment.

Tiny refactoring.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108194/new/

https://reviews.llvm.org/D108194

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

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -16,6 +16,8 @@
 namespace clangd {
 namespace {
 
+using ::testing::UnorderedElementsAre;
+
 TEST(IncludeCleaner, ReferencedLocations) {
   struct TestCase {
     std::string HeaderCode;
@@ -131,6 +133,42 @@
   }
 }
 
+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 = AST.computeUnusedIncludes();
+  std::vector<std::string> UnusedHeaders;
+  UnusedHeaders.reserve(UnusedIncludes.size());
+  for (const auto &Include : UnusedIncludes) {
+    // Strip enclosing "".
+    UnusedHeaders.push_back(
+        Include.Written.substr(1, Include.Written.size() - 2));
+  }
+  EXPECT_THAT(UnusedHeaders, UnorderedElementsAre("unused.h", "dir/unused.h"));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ParsedAST.h
===================================================================
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -118,6 +118,8 @@
     return Resolver.get();
   }
 
+  std::vector<Inclusion> computeUnusedIncludes();
+
 private:
   ParsedAST(llvm::StringRef Version,
             std::shared_ptr<const PreambleData> Preamble,
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ 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,32 @@
     return llvm::None;
   return llvm::StringRef(Preamble->Version);
 }
+
+std::vector<Inclusion> ParsedAST::computeUnusedIncludes() {
+  const auto &SM = getSourceManager();
+
+  auto Refs = findReferencedLocations(*this);
+  auto ReferencedFileIDs = findReferencedFiles(Refs, SM);
+  llvm::DenseSet<IncludeStructure::HeaderID> ReferencedFiles;
+  ReferencedFiles.reserve(ReferencedFileIDs.size());
+  for (FileID FID : ReferencedFileIDs) {
+    const FileEntry *FE = SM.getFileEntryForID(FID);
+    if (!FE) {
+      elog("Missing FE for {0}", SM.getComposedLoc(FID, 0).printToString(SM));
+      continue;
+    }
+    const auto File = Includes.getID(FE, SM);
+    if (!File) {
+      elog("Missing FE for {0}", SM.getComposedLoc(FID, 0).printToString(SM));
+      continue;
+    }
+    ReferencedFiles.insert(*File);
+  }
+  auto MainFileIndex =
+      Includes.getID(SM.getFileEntryForID(SM.getMainFileID()), SM);
+  assert(MainFileIndex && "MainFile should always have HeaderID (0)");
+  return getUnused(*MainFileIndex, Includes, ReferencedFiles, SM);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.h
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ 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,31 @@
 /// - err on the side of reporting all possible locations
 ReferencedLocations findReferencedLocations(ParsedAST &AST);
 
+/// Retrieves IDs of all files containing SourceLocations from \p Locs.
+/// FIXME: Those locations could be within macro expansions and are resolved to
+/// their spelling/expansion locations.
+llvm::DenseSet<FileID> findReferencedFiles(const ReferencedLocations &Locs,
+                                           const SourceManager &SM);
+
+inline llvm::DenseMap<IncludeStructure::HeaderID, bool> directlyReferencedFiles(
+    const IncludeStructure &Includes,
+    const llvm::DenseSet<IncludeStructure::HeaderID> &Referenced,
+    IncludeStructure::HeaderID EntryPoint) {
+  llvm::DenseMap<IncludeStructure::HeaderID, bool> Result;
+  for (IncludeStructure::HeaderID Inclusion :
+       Includes.IncludeChildren.lookup(EntryPoint))
+    Result.try_emplace(Inclusion, Referenced.contains(Inclusion));
+  return Result;
+}
+
+/// Retrieves headers that are referenced from the main file (\p EntryPoint)
+/// but not used.
+std::vector<Inclusion>
+getUnused(IncludeStructure::HeaderID EntryPoint,
+          const IncludeStructure &Includes,
+          const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles,
+          const SourceManager &SM);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -98,6 +98,35 @@
   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,50 @@
   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<Inclusion>
+getUnused(IncludeStructure::HeaderID EntryPoint,
+          const IncludeStructure &Structure,
+          const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles,
+          const SourceManager &SM) {
+  std::vector<Inclusion> Unused;
+  for (auto &MFI : Structure.MainFileIncludes) {
+    // FIXME: Skip includes that are not self-contained.
+    auto Entry = SM.getFileManager().getFile(MFI.Resolved);
+    if (!Entry) {
+      elog("Missing FileEntry for {0}", MFI.Resolved);
+      continue;
+    }
+    auto It = Structure.getID(*Entry, SM);
+    if (!It) {
+      elog("Missing IncludeStructure::File for {0}", MFI.Resolved);
+      continue;
+    }
+    bool Used = ReferencedFiles.find(*It) != ReferencedFiles.end();
+    if (!Used) {
+      Unused.push_back(MFI);
+    }
+    dlog("{0} is {1}", MFI.Written, Used ? "USED" : "UNUSED");
+  }
+  return Unused;
+}
+
 } // namespace clangd
 } // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to