[clang-tools-extra] 64366d4 - [clangd] Rollforward include-cleaner library usage in symbol collector.
Viktoriia Bakalova via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 11 00:57:45 PDT 2023
Author: Viktoriia Bakalova
Date: 2023-09-11T07:57:35Z
New Revision: 64366d4935d3c56ce5906a321edb2e91d4f886bc
URL: https://github.com/llvm/llvm-project/commit/64366d4935d3c56ce5906a321edb2e91d4f886bc
DIFF: https://github.com/llvm/llvm-project/commit/64366d4935d3c56ce5906a321edb2e91d4f886bc.diff
LOG: [clangd] Rollforward include-cleaner library usage in symbol collector.
Differential Revision: https://reviews.llvm.org/D156659
Added:
Modified:
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/index/SymbolCollector.h
clang-tools-extra/clangd/unittests/FileIndexTests.cpp
clang-tools-extra/clangd/unittests/IndexActionTests.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index e843413601f5a29..699370fe1adf3af 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -821,25 +821,45 @@ void SymbolCollector::setIncludeLocation(const Symbol &S, SourceLocation DefLoc,
// Use the expansion location to get the #include header since this is
// where the symbol is exposed.
IncludeFiles[S.ID] = SM.getDecomposedExpansionLoc(DefLoc).first;
+
+ // We update providers for a symbol with each occurence, as SymbolCollector
+ // might run while parsing, rather than at the end of a translation unit.
+ // Hence we see more and more redecls over time.
+ auto [It, Inserted] = SymbolProviders.try_emplace(S.ID);
+ auto Headers =
+ include_cleaner::headersForSymbol(Sym, SM, Opts.PragmaIncludes);
+ if (Headers.empty())
+ return;
+
+ auto *HeadersIter = Headers.begin();
+ include_cleaner::Header H = *HeadersIter;
+ while (HeadersIter != Headers.end() &&
+ H.kind() == include_cleaner::Header::Physical &&
+ !tooling::isSelfContainedHeader(H.physical(), SM,
+ PP->getHeaderSearchInfo())) {
+ H = *HeadersIter;
+ HeadersIter++;
+ }
+ It->second = H;
}
llvm::StringRef getStdHeader(const Symbol *S, const LangOptions &LangOpts) {
tooling::stdlib::Lang Lang = tooling::stdlib::Lang::CXX;
- if (LangOpts.C11)
- Lang = tooling::stdlib::Lang::C;
- else if(!LangOpts.CPlusPlus)
- return "";
-
- if (S->Scope == "std::" && S->Name == "move") {
- if (!S->Signature.contains(','))
- return "<utility>";
- return "<algorithm>";
- }
-
- if (auto StdSym = tooling::stdlib::Symbol::named(S->Scope, S->Name, Lang))
- if (auto Header = StdSym->header())
- return Header->name();
+ if (LangOpts.C11)
+ Lang = tooling::stdlib::Lang::C;
+ else if(!LangOpts.CPlusPlus)
return "";
+
+ if (S->Scope == "std::" && S->Name == "move") {
+ if (!S->Signature.contains(','))
+ return "<utility>";
+ return "<algorithm>";
+ }
+
+ if (auto StdSym = tooling::stdlib::Symbol::named(S->Scope, S->Name, Lang))
+ if (auto Header = StdSym->header())
+ return Header->name();
+ return "";
}
void SymbolCollector::finish() {
@@ -865,13 +885,16 @@ void SymbolCollector::finish() {
}
}
llvm::DenseMap<FileID, bool> FileToContainsImportsOrObjC;
+ llvm::DenseMap<include_cleaner::Header, std::string> HeaderSpelling;
// Fill in IncludeHeaders.
// We delay this until end of TU so header guards are all resolved.
- for (const auto &[SID, FID] : IncludeFiles) {
+ for (const auto &[SID, OptionalProvider] : SymbolProviders) {
const Symbol *S = Symbols.find(SID);
if (!S)
continue;
+ assert(IncludeFiles.find(SID) != IncludeFiles.end());
+ const auto FID = IncludeFiles.at(SID);
// Determine if the FID is #include'd or #import'ed.
Symbol::IncludeDirective Directives = Symbol::Invalid;
auto CollectDirectives = shouldCollectIncludePath(S->SymInfo.Kind);
@@ -891,20 +914,61 @@ void SymbolCollector::finish() {
if (Directives == Symbol::Invalid)
continue;
- // FIXME: Use the include-cleaner library instead.
- llvm::StringRef IncludeHeader = getStdHeader(S, ASTCtx->getLangOpts());
- if (IncludeHeader.empty())
- IncludeHeader = HeaderFileURIs->getIncludeHeader(FID);
+ // Use the include location-based logic for Objective-C symbols.
+ if (Directives & Symbol::Import) {
+ llvm::StringRef IncludeHeader = getStdHeader(S, ASTCtx->getLangOpts());
+ if (IncludeHeader.empty())
+ IncludeHeader = HeaderFileURIs->getIncludeHeader(FID);
+
+ if (!IncludeHeader.empty()) {
+ auto NewSym = *S;
+ NewSym.IncludeHeaders.push_back({IncludeHeader, 1, Directives});
+ Symbols.insert(NewSym);
+ }
+ // FIXME: use providers from include-cleaner library once it's polished
+ // for Objective-C.
+ continue;
+ }
+
+ assert(Directives == Symbol::Include);
+ // For #include's, use the providers computed by the include-cleaner
+ // library.
+ if (!OptionalProvider)
+ continue;
+ const auto &H = *OptionalProvider;
+ const auto [SpellingIt, Inserted] = HeaderSpelling.try_emplace(H);
+ if (Inserted) {
+ auto &SM = ASTCtx->getSourceManager();
+ if (H.kind() == include_cleaner::Header::Kind::Physical) {
+ // FIXME: Get rid of this once include-cleaner has support for system
+ // headers.
+ if (auto Canonical =
+ HeaderFileURIs->mapCanonical(H.physical()->getName());
+ !Canonical.empty())
+ SpellingIt->second = Canonical;
+ // For physical files, prefer URIs as spellings might change
+ // depending on the translation unit.
+ else if (tooling::isSelfContainedHeader(H.physical(), SM,
+ PP->getHeaderSearchInfo()))
+ SpellingIt->second =
+ HeaderFileURIs->toURI(H.physical()->getLastRef());
+ } else {
+ SpellingIt->second = include_cleaner::spellHeader(
+ {H, PP->getHeaderSearchInfo(),
+ SM.getFileEntryForID(SM.getMainFileID())});
+ }
+ }
- if (!IncludeHeader.empty()) {
+ if (!SpellingIt->second.empty()) {
auto NewSym = *S;
- NewSym.IncludeHeaders.push_back({IncludeHeader, 1, Directives});
+ NewSym.IncludeHeaders.push_back({SpellingIt->second, 1, Directives});
Symbols.insert(NewSym);
}
}
ReferencedSymbols.clear();
IncludeFiles.clear();
+ SymbolProviders.clear();
FilesWithObjCConstructs.clear();
}
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h
index 4687e2c3975f5ae..10765020de518bd 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.h
+++ b/clang-tools-extra/clangd/index/SymbolCollector.h
@@ -175,6 +175,10 @@ class SymbolCollector : public index::IndexDataConsumer {
void setIncludeLocation(const Symbol &S, SourceLocation,
const include_cleaner::Symbol &Sym);
+ // Providers for Symbol.IncludeHeaders.
+ // The final spelling is calculated in finish().
+ llvm::DenseMap<SymbolID, std::optional<include_cleaner::Header>>
+ SymbolProviders;
// Files which contain ObjC symbols.
// This is finalized and used in finish().
llvm::DenseSet<FileID> FilesWithObjCConstructs;
diff --git a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp
index 3dd017572d7d088..cf30b388d234dfd 100644
--- a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -239,7 +239,7 @@ TEST(FileIndexTest, IncludeCollected) {
"<the/good/header.h>");
}
-TEST(FileIndexTest, DISABLED_IWYUPragmaExport) {
+TEST(FileIndexTest, IWYUPragmaExport) {
FileIndex M;
TestTU File;
diff --git a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp
index 0fdd0e26812a782..fad751bd0f7dc22 100644
--- a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp
@@ -8,11 +8,15 @@
#include "Headers.h"
#include "TestFS.h"
+#include "URI.h"
#include "index/IndexAction.h"
#include "index/Serialization.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
#include "clang/Tooling/Tooling.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <string>
namespace clang {
namespace clangd {
@@ -40,6 +44,11 @@ MATCHER(hasSameURI, "") {
return toUri(Path) == URI;
}
+MATCHER_P(includeHeader, P, "") {
+ return (arg.IncludeHeaders.size() == 1) &&
+ (arg.IncludeHeaders.begin()->IncludeHeader == P);
+}
+
::testing::Matcher<const IncludeGraphNode &>
includesAre(const std::vector<std::string> &Includes) {
return ::testing::Field(&IncludeGraphNode::DirectIncludes,
@@ -312,6 +321,26 @@ TEST_F(IndexActionTest, SkipNestedSymbols) {
EXPECT_THAT(*IndexFile.Symbols, testing::Contains(hasName("Bar")));
EXPECT_THAT(*IndexFile.Symbols, Not(testing::Contains(hasName("Baz"))));
}
+
+TEST_F(IndexActionTest, SymbolFromCC) {
+ std::string MainFilePath = testPath("main.cpp");
+ addFile(MainFilePath, R"cpp(
+ #include "main.h"
+ void foo() {}
+ )cpp");
+ addFile(testPath("main.h"), R"cpp(
+ #pragma once
+ void foo();
+ )cpp");
+ Opts.FileFilter = [](const SourceManager &SM, FileID F) {
+ return !SM.getFileEntryRefForID(F)->getName().endswith("main.h");
+ };
+ IndexFileIn IndexFile = runIndexingAction(MainFilePath, {"-std=c++14"});
+ EXPECT_THAT(*IndexFile.Symbols,
+ UnorderedElementsAre(AllOf(
+ hasName("foo"),
+ includeHeader(URI::create(testPath("main.h")).toString()))));
+}
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 58b9c8580105198..f12441061c19836 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -14,6 +14,7 @@
#include "index/SymbolCollector.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Index/IndexingAction.h"
#include "clang/Index/IndexingOptions.h"
@@ -1554,6 +1555,8 @@ TEST_F(SymbolCollectorTest, CanonicalSTLHeader) {
// Move overloads have special handling.
template <typename _T> T&& move(_T&& __value);
template <typename _I, typename _O> _O move(_I, _I, _O);
+ template <typename _T, typename _O, typename _I> _O move(
+ _T&&, _O, _O, _I);
}
)cpp",
/*Main=*/"");
@@ -1565,7 +1568,8 @@ TEST_F(SymbolCollectorTest, CanonicalSTLHeader) {
includeHeader("<string>")),
// Parameter names are demangled.
AllOf(labeled("move(T &&value)"), includeHeader("<utility>")),
- AllOf(labeled("move(I, I, O)"), includeHeader("<algorithm>"))));
+ AllOf(labeled("move(I, I, O)"), includeHeader("<algorithm>")),
+ AllOf(labeled("move(T &&, O, O, I)"), includeHeader("<algorithm>"))));
}
TEST_F(SymbolCollectorTest, IWYUPragma) {
@@ -1592,7 +1596,7 @@ TEST_F(SymbolCollectorTest, IWYUPragmaWithDoubleQuotes) {
includeHeader("\"the/good/header.h\""))));
}
-TEST_F(SymbolCollectorTest, DISABLED_IWYUPragmaExport) {
+TEST_F(SymbolCollectorTest, IWYUPragmaExport) {
CollectorOpts.CollectIncludePath = true;
const std::string Header = R"cpp(#pragma once
#include "exporter.h"
@@ -1978,6 +1982,24 @@ TEST_F(SymbolCollectorTest, Concepts) {
qName("A"), hasKind(clang::index::SymbolKind::Concept))));
}
+TEST_F(SymbolCollectorTest, IncludeHeaderForwardDecls) {
+ CollectorOpts.CollectIncludePath = true;
+ const std::string Header = R"cpp(#pragma once
+struct Foo;
+#include "full.h"
+)cpp";
+ auto FullFile = testPath("full.h");
+ InMemoryFileSystem->addFile(FullFile, 0,
+ llvm::MemoryBuffer::getMemBuffer(R"cpp(
+#pragma once
+struct Foo {};)cpp"));
+ runSymbolCollector(Header, /*Main=*/"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(
+ qName("Foo"),
+ includeHeader(URI::create(FullFile).toString()))))
+ << *Symbols.begin();
+}
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list