diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h --- a/clang-tools-extra/clangd/Headers.h +++ b/clang-tools-extra/clangd/Headers.h @@ -23,6 +23,8 @@ #include "clang/Tooling/Inclusions/StandardLibrary.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" #include "llvm/Support/Error.h" @@ -155,13 +157,29 @@ return !NonSelfContained.contains(ID); } - bool hasIWYUExport(HeaderID ID) const { - return HasIWYUExport.contains(ID); - } + bool hasIWYUExport(HeaderID ID) const { return HasIWYUExport.contains(ID); } // Return all transitively reachable files. llvm::ArrayRef allHeaders() const { return RealPathNames; } + // Returns resolved includes inside the main file with the given spelling. + // Spelling should include brackets or quotes, e.g. . + llvm::SmallVector + mainFileIncludesWithSpelling(llvm::StringRef Spelling) const { + llvm::SmallVector Includes; + auto It = MainFileIncludesBySpelling.find(Spelling); + if (It == MainFileIncludesBySpelling.end()) + return Includes; + + for (unsigned Idx : It->second) { + std::optional HeaderOpt = MainFileIncludes[Idx].HeaderID; + if (HeaderOpt.has_value()) + Includes.push_back( + static_cast(*HeaderOpt)); + } + return Includes; + } + // Return all transitively reachable files, and their minimum include depth. // All transitive includes (absolute paths), with their minimum include depth. // Root --> 0, #included file --> 1, etc. @@ -202,6 +220,10 @@ // Contains a set of headers that have either "IWYU pragma: export" or "IWYU // pragma: begin_exports". llvm::DenseSet HasIWYUExport; + + // Maps written includes to indices in MainFileInclude for easier lookup by + // spelling. + llvm::StringMap> MainFileIncludesBySpelling; }; // Calculates insertion edit for including a new header in a file. diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp --- a/clang-tools-extra/clangd/Headers.cpp +++ b/clang-tools-extra/clangd/Headers.cpp @@ -73,6 +73,8 @@ IDs.push_back(HID); } } + Out->MainFileIncludesBySpelling.try_emplace(Inc.Written) + .first->second.push_back(Out->MainFileIncludes.size() - 1); } // Record include graph (not just for main-file includes) diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -85,7 +85,7 @@ // Using templateName case is handled by the override TraverseTemplateName. if (TST->getTemplateName().getKind() == TemplateName::UsingTemplate) return true; - add(TST->getAsCXXRecordDecl()); // Specialization + add(TST->getAsCXXRecordDecl()); // Specialization return true; } @@ -474,22 +474,16 @@ translateToHeaderIDs(ReferencedFiles, AST.getIncludeStructure(), SM); return getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas); } -std::vector computeUnusedIncludesExperimental(ParsedAST &AST) { - const auto &SM = AST.getSourceManager(); - const auto &Includes = AST.getIncludeStructure(); - // FIXME: this map should probably be in IncludeStructure. - llvm::StringMap> BySpelling; - for (const auto &Inc : Includes.MainFileIncludes) { - if (Inc.HeaderID) - BySpelling.try_emplace(Inc.Written) - .first->second.push_back( - static_cast(*Inc.HeaderID)); - } - // FIXME: !!this is a hacky way to collect macro references. - std::vector Macros; - auto& PP = AST.getPreprocessor(); - for (const syntax::Token &Tok : - AST.getTokens().spelledTokens(SM.getMainFileID())) { +std::vector +computeUnusedIncludesExperimental(ParsedAST &AST) { + const auto &SM = AST.getSourceManager(); + const auto &Includes = AST.getIncludeStructure(); + + // FIXME: !!this is a hacky way to collect macro references. + std::vector Macros; + auto &PP = AST.getPreprocessor(); + for (const syntax::Token &Tok : + AST.getTokens().spelledTokens(SM.getMainFileID())) { auto Macro = locateMacroAt(Tok, PP); if (!Macro) continue; @@ -499,31 +493,32 @@ include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)), DefLoc}, include_cleaner::RefType::Explicit}); - } - llvm::DenseSet Used; - include_cleaner::walkUsed( - AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros, - AST.getPragmaIncludes(), SM, - [&](const include_cleaner::SymbolReference &Ref, - llvm::ArrayRef Providers) { - for (const auto &H : Providers) { - switch (H.kind()) { - case include_cleaner::Header::Physical: - if (auto HeaderID = Includes.getID(H.physical())) - Used.insert(*HeaderID); - break; - case include_cleaner::Header::Standard: - for (auto HeaderID : Includes.StdlibHeaders.lookup(H.standard())) - Used.insert(HeaderID); - break; - case include_cleaner::Header::Verbatim: - for (auto HeaderID : BySpelling.lookup(H.verbatim())) - Used.insert(HeaderID); - break; - } - } - }); - return getUnused(AST, Used, /*ReferencedPublicHeaders*/{}); + } + llvm::DenseSet Used; + include_cleaner::walkUsed( + AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros, + AST.getPragmaIncludes(), SM, + [&](const include_cleaner::SymbolReference &Ref, + llvm::ArrayRef Providers) { + for (const auto &H : Providers) { + switch (H.kind()) { + case include_cleaner::Header::Physical: + if (auto HeaderID = Includes.getID(H.physical())) + Used.insert(*HeaderID); + break; + case include_cleaner::Header::Standard: + for (auto HeaderID : Includes.StdlibHeaders.lookup(H.standard())) + Used.insert(HeaderID); + break; + case include_cleaner::Header::Verbatim: + for (auto HeaderID : + Includes.mainFileIncludesWithSpelling(H.verbatim())) + Used.insert(HeaderID); + break; + } + } + }); + return getUnused(AST, Used, /*ReferencedPublicHeaders*/ {}); } std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST, diff --git a/clang-tools-extra/clangd/unittests/HeadersTests.cpp b/clang-tools-extra/clangd/unittests/HeadersTests.cpp --- a/clang-tools-extra/clangd/unittests/HeadersTests.cpp +++ b/clang-tools-extra/clangd/unittests/HeadersTests.cpp @@ -197,6 +197,39 @@ Distance(getID(BazHeader, Includes), 1u))); } +TEST_F(HeadersTest, CacheBySpellingIsBuiltForMainInclusions) { + std::string FooHeader = testPath("foo.h"); + FS.Files[FooHeader] = R"cpp( + void foo(); +)cpp"; + std::string BarHeader = testPath("bar.h"); + FS.Files[BarHeader] = R"cpp( + void bar(); +)cpp"; + std::string BazHeader = testPath("baz.h"); + FS.Files[BazHeader] = R"cpp( + void baz(); +)cpp"; + FS.Files[MainFile] = R"cpp( +#include "foo.h" +#include "bar.h" +#include "baz.h" +)cpp"; + auto Includes = collectIncludes(); + EXPECT_THAT(Includes.MainFileIncludes, + UnorderedElementsAre(written("\"foo.h\""), written("\"bar.h\""), + written("\"baz.h\""))); + EXPECT_THAT(Includes.mainFileIncludesWithSpelling("\"foo.h\""), + UnorderedElementsAre(static_cast( + *(Includes.MainFileIncludes[0]).HeaderID))); + EXPECT_THAT(Includes.mainFileIncludesWithSpelling("\"bar.h\""), + UnorderedElementsAre(static_cast( + *(Includes.MainFileIncludes[1]).HeaderID))); + EXPECT_THAT(Includes.mainFileIncludesWithSpelling("\"baz.h\""), + UnorderedElementsAre(static_cast( + *(Includes.MainFileIncludes[2]).HeaderID))); +} + TEST_F(HeadersTest, PreambleIncludesPresentOnce) { // We use TestTU here, to ensure we use the preamble replay logic. // We're testing that the logic doesn't crash, and doesn't result in duplicate