[clang-tools-extra] r352864 - [clangd] Implement textDocument/declaration from LSP 3.14
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 1 03:26:13 PST 2019
Author: sammccall
Date: Fri Feb 1 03:26:13 2019
New Revision: 352864
URL: http://llvm.org/viewvc/llvm-project?rev=352864&view=rev
Log:
[clangd] Implement textDocument/declaration from LSP 3.14
Summary:
LSP now reflects the declaration/definition distinction.
Language server changes:
- textDocument/definition now returns a definition if one is found, otherwise
the declaration. It no longer returns declaration + definition if they are
distinct.
- textDocument/declaration returns the best declaration we can find.
- For macros, the active macro definition is returned for both methods.
- For include directive, the top of the target file is returned for both.
There doesn't appear to be a discovery mechanism (we can't return everything to
clients that only know about definition), so this changes existing behavior.
In practice, it should greatly reduce the fraction of the time we need to show
the user a menu of options.
C++ API changes:
- findDefinitions is replaced by locateSymbolAt, which returns a
vector<LocatedSymbol> - one for each symbol under the cursor.
- this contains the preferred declaration, the definition (if found), and
the symbol name
This API enables some potentially-neat extensions, like swapping between decl
and def, and exposing the symbol name to the UI in the case of multiple symbols.
Reviewers: hokein
Subscribers: ilya-biryukov, javed.absar, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D57388
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/XRefs.h
clang-tools-extra/trunk/test/clangd/initialize-params.test
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
clang-tools-extra/trunk/unittests/clangd/SyncAPI.h
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=352864&r1=352863&r2=352864&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Feb 1 03:26:13 2019
@@ -354,6 +354,7 @@ void ClangdLSPServer::onInitialize(const
llvm::json::Object{
{"triggerCharacters", {"(", ","}},
}},
+ {"declarationProvider", true},
{"definitionProvider", true},
{"documentHighlightProvider", true},
{"hoverProvider", true},
@@ -727,8 +728,37 @@ void ClangdLSPServer::onSignatureHelp(co
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,
@@ -814,6 +844,7 @@ ClangdLSPServer::ClangdLSPServer(class T
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);
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=352864&r1=352863&r2=352864&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Fri Feb 1 03:26:13 2019
@@ -77,6 +77,8 @@ private:
void onCompletion(const CompletionParams &, Callback<CompletionList>);
void onSignatureHelp(const TextDocumentPositionParams &,
Callback<SignatureHelp>);
+ void onGoToDeclaration(const TextDocumentPositionParams &,
+ Callback<std::vector<Location>>);
void onGoToDefinition(const TextDocumentPositionParams &,
Callback<std::vector<Location>>);
void onReference(const ReferenceParams &, Callback<std::vector<Location>>);
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=352864&r1=352863&r2=352864&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Feb 1 03:26:13 2019
@@ -13,7 +13,6 @@
#include "Headers.h"
#include "SourceCode.h"
#include "Trace.h"
-#include "XRefs.h"
#include "index/FileIndex.h"
#include "index/Merge.h"
#include "refactor/Tweak.h"
@@ -400,13 +399,13 @@ void ClangdServer::dumpAST(PathRef File,
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)));
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=352864&r1=352863&r2=352864&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Fri Feb 1 03:26:13 2019
@@ -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"
@@ -167,9 +168,9 @@ public:
/// called for tracked files.
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);
+ /// Find declaration/definition locations of symbol at a specified position.
+ 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.
Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=352864&r1=352863&r2=352864&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Fri Feb 1 03:26:13 2019
@@ -21,18 +21,26 @@ namespace clang {
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
+// Kinds of nodes that always return nullptr here will not have definitions
+// reported by locateSymbolAt().
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 +248,8 @@ llvm::Optional<Location> makeLocation(Pa
} // 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 +258,83 @@ std::vector<Location> findDefinitions(Pa
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);
- }
-
- // 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;
+ // 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));
+ }
+ }
+
+ // 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.
- SymbolID Key("");
- if (auto ID = getSymbolID(D))
- Key = *ID;
+ 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;
- 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;
+ // Record SymbolID for index lookup later.
+ if (auto ID = getSymbolID(D))
+ 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)];
- // 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);
+ // 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;
+
+ if (!R.Definition)
+ R.Definition = toLSPLocation(Sym.Definition, *MainFilePath);
+ });
}
return Result;
@@ -805,5 +782,12 @@ std::vector<SymbolDetails> getSymbolInfo
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
Modified: clang-tools-extra/trunk/clangd/XRefs.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.h?rev=352864&r1=352863&r2=352864&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.h (original)
+++ clang-tools-extra/trunk/clangd/XRefs.h Fri Feb 1 03:26:13 2019
@@ -22,9 +22,25 @@
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)
+// For some types of symbol, e.g. macros, definition == declaration always.
+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,
Modified: clang-tools-extra/trunk/test/clangd/initialize-params.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/initialize-params.test?rev=352864&r1=352863&r2=352864&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/initialize-params.test (original)
+++ clang-tools-extra/trunk/test/clangd/initialize-params.test Fri Feb 1 03:26:13 2019
@@ -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,
Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=352864&r1=352863&r2=352864&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Fri Feb 1 03:26:13 2019
@@ -43,8 +43,9 @@ using ::testing::IsEmpty;
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 @@ int hello;
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 @@ int hello;
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 @@ TEST_F(ClangdVFSTest, InvalidCompileComm
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 @@ int d;
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 @@ int d;
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(
Modified: clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp?rev=352864&r1=352863&r2=352864&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp Fri Feb 1 03:26:13 2019
@@ -84,10 +84,10 @@ llvm::Expected<SignatureHelp> runSignatu
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);
}
Modified: clang-tools-extra/trunk/unittests/clangd/SyncAPI.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SyncAPI.h?rev=352864&r1=352863&r2=352864&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SyncAPI.h (original)
+++ clang-tools-extra/trunk/unittests/clangd/SyncAPI.h Fri Feb 1 03:26:13 2019
@@ -32,8 +32,8 @@ runCodeComplete(ClangdServer &Server, Pa
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);
Modified: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp?rev=352864&r1=352863&r2=352864&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp Fri Feb 1 03:26:13 2019
@@ -687,10 +687,10 @@ TEST_F(TUSchedulerTests, TUStatus) {
// 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());
Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=352864&r1=352863&r2=352864&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Fri Feb 1 03:26:13 2019
@@ -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 clangd {
namespace {
using testing::ElementsAre;
-using testing::Field;
using testing::IsEmpty;
using testing::Matcher;
using testing::UnorderedElementsAreArray;
@@ -95,9 +95,35 @@ TEST(HighlightsTest, All) {
}
}
+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 @@ TEST(GoToDefinition, WithIndex) {
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 @@ TEST(GoToDefinition, WithIndex) {
^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 @@ TEST(GoToDefinition, WithIndex) {
^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 @@ TEST(GoToDefinition, All) {
)cpp",
R"cpp(// Function declaration via call
- int [[foo]](int);
+ int $decl[[foo]](int);
int main() {
return ^foo(42);
}
@@ -222,7 +247,7 @@ TEST(GoToDefinition, All) {
)cpp",
R"cpp(// Method call
- struct Foo { int [[x]](); };
+ struct Foo { int $decl[[x]](); };
int main() {
Foo bar;
bar.^x();
@@ -230,7 +255,7 @@ TEST(GoToDefinition, All) {
)cpp",
R"cpp(// Typedef
- typedef int [[Foo]];
+ typedef int $decl[[Foo]];
int main() {
^Foo bar;
}
@@ -243,7 +268,7 @@ TEST(GoToDefinition, All) {
)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 @@ TEST(GoToDefinition, All) {
};
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())
+ WantDef = 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 @@ TEST(GoToDefinition, Rank) {
}
)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 @@ int [[bar_not_preamble]];
// 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 @@ TEST(GoToInclude, All) {
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 = 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("2"));
+ ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+ EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", 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 = 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("5"));
+ ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
+ EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", 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 @@ TEST(GoToDefinition, WithPreamble) {
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 @@ TEST(GoToDefinition, WithPreamble) {
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) {
More information about the cfe-commits
mailing list