This revision was automatically updated to reflect the committed changes. Closed by commit rGebfcd06d4222: [clangd] IncludeCleaner: Mark used headers (authored by kbobyrev).
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/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
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,40 @@ } } +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 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,6 @@ return llvm::None; return llvm::StringRef(Preamble->Version); } + } // 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,22 @@ /// - 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 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,65 @@ 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 Index: clang-tools-extra/clangd/Headers.h =================================================================== --- clang-tools-extra/clangd/Headers.h +++ clang-tools-extra/clangd/Headers.h @@ -61,6 +61,7 @@ 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); Index: clang-tools-extra/clangd/Headers.cpp =================================================================== --- clang-tools-extra/clangd/Headers.cpp +++ clang-tools-extra/clangd/Headers.cpp @@ -56,6 +56,8 @@ 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)
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits