VitaNuo updated this revision to Diff 499456.
VitaNuo marked an inline comment as done.
VitaNuo added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143509/new/
https://reviews.llvm.org/D143509
Files:
clang-tools-extra/clangd/Headers.cpp
clang-tools-extra/clangd/Headers.h
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/HeadersTests.cpp
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -197,6 +197,36 @@
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(&Includes.MainFileIncludes[0]));
+ EXPECT_THAT(Includes.mainFileIncludesWithSpelling("\"bar.h\""),
+ UnorderedElementsAre(&Includes.MainFileIncludes[1]));
+ EXPECT_THAT(Includes.mainFileIncludesWithSpelling("\"baz.h\""),
+ UnorderedElementsAre(&Includes.MainFileIncludes[2]));
+}
+
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
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ 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<const Inclusion *> computeUnusedIncludesExperimental(ParsedAST &AST) {
- const auto &SM = AST.getSourceManager();
- const auto &Includes = AST.getIncludeStructure();
- // FIXME: this map should probably be in IncludeStructure.
- llvm::StringMap<llvm::SmallVector<IncludeStructure::HeaderID>> BySpelling;
- for (const auto &Inc : Includes.MainFileIncludes) {
- if (Inc.HeaderID)
- BySpelling.try_emplace(Inc.Written)
- .first->second.push_back(
- static_cast<IncludeStructure::HeaderID>(*Inc.HeaderID));
- }
- // FIXME: !!this is a hacky way to collect macro references.
- std::vector<include_cleaner::SymbolReference> Macros;
- auto& PP = AST.getPreprocessor();
- for (const syntax::Token &Tok :
- AST.getTokens().spelledTokens(SM.getMainFileID())) {
+std::vector<const Inclusion *>
+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<include_cleaner::SymbolReference> 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,37 @@
include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)),
DefLoc},
include_cleaner::RefType::Explicit});
- }
- llvm::DenseSet<IncludeStructure::HeaderID> Used;
- include_cleaner::walkUsed(
- AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros,
- AST.getPragmaIncludes(), SM,
- [&](const include_cleaner::SymbolReference &Ref,
- llvm::ArrayRef<include_cleaner::Header> 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<IncludeStructure::HeaderID> Used;
+ include_cleaner::walkUsed(
+ AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros,
+ AST.getPragmaIncludes(), SM,
+ [&](const include_cleaner::SymbolReference &Ref,
+ llvm::ArrayRef<include_cleaner::Header> 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 *Inc :
+ Includes.mainFileIncludesWithSpelling(H.verbatim())) {
+ if (!Inc->HeaderID.has_value())
+ continue;
+ IncludeStructure::HeaderID ID =
+ static_cast<IncludeStructure::HeaderID>(*Inc->HeaderID);
+ Used.insert(ID);
+ }
+ break;
+ }
+ }
+ });
+ return getUnused(AST, Used, /*ReferencedPublicHeaders*/ {});
}
std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
Index: clang-tools-extra/clangd/Headers.h
===================================================================
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -155,13 +155,16 @@
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<std::string> allHeaders() const { return RealPathNames; }
+ // Returns includes inside the main file with the given spelling.
+ // Spelling should include brackets or quotes, e.g. <foo>.
+ llvm::SmallVector<const Inclusion *>
+ mainFileIncludesWithSpelling(llvm::StringRef Spelling) const;
+
// 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 +205,10 @@
// Contains a set of headers that have either "IWYU pragma: export" or "IWYU
// pragma: begin_exports".
llvm::DenseSet<HeaderID> HasIWYUExport;
+
+ // Maps written includes to indices in MainFileInclude for easier lookup by
+ // spelling.
+ llvm::StringMap<llvm::SmallVector<unsigned>> MainFileIncludesBySpelling;
};
// Calculates insertion edit for including a new header in a file.
Index: clang-tools-extra/clangd/Headers.cpp
===================================================================
--- clang-tools-extra/clangd/Headers.cpp
+++ clang-tools-extra/clangd/Headers.cpp
@@ -16,6 +16,7 @@
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Inclusions/HeaderAnalysis.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Path.h"
#include <cstring>
@@ -73,6 +74,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)
@@ -294,6 +297,14 @@
return Result;
}
+llvm::SmallVector<const Inclusion *>
+IncludeStructure::mainFileIncludesWithSpelling(llvm::StringRef Spelling) const {
+ llvm::SmallVector<const Inclusion *> Includes;
+ for (auto Idx : MainFileIncludesBySpelling.lookup(Spelling))
+ Includes.push_back(&MainFileIncludes[Idx]);
+ return Includes;
+}
+
void IncludeInserter::addExisting(const Inclusion &Inc) {
IncludedHeaders.insert(Inc.Written);
if (!Inc.Resolved.empty())
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
