[PATCH] D72395: [clangd] Publish xref for macros from Index and AST.
merge_guards_bot added a comment.
{icon check-circle color=green} Unit tests: pass. 61641 tests passed, 0 failed
and 777 were skipped.
{icon check-circle color=green} clang-tidy: pass.
{icon check-circle color=green} clang-format: pass.
Build artifacts
:
diff.json
,
clang-tidy.txt
,
clang-format.patch
,
CMakeCache.txt
,
console-log.txt
,
test-results.xml
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72395/new/
https://reviews.llvm.org/D72395
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72395: [clangd] Publish xref for macros from Index and AST.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG734aa1d133f2: [clangd] Publish xref for macros from Index
and AST. (authored by usaxena95).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72395/new/
https://reviews.llvm.org/D72395
Files:
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -937,6 +937,13 @@
[[CAT]](Fo, o) foo4;
}
)cpp",
+
+ R"cpp(// Macros
+#define [[MA^CRO]](X) (X+1)
+void test() {
+ int x = [[MACRO]]([[MACRO]](1));
+}
+ )cpp",
};
for (const char *Test : Tests) {
Annotations T(Test);
@@ -1011,7 +1018,7 @@
}
}
-TEST(FindReferences, NeedsIndex) {
+TEST(FindReferences, NeedsIndexForSymbols) {
const char *Header = "int foo();";
Annotations Main("int main() { [[f^oo]](); }");
TestTU TU;
@@ -1040,13 +1047,49 @@
EXPECT_EQ(1u, LimitRefs.References.size());
EXPECT_TRUE(LimitRefs.HasMore);
- // If the main file is in the index, we don't return duplicates.
- // (even if the references are in a different location)
+ // Avoid indexed results for the main file. Use AST for the mainfile.
TU.Code = ("\n\n" + Main.code()).str();
EXPECT_THAT(findReferences(AST, Main.point(), 0, TU.index().get()).References,
ElementsAre(RangeIs(Main.range(;
}
+TEST(FindReferences, NeedsIndexForMacro) {
+ const char *Header = "#define MACRO(X) (X+1)";
+ Annotations Main(R"cpp(
+int main() {
+ int a = [[MA^CRO]](1);
+}
+ )cpp");
+ TestTU TU;
+ TU.Code = Main.code();
+ TU.HeaderCode = Header;
+ auto AST = TU.build();
+
+ // References in main file are returned without index.
+ EXPECT_THAT(
+ findReferences(AST, Main.point(), 0, /*Index=*/nullptr).References,
+ ElementsAre(RangeIs(Main.range(;
+
+ Annotations IndexedMain(R"cpp(
+int indexed_main() {
+ int a = [[MACRO]](1);
+}
+ )cpp");
+
+ // References from indexed files are included.
+ TestTU IndexedTU;
+ IndexedTU.Code = IndexedMain.code();
+ IndexedTU.Filename = "Indexed.cpp";
+ IndexedTU.HeaderCode = Header;
+ EXPECT_THAT(
+ findReferences(AST, Main.point(), 0, IndexedTU.index().get()).References,
+ ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
+ auto LimitRefs =
+ findReferences(AST, Main.point(), /*Limit*/ 1, IndexedTU.index().get());
+ EXPECT_EQ(1u, LimitRefs.References.size());
+ EXPECT_TRUE(LimitRefs.HasMore);
+}
+
TEST(FindReferences, NoQueryForLocalSymbols) {
struct RecordingIndex : public MemIndex {
mutable Optional> RefIDs;
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -432,52 +432,73 @@
elog("Failed to get a path for the main file, so no references");
return Results;
}
+ auto URIMainFile = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
auto Loc = SM.getMacroArgExpandedLocation(
getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
- // TODO: should we handle macros, too?
- // We also show references to the targets of using-decls, so we include
- // DeclRelation::Underlying.
- DeclRelationSet Relations = DeclRelation::TemplatePattern |
- DeclRelation::Alias | DeclRelation::Underlying;
- auto Decls = getDeclAtPosition(AST, Loc, Relations);
-
- // We traverse the AST to find references in the main file.
- auto MainFileRefs = findRefs(Decls, AST);
- // We may get multiple refs with the same location and different Roles, as
- // cross-reference is only interested in locations, we deduplicate them
- // by the location to avoid emitting duplicated locations.
- MainFileRefs.erase(std::unique(MainFileRefs.begin(), MainFileRefs.end(),
- [](const ReferenceFinder::Reference ,
-const ReferenceFinder::Reference ) {
- return L.Loc == R.Loc;
- }),
- MainFileRefs.end());
- for (const auto : MainFileRefs) {
-if (auto Range = getTokenRange(SM, AST.getLangOpts(), Ref.Loc)) {
- Location Result;
- Result.range = *Range;
- Result.uri = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
- Results.References.push_back(std::move(Result));
+ RefsRequest Req;
+
+ if (auto Macro = locateMacroAt(Loc, AST.getPreprocessor())) {
+// Handle references to macro.
+if (auto MacroSID = getSymbolID(Macro->Name, Macro->Info, SM)) {
+ // Collect macro references from main file.
+ const auto
[PATCH] D72395: [clangd] Publish xref for macros from Index and AST.
usaxena95 added inline comments. Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1093 + + // If the main file is in the index, we don't return duplicates. + // (even if the references are in a different location) kadircet wrote: > i know the comment is copied over, but it doesn't reflect what's going on. > > as this check makes sure we prever results from AST rather than index for > main file, not the duplicate handling(which actually de-dups references on > the *same* location). > > since this is handled in the common code path, it is OK to check only in one > of the tests(up to you though). but please update the comments. Updated the old comment to show the correct intent. Also removed the check from the macros test since this was on the common code path. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72395/new/ https://reviews.llvm.org/D72395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72395: [clangd] Publish xref for macros from Index and AST.
usaxena95 updated this revision to Diff 237596.
usaxena95 marked 3 inline comments as done.
usaxena95 added a comment.
Removed repeated check from the test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72395/new/
https://reviews.llvm.org/D72395
Files:
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -937,6 +937,13 @@
[[CAT]](Fo, o) foo4;
}
)cpp",
+
+ R"cpp(// Macros
+#define [[MA^CRO]](X) (X+1)
+void test() {
+ int x = [[MACRO]]([[MACRO]](1));
+}
+ )cpp",
};
for (const char *Test : Tests) {
Annotations T(Test);
@@ -1011,7 +1018,7 @@
}
}
-TEST(FindReferences, NeedsIndex) {
+TEST(FindReferences, NeedsIndexForSymbols) {
const char *Header = "int foo();";
Annotations Main("int main() { [[f^oo]](); }");
TestTU TU;
@@ -1040,13 +1047,49 @@
EXPECT_EQ(1u, LimitRefs.References.size());
EXPECT_TRUE(LimitRefs.HasMore);
- // If the main file is in the index, we don't return duplicates.
- // (even if the references are in a different location)
+ // Avoid indexed results for the main file. Use AST for the mainfile.
TU.Code = ("\n\n" + Main.code()).str();
EXPECT_THAT(findReferences(AST, Main.point(), 0, TU.index().get()).References,
ElementsAre(RangeIs(Main.range(;
}
+TEST(FindReferences, NeedsIndexForMacro) {
+ const char *Header = "#define MACRO(X) (X+1)";
+ Annotations Main(R"cpp(
+int main() {
+ int a = [[MA^CRO]](1);
+}
+ )cpp");
+ TestTU TU;
+ TU.Code = Main.code();
+ TU.HeaderCode = Header;
+ auto AST = TU.build();
+
+ // References in main file are returned without index.
+ EXPECT_THAT(
+ findReferences(AST, Main.point(), 0, /*Index=*/nullptr).References,
+ ElementsAre(RangeIs(Main.range(;
+
+ Annotations IndexedMain(R"cpp(
+int indexed_main() {
+ int a = [[MACRO]](1);
+}
+ )cpp");
+
+ // References from indexed files are included.
+ TestTU IndexedTU;
+ IndexedTU.Code = IndexedMain.code();
+ IndexedTU.Filename = "Indexed.cpp";
+ IndexedTU.HeaderCode = Header;
+ EXPECT_THAT(
+ findReferences(AST, Main.point(), 0, IndexedTU.index().get()).References,
+ ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
+ auto LimitRefs =
+ findReferences(AST, Main.point(), /*Limit*/ 1, IndexedTU.index().get());
+ EXPECT_EQ(1u, LimitRefs.References.size());
+ EXPECT_TRUE(LimitRefs.HasMore);
+}
+
TEST(FindReferences, NoQueryForLocalSymbols) {
struct RecordingIndex : public MemIndex {
mutable Optional> RefIDs;
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -432,52 +432,73 @@
elog("Failed to get a path for the main file, so no references");
return Results;
}
+ auto URIMainFile = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
auto Loc = SM.getMacroArgExpandedLocation(
getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
- // TODO: should we handle macros, too?
- // We also show references to the targets of using-decls, so we include
- // DeclRelation::Underlying.
- DeclRelationSet Relations = DeclRelation::TemplatePattern |
- DeclRelation::Alias | DeclRelation::Underlying;
- auto Decls = getDeclAtPosition(AST, Loc, Relations);
-
- // We traverse the AST to find references in the main file.
- auto MainFileRefs = findRefs(Decls, AST);
- // We may get multiple refs with the same location and different Roles, as
- // cross-reference is only interested in locations, we deduplicate them
- // by the location to avoid emitting duplicated locations.
- MainFileRefs.erase(std::unique(MainFileRefs.begin(), MainFileRefs.end(),
- [](const ReferenceFinder::Reference ,
-const ReferenceFinder::Reference ) {
- return L.Loc == R.Loc;
- }),
- MainFileRefs.end());
- for (const auto : MainFileRefs) {
-if (auto Range = getTokenRange(SM, AST.getLangOpts(), Ref.Loc)) {
- Location Result;
- Result.range = *Range;
- Result.uri = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
- Results.References.push_back(std::move(Result));
+ RefsRequest Req;
+
+ if (auto Macro = locateMacroAt(Loc, AST.getPreprocessor())) {
+// Handle references to macro.
+if (auto MacroSID = getSymbolID(Macro->Name, Macro->Info, SM)) {
+ // Collect macro references from main file.
+ const auto =
[PATCH] D72395: [clangd] Publish xref for macros from Index and AST.
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, LGTM! Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1093 + + // If the main file is in the index, we don't return duplicates. + // (even if the references are in a different location) i know the comment is copied over, but it doesn't reflect what's going on. as this check makes sure we prever results from AST rather than index for main file, not the duplicate handling(which actually de-dups references on the *same* location). since this is handled in the common code path, it is OK to check only in one of the tests(up to you though). but please update the comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72395/new/ https://reviews.llvm.org/D72395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72395: [clangd] Publish xref for macros from Index and AST.
merge_guards_bot added a comment.
{icon check-circle color=green} Unit tests: pass. 61307 tests passed, 0 failed
and 736 were skipped.
{icon check-circle color=green} clang-tidy: pass.
{icon check-circle color=green} clang-format: pass.
Build artifacts
:
diff.json
,
clang-tidy.txt
,
clang-format.patch
,
CMakeCache.txt
,
console-log.txt
,
test-results.xml
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72395/new/
https://reviews.llvm.org/D72395
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72395: [clangd] Publish xref for macros from Index and AST.
usaxena95 marked 2 inline comments as done.
usaxena95 added inline comments.
Comment at: clang-tools-extra/clangd/XRefs.cpp:463
+ }
+ // Query the index for references from other files.
+ if (Index && Results.References.size() < Limit) {
kadircet wrote:
> could we merge this and the code for decls by only populating `Req.IDs` with
> `MacroSID` here and with the symbol id below and by finally making the query
> in the end ? i.e.
>
> ```
> RefsRequest Req;
> Req.Limit = Limit;
> if (macro) {
> handleMainFileMacros;
> Req.IDs.insert(*MacroID);
> } else {
> handleMainFileDecls;
> populateReqIDs(Req);
> }
> handleIndexResults(Req);
> ```
Tried to do something on those lines.
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1021
TEST(FindReferences, NeedsIndex) {
+ const char *Header = (R"cpp(
kadircet wrote:
> nit: i don't think there's much benefit in combining refs for macros and
> symbols in a single test. their handling in the code is disjoint, whereas
> this test is not. so it makes the test a little bit harder to read and also
> failures would be harder to reason about. but the test is currently small, so
> up to you whether you want to separate it or not.
Added the test for macros as a separate test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72395/new/
https://reviews.llvm.org/D72395
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72395: [clangd] Publish xref for macros from Index and AST.
usaxena95 updated this revision to Diff 236829.
usaxena95 marked 3 inline comments as done.
usaxena95 added a comment.
Addressed comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72395/new/
https://reviews.llvm.org/D72395
Files:
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -937,6 +937,13 @@
[[CAT]](Fo, o) foo4;
}
)cpp",
+
+ R"cpp(// Macros
+#define [[MA^CRO]](X) (X+1)
+void test() {
+ int x = [[MACRO]]([[MACRO]](1));
+}
+ )cpp",
};
for (const char *Test : Tests) {
Annotations T(Test);
@@ -1011,7 +1018,7 @@
}
}
-TEST(FindReferences, NeedsIndex) {
+TEST(FindReferences, NeedsIndexForSymbols) {
const char *Header = "int foo();";
Annotations Main("int main() { [[f^oo]](); }");
TestTU TU;
@@ -1047,6 +1054,49 @@
ElementsAre(RangeIs(Main.range(;
}
+TEST(FindReferences, NeedsIndexForMacro) {
+ const char *Header = "#define MACRO(X) (X+1)";
+ Annotations Main(R"cpp(
+int main() {
+ int a = [[MA^CRO]](1);
+}
+ )cpp");
+ TestTU TU;
+ TU.Code = Main.code();
+ TU.HeaderCode = Header;
+ auto AST = TU.build();
+
+ // References in main file are returned without index.
+ EXPECT_THAT(
+ findReferences(AST, Main.point(), 0, /*Index=*/nullptr).References,
+ ElementsAre(RangeIs(Main.range(;
+
+ Annotations IndexedMain(R"cpp(
+int indexed_main() {
+ int a = [[MACRO]](1);
+}
+ )cpp");
+
+ // References from indexed files are included.
+ TestTU IndexedTU;
+ IndexedTU.Code = IndexedMain.code();
+ IndexedTU.Filename = "Indexed.cpp";
+ IndexedTU.HeaderCode = Header;
+ EXPECT_THAT(
+ findReferences(AST, Main.point(), 0, IndexedTU.index().get()).References,
+ ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
+ auto LimitRefs =
+ findReferences(AST, Main.point(), /*Limit*/ 1, IndexedTU.index().get());
+ EXPECT_EQ(1u, LimitRefs.References.size());
+ EXPECT_TRUE(LimitRefs.HasMore);
+
+ // If the main file is in the index, we don't return duplicates.
+ // (even if the references are in a different location)
+ TU.Code = ("\n\n" + Main.code()).str();
+ EXPECT_THAT(findReferences(AST, Main.point(), 0, TU.index().get()).References,
+ ElementsAre(RangeIs(Main.range(;
+}
+
TEST(FindReferences, NoQueryForLocalSymbols) {
struct RecordingIndex : public MemIndex {
mutable Optional> RefIDs;
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -432,52 +432,73 @@
elog("Failed to get a path for the main file, so no references");
return Results;
}
+ auto URIMainFile = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
auto Loc = SM.getMacroArgExpandedLocation(
getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
- // TODO: should we handle macros, too?
- // We also show references to the targets of using-decls, so we include
- // DeclRelation::Underlying.
- DeclRelationSet Relations = DeclRelation::TemplatePattern |
- DeclRelation::Alias | DeclRelation::Underlying;
- auto Decls = getDeclAtPosition(AST, Loc, Relations);
-
- // We traverse the AST to find references in the main file.
- auto MainFileRefs = findRefs(Decls, AST);
- // We may get multiple refs with the same location and different Roles, as
- // cross-reference is only interested in locations, we deduplicate them
- // by the location to avoid emitting duplicated locations.
- MainFileRefs.erase(std::unique(MainFileRefs.begin(), MainFileRefs.end(),
- [](const ReferenceFinder::Reference ,
-const ReferenceFinder::Reference ) {
- return L.Loc == R.Loc;
- }),
- MainFileRefs.end());
- for (const auto : MainFileRefs) {
-if (auto Range = getTokenRange(SM, AST.getLangOpts(), Ref.Loc)) {
- Location Result;
- Result.range = *Range;
- Result.uri = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
- Results.References.push_back(std::move(Result));
+ RefsRequest Req;
+
+ if (auto Macro = locateMacroAt(Loc, AST.getPreprocessor())) {
+// Handle references to macro.
+if (auto MacroSID = getSymbolID(Macro->Name, Macro->Info, SM)) {
+ // Collect macro references from main file.
+ const auto = AST.getMacros().MacroRefs;
+ auto Refs = IDToRefs.find(*MacroSID);
+ if (Refs != IDToRefs.end()) {
+for (const auto Ref :
[PATCH] D72395: [clangd] Publish xref for macros from Index and AST.
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1021
TEST(FindReferences, NeedsIndex) {
+ const char *Header = (R"cpp(
nit: i don't think there's much benefit in combining refs for macros and
symbols in a single test. their handling in the code is disjoint, whereas this
test is not. so it makes the test a little bit harder to read and also failures
would be harder to reason about. but the test is currently small, so up to you
whether you want to separate it or not.
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1022
TEST(FindReferences, NeedsIndex) {
- const char *Header = "int foo();";
- Annotations Main("int main() { [[f^oo]](); }");
+ const char *Header = (R"cpp(
+int foo();
nit: redundant parentheses around `R"cpp`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72395/new/
https://reviews.llvm.org/D72395
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72395: [clangd] Publish xref for macros from Index and AST.
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/XRefs.cpp:463
+ }
+ // Query the index for references from other files.
+ if (Index && Results.References.size() < Limit) {
could we merge this and the code for decls by only populating `Req.IDs` with
`MacroSID` here and with the symbol id below and by finally making the query in
the end ? i.e.
```
RefsRequest Req;
Req.Limit = Limit;
if (macro) {
handleMainFileMacros;
Req.IDs.insert(*MacroID);
} else {
handleMainFileDecls;
populateReqIDs(Req);
}
handleIndexResults(Req);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72395/new/
https://reviews.llvm.org/D72395
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72395: [clangd] Publish xref for macros from Index and AST.
merge_guards_bot added a comment.
{icon check-circle color=green} Unit tests: pass. 61306 tests passed, 0 failed
and 736 were skipped.
{icon check-circle color=green} clang-tidy: pass.
{icon times-circle color=red} clang-format: fail. Please format your changes
with clang-format by running `git-clang-format HEAD^` or applying this patch
.
Build artifacts
:
diff.json
,
clang-tidy.txt
,
clang-format.patch
,
CMakeCache.txt
,
console-log.txt
,
test-results.xml
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72395/new/
https://reviews.llvm.org/D72395
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72395: [clangd] Publish xref for macros from Index and AST.
usaxena95 created this revision.
usaxena95 added a reviewer: kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
With this patch the `findReferences` API will return Xref for macros.
If the symbol under the cursor is a macro then we collect the references to it
from:
1. Main file by looking at the ParsedAST. (These were added to the ParsedAST in
https://reviews.llvm.org/D70008)
2. Files other than the mainfile by looking at the:
- static index (Added in https://reviews.llvm.org/D70489)
- file index (Added in https://reviews.llvm.org/D71406)
This patch collects all the xref from the above places and outputs it in
`findReferences` API.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D72395
Files:
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -937,6 +937,13 @@
[[CAT]](Fo, o) foo4;
}
)cpp",
+
+ R"cpp(// Macros
+#define [[MA^CRO]](X) (X+1)
+void test() {
+ int x = [[MACRO]]([[MACRO]](1));
+}
+ )cpp",
};
for (const char *Test : Tests) {
Annotations T(Test);
@@ -1012,8 +1019,16 @@
}
TEST(FindReferences, NeedsIndex) {
- const char *Header = "int foo();";
- Annotations Main("int main() { [[f^oo]](); }");
+ const char *Header = (R"cpp(
+int foo();
+#define MACRO(X) (X+1)
+ )cpp");
+ Annotations Main(R"cpp(
+int main() {
+ $foo[[f$foo^oo]]();
+ int a = $macro[[MA$macro^CRO]](1);
+}
+ )cpp");
TestTU TU;
TU.Code = Main.code();
TU.HeaderCode = Header;
@@ -1021,10 +1036,17 @@
// References in main file are returned without index.
EXPECT_THAT(
- findReferences(AST, Main.point(), 0, /*Index=*/nullptr).References,
- ElementsAre(RangeIs(Main.range(;
+ findReferences(AST, Main.point("foo"), 0, /*Index=*/nullptr).References,
+ ElementsAre(RangeIs(Main.range("foo";
+ EXPECT_THAT(
+ findReferences(AST, Main.point("macro"), 0, /*Index=*/nullptr).References,
+ ElementsAre(RangeIs(Main.range("macro";
+
Annotations IndexedMain(R"cpp(
-int main() { [[f^oo]](); }
+int indexed_main() {
+ $foo[[foo]]();
+ int a = $macro[[MACRO]](1);
+}
)cpp");
// References from indexed files are included.
@@ -1033,18 +1055,21 @@
IndexedTU.Filename = "Indexed.cpp";
IndexedTU.HeaderCode = Header;
EXPECT_THAT(
- findReferences(AST, Main.point(), 0, IndexedTU.index().get()).References,
- ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
+ findReferences(AST, Main.point("foo"), 0, IndexedTU.index().get()).References,
+ ElementsAre(RangeIs(Main.range("foo")), RangeIs(IndexedMain.range("foo";
+ EXPECT_THAT(
+ findReferences(AST, Main.point("macro"), 0, IndexedTU.index().get()).References,
+ ElementsAre(RangeIs(Main.range("macro")), RangeIs(IndexedMain.range("macro";
auto LimitRefs =
- findReferences(AST, Main.point(), /*Limit*/ 1, IndexedTU.index().get());
+ findReferences(AST, Main.point("foo"), /*Limit*/ 1, IndexedTU.index().get());
EXPECT_EQ(1u, LimitRefs.References.size());
EXPECT_TRUE(LimitRefs.HasMore);
// If the main file is in the index, we don't return duplicates.
// (even if the references are in a different location)
TU.Code = ("\n\n" + Main.code()).str();
- EXPECT_THAT(findReferences(AST, Main.point(), 0, TU.index().get()).References,
- ElementsAre(RangeIs(Main.range(;
+ EXPECT_THAT(findReferences(AST, Main.point("foo"), 0, TU.index().get()).References,
+ ElementsAre(RangeIs(Main.range("foo";
}
TEST(FindReferences, NoQueryForLocalSymbols) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -432,9 +432,48 @@
elog("Failed to get a path for the main file, so no references");
return Results;
}
+ auto URIMainFile = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
auto Loc = SM.getMacroArgExpandedLocation(
getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
- // TODO: should we handle macros, too?
+ auto AddRefFromIndex = [&](const Ref ) {
+// No need to continue process if we reach the limit.
+if (Results.References.size() > Limit)
+ return;
+auto LSPLoc = toLSPLocation(R.Location, *MainFilePath);
+// Avoid indexed results for the main file - the AST is authoritative.
+if (!LSPLoc || LSPLoc->uri.file() == *MainFilePath)
+ return;
+
+Results.References.push_back(std::move(*LSPLoc));
+ };
+ // Handle references to macro.
+ if
