sammccall updated this revision to Diff 184096.
sammccall added a comment.
Add server capability declarationProvider.
Technically an extension, but that's probably just a protocol bug.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57388/new/
https://reviews.llvm.org/D57388
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/XRefs.cpp
clangd/XRefs.h
test/clangd/initialize-params.test
unittests/clangd/ClangdTests.cpp
unittests/clangd/SyncAPI.cpp
unittests/clangd/SyncAPI.h
unittests/clangd/TUSchedulerTests.cpp
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===================================================================
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -17,6 +17,7 @@
#include "index/SymbolCollector.h"
#include "clang/Index/IndexingAction.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -25,7 +26,6 @@
namespace {
using testing::ElementsAre;
-using testing::Field;
using testing::IsEmpty;
using testing::Matcher;
using testing::UnorderedElementsAreArray;
@@ -95,9 +95,35 @@
}
}
+MATCHER_P3(Sym, Name, Decl, DefOrNone, "") {
+ llvm::Optional<Range> Def = DefOrNone;
+ if (Name != arg.Name) {
+ *result_listener << "Name is " << arg.Name;
+ return false;
+ }
+ if (Decl != arg.PreferredDeclaration.range) {
+ *result_listener << "Declaration is "
+ << llvm::to_string(arg.PreferredDeclaration);
+ return false;
+ }
+ if (Def && !arg.Definition) {
+ *result_listener << "Has no definition";
+ return false;
+ }
+ if (Def && arg.Definition->range != *Def) {
+ *result_listener << "Definition is " << llvm::to_string(arg.Definition);
+ return false;
+ }
+ return true;
+}
+testing::Matcher<LocatedSymbol> Sym(std::string Name, Range Decl) {
+ return Sym(Name, Decl, llvm::None);
+}
+MATCHER_P(Sym, Name, "") { return arg.Name == Name; }
+
MATCHER_P(RangeIs, R, "") { return arg.range == R; }
-TEST(GoToDefinition, WithIndex) {
+TEST(LocateSymbol, WithIndex) {
Annotations SymbolHeader(R"cpp(
class $forward[[Forward]];
class $foo[[Foo]] {};
@@ -115,9 +141,9 @@
TU.Code = SymbolCpp.code();
TU.HeaderCode = SymbolHeader.code();
auto Index = TU.index();
- auto runFindDefinitionsWithIndex = [&Index](const Annotations &Main) {
+ auto LocateWithIndex = [&Index](const Annotations &Main) {
auto AST = TestTU::withCode(Main.code()).build();
- return clangd::findDefinitions(AST, Main.point(), Index.get());
+ return clangd::locateSymbolAt(AST, Main.point(), Index.get());
};
Annotations Test(R"cpp(// only declaration in AST.
@@ -126,9 +152,8 @@
^f1();
}
)cpp");
- EXPECT_THAT(runFindDefinitionsWithIndex(Test),
- testing::ElementsAreArray(
- {RangeIs(SymbolCpp.range("f1")), RangeIs(Test.range())}));
+ EXPECT_THAT(LocateWithIndex(Test),
+ ElementsAre(Sym("f1", Test.range(), SymbolCpp.range("f1"))));
Test = Annotations(R"cpp(// definition in AST.
void [[f1]]() {}
@@ -136,30 +161,30 @@
^f1();
}
)cpp");
- EXPECT_THAT(runFindDefinitionsWithIndex(Test),
- testing::ElementsAreArray(
- {RangeIs(Test.range()), RangeIs(SymbolHeader.range("f1"))}));
+ EXPECT_THAT(LocateWithIndex(Test),
+ ElementsAre(Sym("f1", SymbolHeader.range("f1"), Test.range())));
Test = Annotations(R"cpp(// forward declaration in AST.
class [[Foo]];
F^oo* create();
)cpp");
- EXPECT_THAT(runFindDefinitionsWithIndex(Test),
- testing::ElementsAreArray(
- {RangeIs(SymbolHeader.range("foo")), RangeIs(Test.range())}));
+ EXPECT_THAT(LocateWithIndex(Test),
+ ElementsAre(Sym("Foo", Test.range(), SymbolHeader.range("foo"))));
Test = Annotations(R"cpp(// defintion in AST.
class [[Forward]] {};
F^orward create();
)cpp");
- EXPECT_THAT(runFindDefinitionsWithIndex(Test),
- testing::ElementsAreArray({
- RangeIs(Test.range()),
- RangeIs(SymbolHeader.range("forward")),
- }));
+ EXPECT_THAT(
+ LocateWithIndex(Test),
+ ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range())));
}
-TEST(GoToDefinition, All) {
+TEST(LocateSymbol, All) {
+ // Ranges in tests:
+ // $decl is the declaration location (if absent, no symbol is located)
+ // $def is the definition location (if absent, symbol has no definition)
+ // unnamed range becomes both $decl and $def.
const char *Tests[] = {
R"cpp(// Local variable
int main() {
@@ -186,7 +211,7 @@
)cpp",
R"cpp(// Function declaration via call
- int [[foo]](int);
+ int $decl[[foo]](int);
int main() {
return ^foo(42);
}
@@ -222,7 +247,7 @@
)cpp",
R"cpp(// Method call
- struct Foo { int [[x]](); };
+ struct Foo { int $decl[[x]](); };
int main() {
Foo bar;
bar.^x();
@@ -230,7 +255,7 @@
)cpp",
R"cpp(// Typedef
- typedef int [[Foo]];
+ typedef int $decl[[Foo]];
int main() {
^Foo bar;
}
@@ -243,7 +268,7 @@
)cpp", */
R"cpp(// Namespace
- namespace [[ns]] {
+ namespace $decl[[ns]] {
struct Foo { static void bar(); }
} // namespace ns
int main() { ^ns::Foo::bar(); }
@@ -304,30 +329,45 @@
};
for (const char *Test : Tests) {
Annotations T(Test);
+ llvm::Optional<Range> WantDecl;
+ llvm::Optional<Range> WantDef;
+ if (!T.ranges().empty())
+ WantDecl = WantDef = T.range();
+ if (!T.ranges("decl").empty())
+ WantDecl = T.range("decl");
+ if (!T.ranges("def").empty())
+ WantDecl = T.range("def");
+
auto AST = TestTU::withCode(T.code()).build();
- std::vector<Matcher<Location>> ExpectedLocations;
- for (const auto &R : T.ranges())
- ExpectedLocations.push_back(RangeIs(R));
- EXPECT_THAT(findDefinitions(AST, T.point()),
- ElementsAreArray(ExpectedLocations))
- << Test;
+ auto Results = locateSymbolAt(AST, T.point());
+
+ if (!WantDecl) {
+ EXPECT_THAT(Results, IsEmpty()) << Test;
+ } else {
+ ASSERT_THAT(Results, ::testing::SizeIs(1)) << Test;
+ EXPECT_EQ(Results[0].PreferredDeclaration.range, *WantDecl) << Test;
+ llvm::Optional<Range> GotDef;
+ if (Results[0].Definition)
+ GotDef = Results[0].Definition->range;
+ EXPECT_EQ(WantDef, GotDef) << Test;
+ }
}
}
-TEST(GoToDefinition, Rank) {
+TEST(LocateSymbol, Ambiguous) {
auto T = Annotations(R"cpp(
- struct $foo1[[Foo]] {
- $foo2[[Foo]]();
- $foo3[[Foo]](Foo&&);
- $foo4[[Foo]](const char*);
+ struct Foo {
+ Foo();
+ Foo(Foo&&);
+ Foo(const char*);
};
- Foo $f[[f]]();
+ Foo f();
- void $g[[g]](Foo foo);
+ void g(Foo foo);
void call() {
- const char* $str[[str]] = "123";
+ const char* str = "123";
Foo a = $1^str;
Foo b = Foo($2^str);
Foo c = $3^f();
@@ -336,26 +376,20 @@
}
)cpp");
auto AST = TestTU::withCode(T.code()).build();
- EXPECT_THAT(findDefinitions(AST, T.point("1")),
- ElementsAre(RangeIs(T.range("str")), RangeIs(T.range("foo4"))));
- EXPECT_THAT(findDefinitions(AST, T.point("2")),
- ElementsAre(RangeIs(T.range("str"))));
- EXPECT_THAT(findDefinitions(AST, T.point("3")),
- ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3"))));
- EXPECT_THAT(findDefinitions(AST, T.point("4")),
- ElementsAre(RangeIs(T.range("g"))));
- EXPECT_THAT(findDefinitions(AST, T.point("5")),
- ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3"))));
-
- auto DefinitionAtPoint6 = findDefinitions(AST, T.point("6"));
- EXPECT_EQ(3ul, DefinitionAtPoint6.size());
- EXPECT_THAT(DefinitionAtPoint6, HasSubsequence(RangeIs(T.range("str")),
- RangeIs(T.range("foo4"))));
- EXPECT_THAT(DefinitionAtPoint6, HasSubsequence(RangeIs(T.range("str")),
- RangeIs(T.range("foo3"))));
+ // Ordered assertions are deliberate: we expect a predictable order.
+ EXPECT_THAT(locateSymbolAt(AST, T.point("1")),
+ ElementsAre(Sym("str"), Sym("Foo")));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("2")), ElementsAre(Sym("str")));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("3")),
+ ElementsAre(Sym("f"), Sym("Foo")));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g")));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("5")),
+ ElementsAre(Sym("f"), Sym("Foo")));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("6")),
+ ElementsAre(Sym("str"), Sym("Foo"), Sym("Foo")));
}
-TEST(GoToDefinition, RelPathsInCompileCommand) {
+TEST(LocateSymbol, RelPathsInCompileCommand) {
// The source is in "/clangd-test/src".
// We build in "/clangd-test/build".
@@ -397,24 +431,23 @@
// Go to a definition in main source file.
auto Locations =
- runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p1"));
+ runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("p1"));
EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
- EXPECT_THAT(*Locations,
- ElementsAre(FileRange(FooCpp, SourceAnnotations.range())));
+ EXPECT_THAT(*Locations, ElementsAre(Sym("foo", SourceAnnotations.range())));
// Go to a definition in header_in_preamble.h.
- Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p2"));
+ Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("p2"));
EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
- EXPECT_THAT(*Locations,
- ElementsAre(FileRange(HeaderInPreambleH,
- HeaderInPreambleAnnotations.range())));
+ EXPECT_THAT(
+ *Locations,
+ ElementsAre(Sym("bar_preamble", HeaderInPreambleAnnotations.range())));
// Go to a definition in header_not_in_preamble.h.
- Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p3"));
+ Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("p3"));
EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
EXPECT_THAT(*Locations,
- ElementsAre(FileRange(HeaderNotInPreambleH,
- HeaderNotInPreambleAnnotations.range())));
+ ElementsAre(Sym("bar_not_preamble",
+ HeaderNotInPreambleAnnotations.range())));
}
TEST(Hover, All) {
@@ -1061,46 +1094,39 @@
Server.addDocument(FooCpp, SourceAnnotations.code());
// Test include in preamble.
- auto Locations =
- runFindDefinitions(Server, FooCpp, SourceAnnotations.point());
- ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
- EXPECT_THAT(*Locations,
- ElementsAre(FileRange(FooH, HeaderAnnotations.range())));
+ auto Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point());
+ ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+ EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range())));
// Test include in preamble, last char.
- Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("2"));
- ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
- EXPECT_THAT(*Locations,
- ElementsAre(FileRange(FooH, HeaderAnnotations.range())));
+ Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("2"));
+ ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+ EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range())));
- Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("3"));
- ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
- EXPECT_THAT(*Locations,
- ElementsAre(FileRange(FooH, HeaderAnnotations.range())));
+ Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("3"));
+ ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+ EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range())));
// Test include outside of preamble.
- Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("6"));
- ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
- EXPECT_THAT(*Locations,
- ElementsAre(FileRange(FooH, HeaderAnnotations.range())));
+ Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("6"));
+ ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+ EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range())));
// Test a few positions that do not result in Locations.
- Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("4"));
- ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+ Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("4"));
+ ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
EXPECT_THAT(*Locations, IsEmpty());
- Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("5"));
- ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
- EXPECT_THAT(*Locations,
- ElementsAre(FileRange(FooH, HeaderAnnotations.range())));
+ Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("5"));
+ ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+ EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range())));
- Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("7"));
- ASSERT_TRUE(bool(Locations)) << "findDefinitions returned an error";
- EXPECT_THAT(*Locations,
- ElementsAre(FileRange(FooH, HeaderAnnotations.range())));
+ Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("7"));
+ ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+ EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range())));
}
-TEST(GoToDefinition, WithPreamble) {
+TEST(LocateSymbol, WithPreamble) {
// Test stragety: AST should always use the latest preamble instead of last
// good preamble.
MockFSProvider FS;
@@ -1120,18 +1146,18 @@
FS.Files[FooH] = FooHeader.code();
runAddDocument(Server, FooCpp, FooWithHeader.code());
- // GoToDefinition goes to a #include file: the result comes from the preamble.
+ // LocateSymbol goes to a #include file: the result comes from the preamble.
EXPECT_THAT(
- cantFail(runFindDefinitions(Server, FooCpp, FooWithHeader.point())),
- ElementsAre(FileRange(FooH, FooHeader.range())));
+ cantFail(runLocateSymbolAt(Server, FooCpp, FooWithHeader.point())),
+ ElementsAre(Sym("foo.h", FooHeader.range())));
// Only preamble is built, and no AST is built in this request.
Server.addDocument(FooCpp, FooWithoutHeader.code(), WantDiagnostics::No);
// We build AST here, and it should use the latest preamble rather than the
// stale one.
EXPECT_THAT(
- cantFail(runFindDefinitions(Server, FooCpp, FooWithoutHeader.point())),
- ElementsAre(FileRange(FooCpp, FooWithoutHeader.range())));
+ cantFail(runLocateSymbolAt(Server, FooCpp, FooWithoutHeader.point())),
+ ElementsAre(Sym("foo", FooWithoutHeader.range())));
// Reset test environment.
runAddDocument(Server, FooCpp, FooWithHeader.code());
@@ -1139,8 +1165,8 @@
Server.addDocument(FooCpp, FooWithoutHeader.code(), WantDiagnostics::Yes);
// Use the AST being built in above request.
EXPECT_THAT(
- cantFail(runFindDefinitions(Server, FooCpp, FooWithoutHeader.point())),
- ElementsAre(FileRange(FooCpp, FooWithoutHeader.range())));
+ cantFail(runLocateSymbolAt(Server, FooCpp, FooWithoutHeader.point())),
+ ElementsAre(Sym("foo", FooWithoutHeader.range())));
}
TEST(FindReferences, WithinAST) {
Index: unittests/clangd/TUSchedulerTests.cpp
===================================================================
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -685,10 +685,10 @@
// We schedule the following tasks in the queue:
// [Update] [GoToDefinition]
Server.addDocument(testPath("foo.cpp"), Code.code(), WantDiagnostics::Yes);
- Server.findDefinitions(testPath("foo.cpp"), Code.point(),
- [](Expected<std::vector<Location>> Result) {
- ASSERT_TRUE((bool)Result);
- });
+ Server.locateSymbolAt(testPath("foo.cpp"), Code.point(),
+ [](Expected<std::vector<LocatedSymbol>> Result) {
+ ASSERT_TRUE((bool)Result);
+ });
ASSERT_TRUE(Server.blockUntilIdleForTest());
Index: unittests/clangd/SyncAPI.h
===================================================================
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -32,8 +32,8 @@
llvm::Expected<SignatureHelp> runSignatureHelp(ClangdServer &Server,
PathRef File, Position Pos);
-llvm::Expected<std::vector<Location>>
-runFindDefinitions(ClangdServer &Server, PathRef File, Position Pos);
+llvm::Expected<std::vector<LocatedSymbol>>
+runLocateSymbolAt(ClangdServer &Server, PathRef File, Position Pos);
llvm::Expected<std::vector<DocumentHighlight>>
runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
Index: unittests/clangd/SyncAPI.cpp
===================================================================
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -84,10 +84,10 @@
return std::move(*Result);
}
-llvm::Expected<std::vector<Location>>
-runFindDefinitions(ClangdServer &Server, PathRef File, Position Pos) {
- llvm::Optional<llvm::Expected<std::vector<Location>>> Result;
- Server.findDefinitions(File, Pos, capture(Result));
+llvm::Expected<std::vector<LocatedSymbol>>
+runLocateSymbolAt(ClangdServer &Server, PathRef File, Position Pos) {
+ llvm::Optional<llvm::Expected<std::vector<LocatedSymbol>>> Result;
+ Server.locateSymbolAt(File, Pos, capture(Result));
return std::move(*Result);
}
Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -43,8 +43,9 @@
using ::testing::Pair;
using ::testing::UnorderedElementsAre;
-MATCHER_P2(FileRange, File, Range, "") {
- return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
+MATCHER_P2(DeclAt, File, Range, "") {
+ return arg.PreferredDeclaration ==
+ Location{URIForFile::canonicalize(File, testRoot()), Range};
}
bool diagsContainErrors(const std::vector<Diag> &Diagnostics) {
@@ -458,10 +459,9 @@
UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, true),
Pair(BazCpp, false)));
- auto Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
+ auto Locations = runLocateSymbolAt(Server, FooCpp, FooSource.point());
EXPECT_TRUE(bool(Locations));
- EXPECT_THAT(*Locations,
- ElementsAre(FileRange(FooCpp, FooSource.range("one"))));
+ EXPECT_THAT(*Locations, ElementsAre(DeclAt(FooCpp, FooSource.range("one"))));
// Undefine MACRO, close baz.cpp.
CDB.ExtraClangFlags.clear();
@@ -474,10 +474,9 @@
EXPECT_THAT(DiagConsumer.filesWithDiags(),
UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, false)));
- Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
+ Locations = runLocateSymbolAt(Server, FooCpp, FooSource.point());
EXPECT_TRUE(bool(Locations));
- EXPECT_THAT(*Locations,
- ElementsAre(FileRange(FooCpp, FooSource.range("two"))));
+ EXPECT_THAT(*Locations, ElementsAre(DeclAt(FooCpp, FooSource.range("two"))));
}
TEST_F(ClangdVFSTest, MemoryUsage) {
@@ -532,7 +531,7 @@
runAddDocument(Server, FooCpp, "int main() {}");
EXPECT_EQ(runDumpAST(Server, FooCpp), "<no-ast>");
- EXPECT_ERROR(runFindDefinitions(Server, FooCpp, Position()));
+ EXPECT_ERROR(runLocateSymbolAt(Server, FooCpp, Position()));
EXPECT_ERROR(runFindDocumentHighlights(Server, FooCpp, Position()));
EXPECT_ERROR(runRename(Server, FooCpp, Position(), "new_name"));
// FIXME: codeComplete and signatureHelp should also return errors when they
@@ -717,7 +716,7 @@
clangd::CodeCompleteOptions()));
};
- auto FindDefinitionsRequest = [&]() {
+ auto LocateSymbolRequest = [&]() {
unsigned FileIndex = FileIndexDist(RandGen);
// Make sure we don't violate the ClangdServer's contract.
if (ReqStats[FileIndex].FileIsRemoved)
@@ -727,13 +726,13 @@
Pos.line = LineDist(RandGen);
Pos.character = ColumnDist(RandGen);
- ASSERT_TRUE(!!runFindDefinitions(Server, FilePaths[FileIndex], Pos));
+ ASSERT_TRUE(!!runLocateSymbolAt(Server, FilePaths[FileIndex], Pos));
};
std::vector<std::function<void()>> AsyncRequests = {
AddDocumentRequest, ForceReparseRequest, RemoveDocumentRequest};
std::vector<std::function<void()>> BlockingRequests = {
- CodeCompletionRequest, FindDefinitionsRequest};
+ CodeCompletionRequest, LocateSymbolRequest};
// Bash requests to ClangdServer in a loop.
std::uniform_int_distribution<int> AsyncRequestIndexDist(
Index: test/clangd/initialize-params.test
===================================================================
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -14,6 +14,7 @@
# CHECK-NEXT: ":"
# CHECK-NEXT: ]
# CHECK-NEXT: },
+# CHECK-NEXT: "declarationProvider": true,
# CHECK-NEXT: "definitionProvider": true,
# CHECK-NEXT: "documentFormattingProvider": true,
# CHECK-NEXT: "documentHighlightProvider": true,
Index: clangd/XRefs.h
===================================================================
--- clangd/XRefs.h
+++ clangd/XRefs.h
@@ -22,9 +22,24 @@
namespace clang {
namespace clangd {
+// Describes where a symbol is declared and defined (as far as clangd knows).
+// There are three cases:
+// - a declaration only, no definition is known (e.g. only header seen)
+// - a declaration and a distinct definition (e.g. function declared in header)
+// - a declaration and an equal definition (e.g. inline function, or class)
+struct LocatedSymbol {
+ // The (unqualified) name of the symbol.
+ std::string Name;
+ // The canonical or best declaration: where most users find its interface.
+ Location PreferredDeclaration;
+ // Where the symbol is defined, if known. May equal PreferredDeclaration.
+ llvm::Optional<Location> Definition;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const LocatedSymbol &);
/// Get definition of symbol at a specified \p Pos.
-std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos,
- const SymbolIndex *Index = nullptr);
+/// Multiple locations may be returned, corresponding to distinct symbols.
+std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
+ const SymbolIndex *Index = nullptr);
/// Returns highlights for all usages of a symbol at \p Pos.
std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
Index: clangd/XRefs.cpp
===================================================================
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -21,18 +21,24 @@
namespace clangd {
namespace {
-// Get the definition from a given declaration `D`.
-// Return nullptr if no definition is found, or the declaration type of `D` is
-// not supported.
+// Returns the single definition of the entity declared by D, if visible.
+// In particular:
+// - for non-redeclarable kinds (e.g. local vars), return D
+// - for kinds that allow multiple definitions (e.g. namespaces), return nullptr
const Decl *getDefinition(const Decl *D) {
assert(D);
+ // Decl has one definition that we can find.
if (const auto *TD = dyn_cast<TagDecl>(D))
return TD->getDefinition();
- else if (const auto *VD = dyn_cast<VarDecl>(D))
+ if (const auto *VD = dyn_cast<VarDecl>(D))
return VD->getDefinition();
- else if (const auto *FD = dyn_cast<FunctionDecl>(D))
+ if (const auto *FD = dyn_cast<FunctionDecl>(D))
return FD->getDefinition();
- return nullptr;
+ // Only a single declaration is allowed.
+ if (isa<ValueDecl>(D)) // except cases above
+ return D;
+ // Multiple definitions are allowed.
+ return nullptr; // except cases above
}
void logIfOverflow(const SymbolLocation &Loc) {
@@ -240,8 +246,8 @@
} // namespace
-std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos,
- const SymbolIndex *Index) {
+std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
+ const SymbolIndex *Index) {
const auto &SM = AST.getASTContext().getSourceManager();
auto MainFilePath =
getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
@@ -250,114 +256,84 @@
return {};
}
- std::vector<Location> Result;
- // Handle goto definition for #include.
+ // Treat #included files as symbols, to enable go-to-definition on them.
for (auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
- if (!Inc.Resolved.empty() && Inc.R.start.line == Pos.line)
- Result.push_back(
- Location{URIForFile::canonicalize(Inc.Resolved, *MainFilePath), {}});
+ if (!Inc.Resolved.empty() && Inc.R.start.line == Pos.line) {
+ LocatedSymbol File;
+ File.Name = llvm::sys::path::filename(Inc.Resolved);
+ File.PreferredDeclaration = {
+ URIForFile::canonicalize(Inc.Resolved, *MainFilePath), Range{}};
+ File.Definition = File.PreferredDeclaration;
+ // We're not going to find any further symbols on #include lines.
+ return {std::move(File)};
+ }
}
- if (!Result.empty())
- return Result;
- // Identified symbols at a specific position.
SourceLocation SourceLocationBeg =
getBeginningOfIdentifier(AST, Pos, SM.getMainFileID());
auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg);
- for (auto Item : Symbols.Macros) {
- auto Loc = Item.Info->getDefinitionLoc();
- auto L = makeLocation(AST, Loc, *MainFilePath);
- if (L)
- Result.push_back(*L);
+ // Macros are simple: there's no declaration/definition distinction.
+ // As a consequence, there's no need to look them up in the index either.
+ std::vector<LocatedSymbol> Result;
+ for (auto M : Symbols.Macros) {
+ if (auto Loc =
+ makeLocation(AST, M.Info->getDefinitionLoc(), *MainFilePath)) {
+ LocatedSymbol Macro;
+ Macro.Name = M.Name;
+ Macro.PreferredDeclaration = *Loc;
+ Macro.Definition = Loc;
+ Result.push_back(std::move(Macro));
+ }
}
- // Declaration and definition are different terms in C-family languages, and
- // LSP only defines the "GoToDefinition" specification, so we try to perform
- // the "most sensible" GoTo operation:
- //
- // - We use the location from AST and index (if available) to provide the
- // final results. When there are duplicate results, we prefer AST over
- // index because AST is more up-to-date.
- //
- // - For each symbol, we will return a location of the canonical declaration
- // (e.g. function declaration in header), and a location of definition if
- // they are available.
- //
- // So the work flow:
- //
- // 1. Identify the symbols being search for by traversing the AST.
- // 2. Populate one of the locations with the AST location.
- // 3. Use the AST information to query the index, and populate the index
- // location (if available).
- // 4. Return all populated locations for all symbols, definition first (
- // which we think is the users wants most often).
- struct CandidateLocation {
- llvm::Optional<Location> Def;
- llvm::Optional<Location> Decl;
- };
- // We respect the order in Symbols.Decls.
- llvm::SmallVector<CandidateLocation, 8> ResultCandidates;
- llvm::DenseMap<SymbolID, size_t> CandidatesIndex;
+ // Decls are more complicated.
+ // The AST contains at least a declaration, maybe a definition.
+ // These are up-to-date, and so generally preferred over index results.
+ // We perform a single batch index lookup to find additional definitions.
+
+ // Results follow the order of Symbols.Decls.
+ // Keep track of SymbolID -> index mapping, to fill in index data later.
+ llvm::DenseMap<SymbolID, size_t> ResultIndex;
// Emit all symbol locations (declaration or definition) from AST.
for (const DeclInfo &DI : Symbols.Decls) {
const Decl *D = DI.D;
- // Fake key for symbols don't have USR (no SymbolID).
- // Ideally, there should be a USR for each identified symbols. Symbols
- // without USR are rare and unimportant cases, we use the a fake holder to
- // minimize the invasiveness of these cases.
+ auto Loc = makeLocation(AST, findNameLoc(D), *MainFilePath);
+ if (!Loc)
+ continue;
+
+ Result.emplace_back();
+ if (auto *ND = dyn_cast<NamedDecl>(D))
+ Result.back().Name = printName(AST.getASTContext(), *ND);
+ Result.back().PreferredDeclaration = *Loc;
+ // DeclInfo.D is always a definition if possible, so this check works.
+ if (getDefinition(D) == D)
+ Result.back().Definition = *Loc;
+
+ // Record SymbolID for index lookup later.
SymbolID Key("");
if (auto ID = getSymbolID(D))
- Key = *ID;
-
- auto R = CandidatesIndex.try_emplace(Key, ResultCandidates.size());
- if (R.second) // new entry
- ResultCandidates.emplace_back();
- auto &Candidate = ResultCandidates[R.first->second];
-
- auto Loc = findNameLoc(D);
- auto L = makeLocation(AST, Loc, *MainFilePath);
- // The declaration in the identified symbols is a definition if possible
- // otherwise it is declaration.
- bool IsDef = getDefinition(D) == D;
- // Populate one of the slots with location for the AST.
- if (!IsDef)
- Candidate.Decl = L;
- else
- Candidate.Def = L;
+ ResultIndex[*ID] = Result.size() - 1;
}
- if (Index) {
+ // Now query the index for all Symbol IDs we found in the AST.
+ if (Index && !ResultIndex.empty()) {
LookupRequest QueryRequest;
- // Build request for index query, using SymbolID.
- for (auto It : CandidatesIndex)
+ for (auto It : ResultIndex)
QueryRequest.IDs.insert(It.first);
- std::string TUPath;
- const FileEntry *FE = SM.getFileEntryForID(SM.getMainFileID());
- if (auto Path = getCanonicalPath(FE, SM))
- TUPath = *Path;
- // Query the index and populate the empty slot.
- Index->lookup(QueryRequest, [&TUPath, &ResultCandidates,
- &CandidatesIndex](const Symbol &Sym) {
- auto It = CandidatesIndex.find(Sym.ID);
- assert(It != CandidatesIndex.end());
- auto &Value = ResultCandidates[It->second];
-
- if (!Value.Def)
- Value.Def = toLSPLocation(Sym.Definition, TUPath);
- if (!Value.Decl)
- Value.Decl = toLSPLocation(Sym.CanonicalDeclaration, TUPath);
- });
- }
+ Index->lookup(QueryRequest, [&](const Symbol &Sym) {
+ auto &R = Result[ResultIndex.lookup(Sym.ID)];
+
+ // Special case: if the AST yielded a definition, then it may not be
+ // the right *declaration*. Prefer the one from the index.
+ if (R.Definition) // from AST
+ if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, *MainFilePath))
+ R.PreferredDeclaration = *Loc;
- // Populate the results, definition first.
- for (const auto &Candidate : ResultCandidates) {
- if (Candidate.Def)
- Result.push_back(*Candidate.Def);
- if (Candidate.Decl &&
- Candidate.Decl != Candidate.Def) // Decl and Def might be the same
- Result.push_back(*Candidate.Decl);
+ if (!R.Definition)
+ R.Definition = toLSPLocation(Sym.Definition, *MainFilePath);
+ });
}
return Result;
@@ -804,5 +780,12 @@
return Results;
}
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const LocatedSymbol &S) {
+ OS << S.Name << ": " << S.PreferredDeclaration;
+ if (S.Definition)
+ OS << " def=" << *S.Definition;
+ return OS;
+}
+
} // namespace clangd
} // namespace clang
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -18,6 +18,7 @@
#include "GlobalCompilationDatabase.h"
#include "Protocol.h"
#include "TUScheduler.h"
+#include "XRefs.h"
#include "index/Background.h"
#include "index/FileIndex.h"
#include "index/Index.h"
@@ -164,8 +165,8 @@
void signatureHelp(PathRef File, Position Pos, Callback<SignatureHelp> CB);
/// Get definition of symbol at a specified \p Line and \p Column in \p File.
- void findDefinitions(PathRef File, Position Pos,
- Callback<std::vector<Location>> CB);
+ void locateSymbolAt(PathRef File, Position Pos,
+ Callback<std::vector<LocatedSymbol>> CB);
/// Helper function that returns a path to the corresponding source file when
/// given a header file and vice versa.
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -12,7 +12,6 @@
#include "Headers.h"
#include "SourceCode.h"
#include "Trace.h"
-#include "XRefs.h"
#include "index/FileIndex.h"
#include "index/Merge.h"
#include "clang/Format/Format.h"
@@ -338,13 +337,13 @@
WorkScheduler.runWithAST("DumpAST", File, Bind(Action, std::move(Callback)));
}
-void ClangdServer::findDefinitions(PathRef File, Position Pos,
- Callback<std::vector<Location>> CB) {
- auto Action = [Pos, this](Callback<std::vector<Location>> CB,
+void ClangdServer::locateSymbolAt(PathRef File, Position Pos,
+ Callback<std::vector<LocatedSymbol>> CB) {
+ auto Action = [Pos, this](decltype(CB) CB,
llvm::Expected<InputsAndAST> InpAST) {
if (!InpAST)
return CB(InpAST.takeError());
- CB(clangd::findDefinitions(InpAST->AST, Pos, Index));
+ CB(clangd::locateSymbolAt(InpAST->AST, Pos, Index));
};
WorkScheduler.runWithAST("Definitions", File, Bind(Action, std::move(CB)));
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -79,6 +79,8 @@
Callback<SignatureHelp>);
void onGoToDefinition(const TextDocumentPositionParams &,
Callback<std::vector<Location>>);
+ void onGoToDeclaration(const TextDocumentPositionParams &,
+ Callback<std::vector<Location>>);
void onReference(const ReferenceParams &, Callback<std::vector<Location>>);
void onSwitchSourceHeader(const TextDocumentIdentifier &,
Callback<std::string>);
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -329,6 +329,7 @@
llvm::json::Object{
{"triggerCharacters", {"(", ","}},
}},
+ {"declarationProvider", true},
{"definitionProvider", true},
{"documentHighlightProvider", true},
{"hoverProvider", true},
@@ -656,8 +657,37 @@
void ClangdLSPServer::onGoToDefinition(const TextDocumentPositionParams &Params,
Callback<std::vector<Location>> Reply) {
- Server->findDefinitions(Params.textDocument.uri.file(), Params.position,
- std::move(Reply));
+ Server->locateSymbolAt(
+ Params.textDocument.uri.file(), Params.position,
+ Bind(
+ [&](decltype(Reply) Reply,
+ llvm::Expected<std::vector<LocatedSymbol>> Symbols) {
+ if (!Symbols)
+ return Reply(Symbols.takeError());
+ std::vector<Location> Defs;
+ for (const auto &S : *Symbols)
+ Defs.push_back(S.Definition.getValueOr(S.PreferredDeclaration));
+ Reply(std::move(Defs));
+ },
+ std::move(Reply)));
+}
+
+void ClangdLSPServer::onGoToDeclaration(
+ const TextDocumentPositionParams &Params,
+ Callback<std::vector<Location>> Reply) {
+ Server->locateSymbolAt(
+ Params.textDocument.uri.file(), Params.position,
+ Bind(
+ [&](decltype(Reply) Reply,
+ llvm::Expected<std::vector<LocatedSymbol>> Symbols) {
+ if (!Symbols)
+ return Reply(Symbols.takeError());
+ std::vector<Location> Decls;
+ for (const auto &S : *Symbols)
+ Decls.push_back(S.PreferredDeclaration);
+ Reply(std::move(Decls));
+ },
+ std::move(Reply)));
}
void ClangdLSPServer::onSwitchSourceHeader(const TextDocumentIdentifier &Params,
@@ -743,6 +773,7 @@
MsgHandler->bind("textDocument/completion", &ClangdLSPServer::onCompletion);
MsgHandler->bind("textDocument/signatureHelp", &ClangdLSPServer::onSignatureHelp);
MsgHandler->bind("textDocument/definition", &ClangdLSPServer::onGoToDefinition);
+ MsgHandler->bind("textDocument/declaration", &ClangdLSPServer::onGoToDeclaration);
MsgHandler->bind("textDocument/references", &ClangdLSPServer::onReference);
MsgHandler->bind("textDocument/switchSourceHeader", &ClangdLSPServer::onSwitchSourceHeader);
MsgHandler->bind("textDocument/rename", &ClangdLSPServer::onRename);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
