hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric,
ilya-biryukov.
The representation of references in LSP is limitted (just location
information). This patch makes our xref C++ API return structured results,
which allows other clients get more data for references.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53188
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/XRefs.cpp
clangd/XRefs.h
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===================================================================
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1184,7 +1184,7 @@
std::vector<Matcher<Location>> ExpectedLocations;
for (const auto &R : T.ranges())
ExpectedLocations.push_back(RangeIs(R));
- EXPECT_THAT(findReferences(AST, T.point()),
+ EXPECT_THAT(findReferences(AST, T.point()).render(""),
ElementsAreArray(ExpectedLocations))
<< Test;
}
@@ -1199,7 +1199,7 @@
auto AST = TU.build();
// References in main file are returned without index.
- EXPECT_THAT(findReferences(AST, Main.point(), /*Index=*/nullptr),
+ EXPECT_THAT(findReferences(AST, Main.point(), /*Index=*/nullptr).render(""),
ElementsAre(RangeIs(Main.range())));
Annotations IndexedMain(R"cpp(
int main() { [[f^oo]](); }
@@ -1210,13 +1210,14 @@
IndexedTU.Code = IndexedMain.code();
IndexedTU.Filename = "Indexed.cpp";
IndexedTU.HeaderCode = Header;
- EXPECT_THAT(findReferences(AST, Main.point(), IndexedTU.index().get()),
- ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range())));
+ EXPECT_THAT(
+ findReferences(AST, Main.point(), IndexedTU.index().get()).render(""),
+ ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range())));
// If the main file is in the index, we don't return duplicates.
// (even if the references are in a different location)
TU.Code = ("\n\n" + Main.code()).str();
- EXPECT_THAT(findReferences(AST, Main.point(), TU.index().get()),
+ EXPECT_THAT(findReferences(AST, Main.point(), TU.index().get()).render(""),
ElementsAre(RangeIs(Main.range())));
}
Index: clangd/XRefs.h
===================================================================
--- clangd/XRefs.h
+++ clangd/XRefs.h
@@ -34,9 +34,20 @@
/// Get the hover information when hovering at \p Pos.
llvm::Optional<Hover> getHover(ParsedAST &AST, Position Pos);
-/// Returns reference locations of the symbol at a specified \p Pos.
-std::vector<Location> findReferences(ParsedAST &AST, Position Pos,
- const SymbolIndex *Index = nullptr);
+/// Represents a list of references for clangd C++ API.
+/// We don't use the LSP structures here as LSP ref representation is limitted,
+/// and we want other C++ API users (unlike ClangdLSPServer) to get more
+/// information about references.
+struct RefResult {
+ std::vector<Ref> Refs;
+ // Storage for the underlying data.
+ llvm::StringSet<> Storage;
+ // Serialize to a list of LSP locations.
+ std::vector<Location> render(llvm::StringRef HintPath) const;
+};
+/// Returns symbol references at a specified \p Pos.
+RefResult findReferences(ParsedAST &AST, Position Pos,
+ const SymbolIndex *Index = nullptr);
} // namespace clangd
} // namespace clang
Index: clangd/XRefs.cpp
===================================================================
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -705,9 +705,18 @@
return None;
}
-std::vector<Location> findReferences(ParsedAST &AST, Position Pos,
- const SymbolIndex *Index) {
+std::vector<Location> RefResult::render(llvm::StringRef HintPath) const {
std::vector<Location> Results;
+ for (auto &R : Refs) {
+ if (auto LSPLoc = toLSPLocation(R.Location, HintPath))
+ Results.push_back(std::move(*LSPLoc));
+ }
+ return Results;
+}
+
+RefResult findReferences(ParsedAST &AST, Position Pos,
+ const SymbolIndex *Index) {
+ RefResult Results;
const SourceManager &SM = AST.getASTContext().getSourceManager();
auto MainFilePath = getRealPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
if (!MainFilePath) {
@@ -723,14 +732,26 @@
TargetDecls.push_back(DI.D);
}
+ auto Intern = [&Results](llvm::StringRef S) {
+ auto It = Results.Storage.find(S);
+ if (It == Results.Storage.end())
+ It = Results.Storage.insert(S).first;
+ return It->first();
+ };
+
// We traverse the AST to find references in the main file.
// TODO: should we handle macros, too?
auto MainFileRefs = findRefs(TargetDecls, AST);
for (const auto &Ref : MainFileRefs) {
- Location Result;
- Result.range = getTokenRange(AST, Ref.Loc);
- Result.uri = URIForFile(*MainFilePath);
- Results.push_back(std::move(Result));
+ clang::clangd::Ref R;
+ auto Range = getTokenRange(AST, Ref.Loc);
+ std::tie(R.Location.Start.Line, R.Location.Start.Column) =
+ std::tie(Range.start.line, Range.start.character);
+ std::tie(R.Location.End.Line, R.Location.End.Column) =
+ std::tie(Range.end.line, Range.end.character);
+ std::string FileURI = URIForFile(*MainFilePath).uri();
+ R.Location.FileURI = Intern(FileURI);
+ Results.Refs.push_back(R);
}
// Now query the index for references from other files.
@@ -751,8 +772,11 @@
Index->refs(Req, [&](const Ref &R) {
auto LSPLoc = toLSPLocation(R.Location, /*HintPath=*/*MainFilePath);
// Avoid indexed results for the main file - the AST is authoritative.
- if (LSPLoc && LSPLoc->uri.file() != *MainFilePath)
- Results.push_back(std::move(*LSPLoc));
+ if (LSPLoc && LSPLoc->uri.file() != *MainFilePath) {
+ Ref NewR = R;
+ NewR.Location.FileURI = Intern(R.Location.FileURI);
+ Results.Refs.push_back(NewR);
+ }
});
return Results;
}
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/FileIndex.h"
#include "index/Index.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -168,8 +169,7 @@
Callback<std::vector<SymbolInformation>> CB);
/// Retrieve locations for symbol references.
- void findReferences(PathRef File, Position Pos,
- Callback<std::vector<Location>> CB);
+ void findReferences(PathRef File, Position Pos, Callback<RefResult> CB);
/// Run formatting for \p Rng inside \p File with content \p Code.
llvm::Expected<tooling::Replacements> formatRange(StringRef Code,
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -508,8 +508,8 @@
}
void ClangdServer::findReferences(PathRef File, Position Pos,
- Callback<std::vector<Location>> CB) {
- auto Action = [Pos, this](Callback<std::vector<Location>> CB,
+ Callback<RefResult> CB) {
+ auto Action = [Pos, this](Callback<RefResult> CB,
llvm::Expected<InputsAndAST> InpAST) {
if (!InpAST)
return CB(InpAST.takeError());
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -459,14 +459,14 @@
}
void ClangdLSPServer::onReference(ReferenceParams &Params) {
- Server->findReferences(Params.textDocument.uri.file(), Params.position,
- [](llvm::Expected<std::vector<Location>> Locations) {
- if (!Locations)
- return replyError(
- ErrorCode::InternalError,
- llvm::toString(Locations.takeError()));
- reply(llvm::json::Array(*Locations));
- });
+ std::string Path = Params.textDocument.uri.file();
+ Server->findReferences(
+ Path, Params.position, [&Path](llvm::Expected<RefResult> Refs) {
+ if (!Refs)
+ return replyError(ErrorCode::InternalError,
+ llvm::toString(Refs.takeError()));
+ reply(llvm::json::Array(Refs->render(Path)));
+ });
}
ClangdLSPServer::ClangdLSPServer(JSONOutput &Out,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
