Page MenuHomePhabricator
Differential D38425 Diff 124951 clangd/ClangdLSPServer.cpp

Changeset View

Standalone View

View Options

clangd/ClangdLSPServer.cpp

Show First 20 Lines • Show All 51 Lines • ▼ Show 20 Line(s)51 json::obj{
52 {"resolveProvider", false}, 52​ {"resolveProvider", false},
53 {"triggerCharacters", {".", ">", ":"}}, 53​ {"triggerCharacters", {".", ">", ":"}},
54 }}, 54​ }},
55 {"signatureHelpProvider", 55​ {"signatureHelpProvider",
56 json::obj{ 56​ json::obj{
57 {"triggerCharacters", {"(", ","}}, 57​ {"triggerCharacters", {"(", ","}},
58 }}, 58​ }},
59 {"definitionProvider", true}, 59​ {"definitionProvider", true},
60​ {"documentHighlightProvider", true},
60 {"renameProvider", true}, 61​ {"renameProvider", true},
61 {"executeCommandProvider", 62​ {"executeCommandProvider",
62 json::obj{ 63​ json::obj{
63 {"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}}, 64​ {"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}},
64 }}, 65​ }},
65 }}}}); 66​ }}}});
66 if (Params.rootUri && !Params.rootUri->file.empty()) 67​ if (Params.rootUri && !Params.rootUri->file.empty())
malaperleUnsubmitted
Done
Reply

extra line

malaperle: extra line
ilya-biryukovUnsubmitted
Not Done
Reply

Still there

ilya-biryukov: Still there
67 Server.setRootPath(Params.rootUri->file); 68​ Server.setRootPath(Params.rootUri->file);
68 else if (Params.rootPath && !Params.rootPath->empty()) 69​ else if (Params.rootPath && !Params.rootPath->empty())
69 Server.setRootPath(*Params.rootPath); 70​ Server.setRootPath(*Params.rootPath);
70} 71​}
71 72​
72void ClangdLSPServer::onShutdown(Ctx C, ShutdownParams &Params) { 73​void ClangdLSPServer::onShutdown(Ctx C, ShutdownParams &Params) {
73 // Do essentially nothing, just say we're ready to exit. 74​ // Do essentially nothing, just say we're ready to exit.
74 ShutdownRequestReceived = true; 75​ ShutdownRequestReceived = true;
▲ Show 20 Lines • Show All 154 Lines • ▼ Show 20 Line(s)
229 230​
230void ClangdLSPServer::onSwitchSourceHeader(Ctx C, 231​void ClangdLSPServer::onSwitchSourceHeader(Ctx C,
231 TextDocumentIdentifier &Params) { 232​ TextDocumentIdentifier &Params) {
232 llvm::Optional<Path> Result = Server.switchSourceHeader(Params.uri.file); 233​ llvm::Optional<Path> Result = Server.switchSourceHeader(Params.uri.file);
233 std::string ResultUri; 234​ std::string ResultUri;
234 C.reply(Result ? URI::fromFile(*Result).uri : ""); 235​ C.reply(Result ? URI::fromFile(*Result).uri : "");
235} 236​}
236 237​
238​void ClangdLSPServer::onDocumentHighlight(Ctx C,
239​ TextDocumentPositionParams &Params) {
240​
241​ auto Highlights = Server.findDocumentHighlights(
malaperleUnsubmitted
Done
Reply

Items -> Highlights?

malaperle: Items -> Highlights?
malaperleUnsubmitted
Not Done
Reply

Items -> Highlights?

malaperle: Items -> Highlights?
242​ Params.textDocument.uri.file,
243​ Position{Params.position.line, Params.position.character});
244​
245​ if (!Highlights) {
malaperleUnsubmitted
Not Done
Reply

I get a test failure here because there is an assertion that the Expected<> needs to be checked. I can't really think of any failure case right now where we wouldn't just return an empty array of highlights. But I think it's better for consistency and future-proofing to keep the Expected<>.
I think you can just do like in onRename for now

if (!Highlights) {
  C.replyError(ErrorCode::InternalError,
               llvm::toString(Highlights.takeError()));
  return;
}
malaperle: I get a test failure here because there is an assertion that the Expected<> needs to be checked.
malaperleUnsubmitted
Not Done
Reply

Just to be clear, the error I see in the output is:
Expected<T> must be checked before access or destruction.
Expected<T> value was in success state. (Note: Expected<T> values in success mode must still be checked prior to being destroyed).

It asserts when in ClangdLSPServer::onDocumentHighlight, referencing Highlights->Value

malaperle: Just to be clear, the error I see in the output is: Expected<T> must be checked before access…
246​ C.replyError(ErrorCode::InternalError,
247​ llvm::toString(Highlights.takeError()));
248​ return;
249​ }
250​
251​ C.reply(json::ary(Highlights->Value));
252​}
253​
237ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount, 254​ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
238 bool StorePreamblesInMemory, 255​ bool StorePreamblesInMemory,
239 const clangd::CodeCompleteOptions &CCOpts, 256​ const clangd::CodeCompleteOptions &CCOpts,
240 llvm::Optional<StringRef> ResourceDir, 257​ llvm::Optional<StringRef> ResourceDir,
241 llvm::Optional<Path> CompileCommandsDir) 258​ llvm::Optional<Path> CompileCommandsDir)
242 : Out(Out), CDB(/*Logger=*/Out, std::move(CompileCommandsDir)), 259​ : Out(Out), CDB(/*Logger=*/Out, std::move(CompileCommandsDir)),
243 Server(CDB, /*DiagConsumer=*/*this, FSProvider, AsyncThreadsCount, 260​ Server(CDB, /*DiagConsumer=*/*this, FSProvider, AsyncThreadsCount,
244 StorePreamblesInMemory, CCOpts, 261​ StorePreamblesInMemory, CCOpts,
▲ Show 20 Lines • Show All 73 Lines • Show Last 20 Lines