[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa70161808bcd: [clangd] Include the underlying decls in
go-to-definition. (authored by hokein).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74054/new/
https://reviews.llvm.org/D74054
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
@@ -47,6 +47,11 @@
MATCHER_P2(FileRange, File, Range, "") {
return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
}
+MATCHER(DeclRange, "") {
+ const LocatedSymbol = ::testing::get<0>(arg);
+ const Range = ::testing::get<1>(arg);
+ return Sym.PreferredDeclaration.range == Range;
+}
// Extracts ranges from an annotated example, and constructs a matcher for a
// highlight set. Ranges should be named $read/$write as appropriate.
@@ -945,15 +950,77 @@
Sym("uniqueMethodName", Header.range("BarLoc"), llvm::None)));
}
-TEST(LocateSymbol, TemplateTypedefs) {
- auto T = Annotations(R"cpp(
-template struct function {};
-template using callback = function;
+TEST(LocateSymbol, Alias) {
+ const char *Tests[] = {
+R"cpp(
+ template struct function {};
+ template using [[callback]] = function;
-c^allback foo;
- )cpp");
- auto AST = TestTU::withCode(T.code()).build();
- EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+ c^allback foo;
+)cpp",
+
+// triggered on non-definition of a renaming alias: should not give any
+// underlying decls.
+R"cpp(
+ class Foo {};
+ typedef Foo [[Bar]];
+
+ B^ar b;
+)cpp",
+R"cpp(
+ class Foo {};
+ using [[Bar]] = Foo; // definition
+ Ba^r b;
+)cpp",
+
+// triggered on the underlying decl of a renaming alias.
+R"cpp(
+ class [[Foo]];
+ using Bar = Fo^o;
+)cpp",
+
+// triggered on definition of a non-renaming alias: should give underlying
+// decls.
+R"cpp(
+ namespace ns { class [[Foo]] {}; }
+ using ns::F^oo;
+)cpp",
+
+R"cpp(
+ namespace ns { int [[x]](char); int [[x]](double); }
+ using ns::^x;
+)cpp",
+
+R"cpp(
+ namespace ns { int [[x]](char); int x(double); }
+ using ns::x;
+ int y = ^x('a');
+)cpp",
+
+R"cpp(
+ namespace ns { class [[Foo]] {}; }
+ using ns::Foo;
+ F^oo f;
+)cpp",
+
+// other cases that don't matter much.
+R"cpp(
+ class Foo {};
+ typedef Foo [[Ba^r]];
+)cpp",
+R"cpp(
+ class Foo {};
+ using [[B^ar]] = Foo;
+)cpp",
+ };
+
+ for (const auto* Case : Tests) {
+SCOPED_TRACE(Case);
+auto T = Annotations(Case);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(locateSymbolAt(AST, T.point()),
+::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+ }
}
TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -290,6 +290,18 @@
}
}
+// Give the underlying decl if navigation is triggered on a non-renaming
+// alias.
+if (llvm::isa(D)) {
+ // FIXME: address more complicated cases. TargetDecl(... Underlying) gives
+ // all overload candidates, we only want the targeted one if the cursor is
+ // on an using-alias usage, workround it with getDeclAtPosition.
+ llvm::for_each(
+ getDeclAtPosition(AST, CurLoc, DeclRelation::Underlying, NodeKind),
+ [&](const NamedDecl *UD) { AddResultDecl(UD); });
+ continue;
+}
+
// Otherwise the target declaration is the right one.
AddResultDecl(D);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.
hokein updated this revision to Diff 283851.
hokein marked 2 inline comments as done.
hokein added a comment.
address comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74054/new/
https://reviews.llvm.org/D74054
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
@@ -47,6 +47,11 @@
MATCHER_P2(FileRange, File, Range, "") {
return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
}
+MATCHER(DeclRange, "") {
+ const LocatedSymbol = ::testing::get<0>(arg);
+ const Range = ::testing::get<1>(arg);
+ return Sym.PreferredDeclaration.range == Range;
+}
// Extracts ranges from an annotated example, and constructs a matcher for a
// highlight set. Ranges should be named $read/$write as appropriate.
@@ -945,15 +950,77 @@
Sym("uniqueMethodName", Header.range("BarLoc"), llvm::None)));
}
-TEST(LocateSymbol, TemplateTypedefs) {
- auto T = Annotations(R"cpp(
-template struct function {};
-template using callback = function;
+TEST(LocateSymbol, Alias) {
+ const char *Tests[] = {
+R"cpp(
+ template struct function {};
+ template using [[callback]] = function;
-c^allback foo;
- )cpp");
- auto AST = TestTU::withCode(T.code()).build();
- EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+ c^allback foo;
+)cpp",
+
+// triggered on non-definition of a renaming alias: should not give any
+// underlying decls.
+R"cpp(
+ class Foo {};
+ typedef Foo [[Bar]];
+
+ B^ar b;
+)cpp",
+R"cpp(
+ class Foo {};
+ using [[Bar]] = Foo; // definition
+ Ba^r b;
+)cpp",
+
+// triggered on the underlying decl of a renaming alias.
+R"cpp(
+ class [[Foo]];
+ using Bar = Fo^o;
+)cpp",
+
+// triggered on definition of a non-renaming alias: should give underlying
+// decls.
+R"cpp(
+ namespace ns { class [[Foo]] {}; }
+ using ns::F^oo;
+)cpp",
+
+R"cpp(
+ namespace ns { int [[x]](char); int [[x]](double); }
+ using ns::^x;
+)cpp",
+
+R"cpp(
+ namespace ns { int [[x]](char); int x(double); }
+ using ns::x;
+ int y = ^x('a');
+)cpp",
+
+R"cpp(
+ namespace ns { class [[Foo]] {}; }
+ using ns::Foo;
+ F^oo f;
+)cpp",
+
+// other cases that don't matter much.
+R"cpp(
+ class Foo {};
+ typedef Foo [[Ba^r]];
+)cpp",
+R"cpp(
+ class Foo {};
+ using [[B^ar]] = Foo;
+)cpp",
+ };
+
+ for (const auto* Case : Tests) {
+SCOPED_TRACE(Case);
+auto T = Annotations(Case);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(locateSymbolAt(AST, T.point()),
+::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+ }
}
TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -290,6 +290,18 @@
}
}
+// Give the underlying decl if navigation is triggered on a non-renaming
+// alias.
+if (llvm::isa(D)) {
+ // FIXME: address more complicated cases. TargetDecl(... Underlying) gives
+ // all overload candidates, we only want the targeted one if the cursor is
+ // on an using-alias usage, workround it with getDeclAtPosition.
+ llvm::for_each(
+ getDeclAtPosition(AST, CurLoc, DeclRelation::Underlying, NodeKind),
+ [&](const NamedDecl *UD) { AddResultDecl(UD); });
+ continue;
+}
+
// Otherwise the target declaration is the right one.
AddResultDecl(D);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Thanks!
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:997
+)cpp",
+R"cpp(
+ class Foo {};
this is basically the same as the above case (which is fine)
but what about `class [[Foo]]; using Bar = ^Foo;`?
(maybe this is tested elsewhere already, but it should be moved here)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74054/new/
https://reviews.llvm.org/D74054
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.
hokein updated this revision to Diff 270715.
hokein marked 2 inline comments as done.
hokein added a comment.
address review comment: fix the wrong result for multiple overloads.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74054/new/
https://reviews.llvm.org/D74054
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
@@ -46,6 +46,11 @@
MATCHER_P2(FileRange, File, Range, "") {
return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
}
+MATCHER(DeclRange, "") {
+ const LocatedSymbol = ::testing::get<0>(arg);
+ const Range = ::testing::get<1>(arg);
+ return Sym.PreferredDeclaration.range == Range;
+}
// Extracts ranges from an annotated example, and constructs a matcher for a
// highlight set. Ranges should be named $read/$write as appropriate.
@@ -937,15 +942,71 @@
Sym("uniqueMethodName", Header.range("BarLoc";
}
-TEST(LocateSymbol, TemplateTypedefs) {
- auto T = Annotations(R"cpp(
-template struct function {};
-template using callback = function;
+TEST(LocateSymbol, Alias) {
+ const char *Tests[] = {
+R"cpp(
+ template struct function {};
+ template using [[callback]] = function;
-c^allback foo;
- )cpp");
- auto AST = TestTU::withCode(T.code()).build();
- EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+ c^allback foo;
+)cpp",
+
+// triggered on non-definition of a renaming alias: should not give any
+// underlying decls.
+R"cpp(
+ class Foo {};
+ typedef Foo [[Bar]];
+
+ B^ar b;
+)cpp",
+R"cpp(
+ class Foo {};
+ using [[Bar]] = Foo; // definition
+ Ba^r b;
+)cpp",
+
+// triggered on definition of a non-renaming alias: should give underlying
+// decls.
+R"cpp(
+ namespace ns { class [[Foo]] {}; }
+ using ns::F^oo;
+)cpp",
+
+R"cpp(
+ namespace ns { int [[x]](char); int [[x]](double); }
+ using ns::^x;
+)cpp",
+
+R"cpp(
+ namespace ns { int [[x]](char); int x(double); }
+ using ns::x;
+ int y = ^x('a');
+)cpp",
+
+R"cpp(
+ namespace ns { class [[Foo]] {}; }
+ using ns::Foo;
+ F^oo f;
+)cpp",
+
+// other cases that don't matter much.
+R"cpp(
+ class Foo {};
+ typedef Foo [[Ba^r]];
+)cpp",
+R"cpp(
+ class Foo {};
+ using [[B^ar]] = Foo;
+)cpp",
+ };
+
+ for (const auto* Case : Tests) {
+SCOPED_TRACE(Case);
+auto T = Annotations(Case);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(locateSymbolAt(AST, T.point()),
+::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+ }
}
TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -286,6 +286,18 @@
}
}
+// Give the underlying decl if navigation is triggered on a non-renaming
+// alias.
+if (llvm::isa(D)) {
+ // FIXME: address more complicated cases. TargetDecl(... Underlying) gives
+ // all overload candidates, we only want the targeted one if the cursor is
+ // on an using-alias usage, workround it with getDeclAtPosition.
+ llvm::for_each(
+ getDeclAtPosition(AST, CurLoc, DeclRelation::Underlying, NodeKind),
+ [&](const NamedDecl *UD) { AddResultDecl(UD); });
+ continue;
+}
+
// Otherwise the target declaration is the right one.
AddResultDecl(D);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.
sammccall added a comment.
Sorry about being late coming back to this.
Comment at: clang-tools-extra/clangd/XRefs.cpp:307
+// alias.
+if (llvm::isa(D)) {
+ llvm::for_each(targetDecl(ast_type_traits::DynTypedNode::create(*D),
I believe this is going to do the wrong thing on overload sets.
```
namespace ns { int x(char); int x(double); }
using ns::x;
int y = ^x('a');
```
getDeclAtPosition(..., TemplatePattern|Alias) will yield UsingDecl, and then
targetDecl(UsingDecl, Underlying) will yield both overloads of x.
However getDeclAtPosition(..., Underlying) would correctly return only the
desired overload.
(Please add a test like this!)
This is awkward to work around, it's a failure of the targetDecl API. Maybe for
now, call getDeclAtPosition with TemplatePattern|Alias, and if there's exactly
one result, replace D with it? And leave a fixme to address more complicated
cases somehow.
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:695
+ namespace ns { class [[Foo]] {}; }
+ using ns::[[F^oo]];
+)cpp",
nridge wrote:
> Why is it useful to give the using-declaration itself as a result?
>
> It seems like the more useful behaviour from the user's point of view is to
> jump directly to the definition of `class Foo`, without having to choose from
> a popup first.
+1, that would mean adding a continue after the addresultdecl
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74054/new/
https://reviews.llvm.org/D74054
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.
nridge added inline comments.
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:695
+ namespace ns { class [[Foo]] {}; }
+ using ns::[[F^oo]];
+)cpp",
Why is it useful to give the using-declaration itself as a result?
It seems like the more useful behaviour from the user's point of view is to
jump directly to the definition of `class Foo`, without having to choose from a
popup first.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74054/new/
https://reviews.llvm.org/D74054
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.
hokein added a comment. friendly ping, this patch should be ready for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74054/new/ https://reviews.llvm.org/D74054 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.
hokein updated this revision to Diff 242856.
hokein added a comment.
remove an accident change.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74054/new/
https://reviews.llvm.org/D74054
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
@@ -45,6 +45,11 @@
MATCHER_P2(FileRange, File, Range, "") {
return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
}
+MATCHER(DeclRange, "") {
+ const LocatedSymbol = ::testing::get<0>(arg);
+ const Range = ::testing::get<1>(arg);
+ return Sym.PreferredDeclaration.range == Range;
+}
// Extracts ranges from an annotated example, and constructs a matcher for a
// highlight set. Ranges should be named $read/$write as appropriate.
@@ -660,15 +665,59 @@
Sym("baz", T.range("StaticOverload2";
}
-TEST(LocateSymbol, TemplateTypedefs) {
- auto T = Annotations(R"cpp(
-template struct function {};
-template using callback = function;
+TEST(LocateSymbol, Alias) {
+ const char *Tests[] = {
+R"cpp(
+ template struct function {};
+ template using [[callback]] = function;
-c^allback foo;
- )cpp");
- auto AST = TestTU::withCode(T.code()).build();
- EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+ c^allback foo;
+)cpp",
+
+// triggered on non-definition of a renaming alias: should not give any
+// underlying decls.
+R"cpp(
+ class Foo {};
+ typedef Foo [[Bar]];
+
+ B^ar b;
+)cpp",
+R"cpp(
+ class Foo {};
+ using [[Bar]] = Foo; // definition
+ Ba^r b;
+)cpp",
+
+// triggered on definition of a non-renaming alias: should give underlying
+// decls.
+R"cpp(
+ namespace ns { class [[Foo]] {}; }
+ using ns::[[F^oo]];
+)cpp",
+
+// other cases that don't matter much.
+R"cpp(
+ class Foo {};
+ typedef Foo [[Ba^r]];
+)cpp",
+R"cpp(
+ class Foo {};
+ using [[B^ar]] = Foo;
+)cpp",
+R"cpp(
+ namespace ns { class [[Foo]] {}; }
+ using ns::Foo;
+ F^oo f;
+)cpp",
+ };
+
+ for (const auto* Case : Tests) {
+SCOPED_TRACE(Case);
+auto T = Annotations(Case);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(locateSymbolAt(AST, T.point()),
+::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+ }
}
TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -21,6 +21,7 @@
#include "index/Relation.h"
#include "index/SymbolLocation.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
#include "clang/AST/Attr.h"
#include "clang/AST/Attrs.inc"
#include "clang/AST/Decl.h"
@@ -301,6 +302,14 @@
}
}
+// Give the underlying decla if navigation is triggered on a non-renaming
+// alias.
+if (llvm::isa(D)) {
+ llvm::for_each(targetDecl(ast_type_traits::DynTypedNode::create(*D),
+DeclRelation::Underlying),
+ [&](const NamedDecl *UD) { AddResultDecl(UD); });
+}
+
// Otherwise the target declaration is the right one.
AddResultDecl(D);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.
hokein updated this revision to Diff 242855.
hokein added a comment.
no regression on non-definition of renaming alias.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74054/new/
https://reviews.llvm.org/D74054
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
@@ -45,6 +45,11 @@
MATCHER_P2(FileRange, File, Range, "") {
return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
}
+MATCHER(DeclRange, "") {
+ const LocatedSymbol = ::testing::get<0>(arg);
+ const Range = ::testing::get<1>(arg);
+ return Sym.PreferredDeclaration.range == Range;
+}
// Extracts ranges from an annotated example, and constructs a matcher for a
// highlight set. Ranges should be named $read/$write as appropriate.
@@ -660,17 +665,72 @@
Sym("baz", T.range("StaticOverload2";
}
-TEST(LocateSymbol, TemplateTypedefs) {
- auto T = Annotations(R"cpp(
-template struct function {};
-template using callback = function;
+TEST(LocateSymbol, Alias) {
+ const char *Tests[] = {
+R"cpp(
+ template struct function {};
+ template using [[callback]] = function;
-c^allback foo;
- )cpp");
- auto AST = TestTU::withCode(T.code()).build();
- EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+ c^allback foo;
+)cpp",
+
+// triggered on non-definition of a renaming alias: should not give any
+// underlying decls.
+R"cpp(
+ class Foo {};
+ typedef Foo [[Bar]];
+
+ B^ar b;
+)cpp",
+R"cpp(
+ class Foo {};
+ using [[Bar]] = Foo; // definition
+ Ba^r b;
+)cpp",
+
+// triggered on definition of a non-renaming alias: should give underlying
+// decls.
+R"cpp(
+ namespace ns { class [[Foo]] {}; }
+ using ns::[[F^oo]];
+)cpp",
+
+// other cases that don't matter much.
+R"cpp(
+ class Foo {};
+ typedef Foo [[Ba^r]];
+)cpp",
+R"cpp(
+ class Foo {};
+ using [[B^ar]] = Foo;
+)cpp",
+R"cpp(
+ namespace ns { class [[Foo]] {}; }
+ using ns::Foo;
+ F^oo f;
+)cpp",
+ };
+
+ for (const auto* Case : Tests) {
+SCOPED_TRACE(Case);
+auto T = Annotations(Case);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(locateSymbolAt(AST, T.point()),
+::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+ }
}
+// TEST(LocateSymbol, TemplateTypedefs) {
+// auto T = Annotations(R"cpp(
+// template struct function {};
+// template using callback = function;
+
+// c^allback foo;
+// )cpp");
+// auto AST = TestTU::withCode(T.code()).build();
+// EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+// }
+
TEST(LocateSymbol, RelPathsInCompileCommand) {
// The source is in "/clangd-test/src".
// We build in "/clangd-test/build".
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -21,6 +21,7 @@
#include "index/Relation.h"
#include "index/SymbolLocation.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
#include "clang/AST/Attr.h"
#include "clang/AST/Attrs.inc"
#include "clang/AST/Decl.h"
@@ -301,6 +302,14 @@
}
}
+// Give the underlying decla if navigation is triggered on a non-renaming
+// alias.
+if (llvm::isa(D)) {
+ llvm::for_each(targetDecl(ast_type_traits::DynTypedNode::create(*D),
+DeclRelation::Underlying),
+ [&](const NamedDecl *UD) { AddResultDecl(UD); });
+}
+
// Otherwise the target declaration is the right one.
AddResultDecl(D);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.
hokein planned changes to this revision.
hokein marked an inline comment as done.
hokein added inline comments.
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:674
-c^allback foo;
- )cpp");
- auto AST = TestTU::withCode(T.code()).build();
- EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+ c^allback foo;
+)cpp",
we need to avoid the underlying decl in this case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74054/new/
https://reviews.llvm.org/D74054
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.
merge_guards_bot added a comment.
{icon question-circle color=gray} Unit tests: unknown.
{icon question-circle color=gray} clang-tidy: unknown.
{icon question-circle color=gray} clang-format: unknown.
Build artifacts
:
CMakeCache.txt
,
console-log.txt
//Pre-merge checks is in beta. Report issue
.
Please join beta .//
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74054/new/
https://reviews.llvm.org/D74054
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
Fixes https://github.com/clangd/clangd/issues/277
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D74054
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
@@ -45,6 +45,11 @@
MATCHER_P2(FileRange, File, Range, "") {
return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
}
+MATCHER(DeclRange, "") {
+ const LocatedSymbol = ::testing::get<0>(arg);
+ const Range = ::testing::get<1>(arg);
+ return Sym.PreferredDeclaration.range == Range;
+}
// Extracts ranges from an annotated example, and constructs a matcher for a
// highlight set. Ranges should be named $read/$write as appropriate.
@@ -660,15 +665,38 @@
Sym("baz", T.range("StaticOverload2";
}
-TEST(LocateSymbol, TemplateTypedefs) {
- auto T = Annotations(R"cpp(
-template struct function {};
-template using callback = function;
+TEST(LocateSymbol, Alias) {
+ const char *Tests[] = {
+R"cpp(
+ template struct [[function]] {};
+ template using [[callback]] = function;
-c^allback foo;
- )cpp");
- auto AST = TestTU::withCode(T.code()).build();
- EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+ c^allback foo;
+)cpp",
+
+R"cpp(
+ class [[Foo]] {};
+ typedef Foo [[Ba^r]];
+)cpp",
+
+R"cpp(
+ namespace ns { class [[Foo]] {}; }
+ using ns::[[F^oo]];
+)cpp",
+
+R"cpp(
+ class [[Foo]] {};
+ using [[B^ar]] = Foo;
+)cpp",
+ };
+
+ for (const auto* Case : Tests) {
+SCOPED_TRACE(Case);
+auto T = Annotations(Case);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(locateSymbolAt(AST, T.point()),
+::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+ }
}
TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -273,8 +273,8 @@
};
// Emit all symbol locations (declaration or definition) from AST.
- DeclRelationSet Relations =
- DeclRelation::TemplatePattern | DeclRelation::Alias;
+ DeclRelationSet Relations = DeclRelation::TemplatePattern |
+ DeclRelation::Alias | DeclRelation::Underlying;
for (const NamedDecl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
// Special case: void foo() ^override: jump to the overridden method.
if (const auto *CMD = llvm::dyn_cast(D)) {
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -45,6 +45,11 @@
MATCHER_P2(FileRange, File, Range, "") {
return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
}
+MATCHER(DeclRange, "") {
+ const LocatedSymbol = ::testing::get<0>(arg);
+ const Range = ::testing::get<1>(arg);
+ return Sym.PreferredDeclaration.range == Range;
+}
// Extracts ranges from an annotated example, and constructs a matcher for a
// highlight set. Ranges should be named $read/$write as appropriate.
@@ -660,15 +665,38 @@
Sym("baz", T.range("StaticOverload2";
}
-TEST(LocateSymbol, TemplateTypedefs) {
- auto T = Annotations(R"cpp(
-template struct function {};
-template using callback = function;
+TEST(LocateSymbol, Alias) {
+ const char *Tests[] = {
+R"cpp(
+ template struct [[function]] {};
+ template using [[callback]] = function;
-c^allback foo;
- )cpp");
- auto AST = TestTU::withCode(T.code()).build();
- EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+ c^allback foo;
+)cpp",
+
+R"cpp(
+ class [[Foo]] {};
+ typedef Foo [[Ba^r]];
+)cpp",
+
+R"cpp(
+ namespace ns { class [[Foo]] {}; }
+ using ns::[[F^oo]];
+)cpp",
+
+R"cpp(
+ class [[Foo]] {};
+ using [[B^ar]] = Foo;
+)cpp",
+ };
+
+ for (const auto* Case : Tests) {
+SCOPED_TRACE(Case);
+auto T = Annotations(Case);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(locateSymbolAt(AST, T.point()),
+::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+ }
}
TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: clang-tools-extra/clangd/XRefs.cpp
