[clang-tools-extra] 9347655 - [clangd] Add xref for macro to static index.
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 4 19:24:12 PST 2019
Author: Utkarsh Saxena
Date: 2019-12-05T04:23:18+01:00
New Revision: 9347655a275456c08222833b11ec699fafbc6de6
URL: https://github.com/llvm/llvm-project/commit/9347655a275456c08222833b11ec699fafbc6de6
DIFF: https://github.com/llvm/llvm-project/commit/9347655a275456c08222833b11ec699fafbc6de6.diff
LOG: [clangd] Add xref for macro to static index.
Summary:
This adds the references for macros to the SymbolCollector (used for static index).
Enabled if `CollectMacro` option is set.
Reviewers: hokein
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D70489
Added:
Modified:
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/index/SymbolCollector.h
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 00adbd84fd62..191cd68ccb29 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -16,6 +16,7 @@
#include "SourceCode.h"
#include "SymbolLocation.h"
#include "URI.h"
+#include "index/SymbolID.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
@@ -345,43 +346,52 @@ bool SymbolCollector::handleMacroOccurence(const IdentifierInfo *Name,
const MacroInfo *MI,
index::SymbolRoleSet Roles,
SourceLocation Loc) {
- if (!Opts.CollectMacro)
- return true;
assert(PP.get());
const auto &SM = PP->getSourceManager();
auto DefLoc = MI->getDefinitionLoc();
+ auto SpellingLoc = SM.getSpellingLoc(Loc);
+ bool IsMainFileSymbol = SM.isInMainFile(SM.getExpansionLoc(DefLoc));
// Builtin macros don't have useful locations and aren't needed in completion.
if (MI->isBuiltinMacro())
return true;
- // Skip main-file symbols if we are not collecting them.
- bool IsMainFileSymbol = SM.isInMainFile(SM.getExpansionLoc(DefLoc));
- if (IsMainFileSymbol && !Opts.CollectMainFileSymbols)
- return false;
-
// Also avoid storing predefined macros like __DBL_MIN__.
if (SM.isWrittenInBuiltinFile(DefLoc))
return true;
+ auto ID = getSymbolID(Name->getName(), MI, SM);
+ if (!ID)
+ return true;
+
+ // Do not store references to main-file macros.
+ if ((static_cast<unsigned>(Opts.RefFilter) & Roles) && !IsMainFileSymbol &&
+ (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
+ MacroRefs[*ID].push_back({Loc, Roles});
+
+ // Collect symbols.
+ if (!Opts.CollectMacro)
+ return true;
+
+ // Skip main-file macros if we are not collecting them.
+ if (IsMainFileSymbol && !Opts.CollectMainFileSymbols)
+ return false;
+
// Mark the macro as referenced if this is a reference coming from the main
// file. The macro may not be an interesting symbol, but it's cheaper to check
// at the end.
if (Opts.CountReferences &&
(Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
- SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
+ SM.getFileID(SpellingLoc) == SM.getMainFileID())
ReferencedMacros.insert(Name);
+
// Don't continue indexing if this is a mere reference.
// FIXME: remove macro with ID if it is undefined.
if (!(Roles & static_cast<unsigned>(index::SymbolRole::Declaration) ||
Roles & static_cast<unsigned>(index::SymbolRole::Definition)))
return true;
- auto ID = getSymbolID(Name->getName(), MI, SM);
- if (!ID)
- return true;
-
// Only collect one instance in case there are multiple.
if (Symbols.find(*ID) != nullptr)
return true;
@@ -485,10 +495,10 @@ void SymbolCollector::finish() {
IncRef(*ID);
}
}
-
// Fill in IncludeHeaders.
// We delay this until end of TU so header guards are all resolved.
- // Symbols in slabs aren' mutable, so insert() has to walk all the strings :-(
+ // Symbols in slabs aren' mutable, so insert() has to walk all the strings
+ // :-(
llvm::SmallString<256> QName;
for (const auto &Entry : IncludeFiles)
if (const Symbol *S = Symbols.find(Entry.first)) {
@@ -518,25 +528,34 @@ void SymbolCollector::finish() {
}
return Found->second;
};
+ auto CollectRef =
+ [&](SymbolID ID,
+ const std::pair<SourceLocation, index::SymbolRoleSet> &LocAndRole) {
+ auto FileID = SM.getFileID(LocAndRole.first);
+ // FIXME: use the result to filter out references.
+ shouldIndexFile(FileID);
+ if (auto FileURI = GetURI(FileID)) {
+ auto Range =
+ getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
+ Ref R;
+ R.Location.Start = Range.first;
+ R.Location.End = Range.second;
+ R.Location.FileURI = FileURI->c_str();
+ R.Kind = toRefKind(LocAndRole.second);
+ Refs.insert(ID, R);
+ }
+ };
+ // Populate Refs slab from MacroRefs.
+ for (const auto &IDAndRefs : MacroRefs) {
+ for (const auto &LocAndRole : IDAndRefs.second)
+ CollectRef(IDAndRefs.first, LocAndRole);
+ }
// Populate Refs slab from DeclRefs.
if (auto MainFileURI = GetURI(SM.getMainFileID())) {
for (const auto &It : DeclRefs) {
if (auto ID = getSymbolID(It.first)) {
- for (const auto &LocAndRole : It.second) {
- auto FileID = SM.getFileID(LocAndRole.first);
- // FIXME: use the result to filter out references.
- shouldIndexFile(FileID);
- if (auto FileURI = GetURI(FileID)) {
- auto Range =
- getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
- Ref R;
- R.Location.Start = Range.first;
- R.Location.End = Range.second;
- R.Location.FileURI = FileURI->c_str();
- R.Kind = toRefKind(LocAndRole.second);
- Refs.insert(*ID, R);
- }
- }
+ for (const auto &LocAndRole : It.second)
+ CollectRef(*ID, LocAndRole);
}
}
}
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h
index 5ad44150b4d5..bc5095d516db 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.h
+++ b/clang-tools-extra/clangd/index/SymbolCollector.h
@@ -151,11 +151,12 @@ class SymbolCollector : public index::IndexDataConsumer {
std::shared_ptr<GlobalCodeCompletionAllocator> CompletionAllocator;
std::unique_ptr<CodeCompletionTUInfo> CompletionTUInfo;
Options Opts;
- using DeclRef = std::pair<SourceLocation, index::SymbolRoleSet>;
+ using SymbolRef = std::pair<SourceLocation, index::SymbolRoleSet>;
// Symbols referenced from the current TU, flushed on finish().
llvm::DenseSet<const NamedDecl *> ReferencedDecls;
llvm::DenseSet<const IdentifierInfo *> ReferencedMacros;
- llvm::DenseMap<const NamedDecl *, std::vector<DeclRef>> DeclRefs;
+ llvm::DenseMap<const NamedDecl *, std::vector<SymbolRef>> DeclRefs;
+ llvm::DenseMap<SymbolID, std::vector<SymbolRef>> MacroRefs;
// Maps canonical declaration provided by clang to canonical declaration for
// an index symbol, if clangd prefers a
diff erent declaration than that
// provided by clang. For example, friend declaration might be considered
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index d737862fa046..abc7aa389bd5 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -39,6 +39,7 @@ using ::testing::Contains;
using ::testing::Each;
using ::testing::ElementsAre;
using ::testing::Field;
+using ::testing::IsEmpty;
using ::testing::Not;
using ::testing::Pair;
using ::testing::UnorderedElementsAre;
@@ -214,7 +215,8 @@ class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override {
if (PragmaHandler)
CI.getPreprocessor().addCommentHandler(PragmaHandler);
- return createIndexingASTConsumer(DataConsumer, Opts, CI.getPreprocessorPtr());
+ return createIndexingASTConsumer(DataConsumer, Opts,
+ CI.getPreprocessorPtr());
}
bool BeginInvocation(CompilerInstance &CI) override {
@@ -577,15 +579,16 @@ o]]();
TEST_F(SymbolCollectorTest, Refs) {
Annotations Header(R"(
- class $foo[[Foo]] {
+ #define MACRO(X) (X + 1)
+ class Foo {
public:
- $foo[[Foo]]() {}
- $foo[[Foo]](int);
+ Foo() {}
+ Foo(int);
};
- class $bar[[Bar]];
- void $func[[func]]();
+ class Bar;
+ void func();
- namespace $ns[[NS]] {} // namespace ref is ignored
+ namespace NS {} // namespace ref is ignored
)");
Annotations Main(R"(
class $bar[[Bar]] {};
@@ -598,19 +601,20 @@ TEST_F(SymbolCollectorTest, Refs) {
$func[[func]]();
int abc = 0;
$foo[[Foo]] foo2 = abc;
+ abc = $macro[[MACRO]](1);
}
)");
Annotations SymbolsOnlyInMainCode(R"(
+ #define FUNC(X) (X+1)
int a;
void b() {}
- static const int c = 0;
+ static const int c = FUNC(1);
class d {};
)");
CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.CollectMacro = true;
runSymbolCollector(Header.code(),
(Main.code() + SymbolsOnlyInMainCode.code()).str());
- auto HeaderSymbols = TestTU::withHeaderCode(Header.code()).headerSymbols();
-
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
HaveRanges(Main.ranges("foo")))));
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Bar").ID,
@@ -618,12 +622,82 @@ TEST_F(SymbolCollectorTest, Refs) {
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID,
HaveRanges(Main.ranges("func")))));
EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _))));
- // Symbols *only* in the main file (a, b, c) had no refs collected.
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID,
+ HaveRanges(Main.ranges("macro")))));
+ // Symbols *only* in the main file (a, b, c, FUNC) had no refs collected.
auto MainSymbols =
TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "a").ID, _))));
EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "b").ID, _))));
EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _))));
+ EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "FUNC").ID, _))));
+}
+
+TEST_F(SymbolCollectorTest, MacroRefInHeader) {
+ Annotations Header(R"(
+ #define $foo[[FOO]](X) (X + 1)
+ #define $bar[[BAR]](X) (X + 2)
+
+ // Macro defined multiple times.
+ #define $ud1[[UD]] 1
+ int ud_1 = $ud1[[UD]];
+ #undef UD
+
+ #define $ud2[[UD]] 2
+ int ud_2 = $ud2[[UD]];
+ #undef UD
+
+ // Macros from token concatenations not included.
+ #define $concat[[CONCAT]](X) X##A()
+ #define $prepend[[PREPEND]](X) MACRO##X()
+ #define $macroa[[MACROA]]() 123
+ int B = $concat[[CONCAT]](MACRO);
+ int D = $prepend[[PREPEND]](A);
+
+ void fff() {
+ int abc = $foo[[FOO]](1) + $bar[[BAR]]($foo[[FOO]](1));
+ }
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.RefsInHeaders = true;
+ // Need this to get the SymbolID for macros for tests.
+ CollectorOpts.CollectMacro = true;
+
+ runSymbolCollector(Header.code(), "");
+
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "FOO").ID,
+ HaveRanges(Header.ranges("foo")))));
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "BAR").ID,
+ HaveRanges(Header.ranges("bar")))));
+ // No unique ID for multiple symbols named UD. Check for ranges only.
+ EXPECT_THAT(Refs, Contains(Pair(_, HaveRanges(Header.ranges("ud1")))));
+ EXPECT_THAT(Refs, Contains(Pair(_, HaveRanges(Header.ranges("ud2")))));
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "CONCAT").ID,
+ HaveRanges(Header.ranges("concat")))));
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "PREPEND").ID,
+ HaveRanges(Header.ranges("prepend")))));
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACROA").ID,
+ HaveRanges(Header.ranges("macroa")))));
+}
+
+TEST_F(SymbolCollectorTest, MacroRefWithoutCollectingSymbol) {
+ Annotations Header(R"(
+ #define $foo[[FOO]](X) (X + 1)
+ int abc = $foo[[FOO]](1);
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.RefsInHeaders = true;
+ CollectorOpts.CollectMacro = false;
+ runSymbolCollector(Header.code(), "");
+ EXPECT_THAT(Refs, Contains(Pair(_, HaveRanges(Header.ranges("foo")))));
+}
+
+TEST_F(SymbolCollectorTest, MacrosWithRefFilter) {
+ Annotations Header("#define $macro[[MACRO]](X) (X + 1)");
+ Annotations Main("void foo() { int x = $macro[[MACRO]](1); }");
+ CollectorOpts.RefFilter = RefKind::Unknown;
+ runSymbolCollector(Header.code(), Main.code());
+ EXPECT_THAT(Refs, IsEmpty());
}
TEST_F(SymbolCollectorTest, NameReferences) {
@@ -675,21 +749,26 @@ TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
TestFileName = testPath("foo.hh");
runSymbolCollector("", Header.code());
EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Func")));
- EXPECT_THAT(Refs, UnorderedElementsAre(Pair(findSymbol(Symbols, "Foo").ID,
- HaveRanges(Header.ranges("Foo"))),
- Pair(findSymbol(Symbols, "Func").ID,
- HaveRanges(Header.ranges("Func")))));
+ EXPECT_THAT(Refs,
+ UnorderedElementsAre(Pair(findSymbol(Symbols, "Foo").ID,
+ HaveRanges(Header.ranges("Foo"))),
+ Pair(findSymbol(Symbols, "Func").ID,
+ HaveRanges(Header.ranges("Func")))));
}
TEST_F(SymbolCollectorTest, RefsInHeaders) {
CollectorOpts.RefFilter = RefKind::All;
CollectorOpts.RefsInHeaders = true;
+ CollectorOpts.CollectMacro = true;
Annotations Header(R"(
- class [[Foo]] {};
+ #define $macro[[MACRO]](x) (x+1)
+ class $foo[[Foo]] {};
)");
runSymbolCollector(Header.code(), "");
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
- HaveRanges(Header.ranges()))));
+ HaveRanges(Header.ranges("foo")))));
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID,
+ HaveRanges(Header.ranges("macro")))));
}
TEST_F(SymbolCollectorTest, Relations) {
@@ -704,7 +783,7 @@ TEST_F(SymbolCollectorTest, Relations) {
Contains(Relation{Base.ID, RelationKind::BaseOf, Derived.ID}));
}
-TEST_F(SymbolCollectorTest, References) {
+TEST_F(SymbolCollectorTest, CountReferences) {
const std::string Header = R"(
class W;
class X {};
More information about the cfe-commits
mailing list