Summary:
Most of its functionality is moved into ClangdLSPServer.
The decoupling between JSONRPCDispatcher, ProtocolCallbacks, ClangdLSPServer
was never real, and only served to obfuscate.
Some previous implicit/magic stuff is now explicit:
- the return type of LSP method calls are now in the signature
- no more reply() that gets the ID using global context magic
- arg tracing no longer relies on RequestArgs::stash context magic either
This is mostly refactoring, but some deliberate fixes while here:
- LSP method params are now by const reference
- notifications and calls are now distinct namespaces.
(some tests had protocol errors and needed updating)
- we now reply to calls we failed to decode
- outgoing calls use distinct IDs
A few error codes and message IDs changed in unimportant ways (see tests).
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman, jfb, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53387
llvm-svn: 344737
Summary:
This paves the way for alternative transports (mac XPC, maybe messagepack?),
and also generally improves layering: testing ClangdLSPServer becomes less of
a pipe dream, we split up the JSONOutput monolith, etc.
This isn't a final state, much of what remains in JSONRPCDispatcher can go away,
handlers can call reply() on the transport directly, JSONOutput can be renamed
to StreamLogger and removed, etc. But this patch is sprawling already.
The main observable change (see tests) is that hitting EOF on input is now an
error: the client should send the 'exit' notification.
This is defensible: the protocol doesn't spell this case out. Reproducing the
current behavior for all combinations of shutdown/exit/EOF clutters interfaces.
We can iterate on this if desired.
Reviewers: jkorous, ioeric, hokein
Subscribers: mgorny, ilya-biryukov, MaskRay, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53286
llvm-svn: 344672
Summary:
This paves the way for alternative transports (mac XPC, maybe messagepack?),
and also generally improves layering: testing ClangdLSPServer becomes less of
a pipe dream, we split up the JSONOutput monolith, etc.
This isn't a final state, much of what remains in JSONRPCDispatcher can go away,
handlers can call reply() on the transport directly, JSONOutput can be renamed
to StreamLogger and removed, etc. But this patch is sprawling already.
The main observable change (see tests) is that hitting EOF on input is now an
error: the client should send the 'exit' notification.
This is defensible: the protocol doesn't spell this case out. Reproducing the
current behavior for all combinations of shutdown/exit/EOF clutters interfaces.
We can iterate on this if desired.
Reviewers: jkorous, ioeric, hokein
Subscribers: mgorny, ilya-biryukov, MaskRay, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53286
llvm-svn: 344620
Summary:
I don't bother mirroring the full capabilities struct, just parse the
bits we care about. I'll send a new patch to use this approach elsewhere too.
Reviewers: kadircet
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D53213
llvm-svn: 344617
Summary:
The cancelable scopes are managed by JSONRPCDispatcher so that all Handlers
run in cancelable contexts.
(Previously ClangdServer did this, for code completion only).
Cancellation request processing is therefore also in JSONRPCDispatcher.
(Previously it was in ClangdLSPServer).
This doesn't actually make any new commands *respect* cancellation - they'd
need to check isCancelled() and bail out. But it opens the door to doing
this incrementally, and putting such logic in common machinery like TUScheduler.
I also rewrote the ClangdServer class/threading comments because I wanted to
add to it and I got carried away.
Reviewers: ilya-biryukov, kadircet
Subscribers: ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D52004
llvm-svn: 342135
Summary:
Task is no longer exposed:
- task cancellation is hidden as a std::function
- task creation returns the new context directly
- checking is via free function only, with no way to avoid the context lookup
The implementation is essentially the same, but a bit terser as it's hidden.
isCancelled() is now safe to use outside any task (it returns false).
This will leave us free to sprinkle cancellation in e.g. TUScheduler without
needing elaborate test setup, and lets callers that don't cancel "just work".
Updated the docs to describe the new expected use pattern.
One thing I noticed: there's nothing async-specific about the cancellation.
Async tasks can be cancelled from any thread (typically the one that created
them), sync tasks can be cancelled from any *other* thread in the same way.
So the docs now refer to "long-running" tasks instead of async ones.
Updated usage in code complete, without any structural changes.
I didn't update all the names of the helpers in ClangdLSPServer (these will
likely be moved to JSONRPCDispatcher anyway).
Reviewers: ilya-biryukov, kadircet
Subscribers: ioeric, MaskRay, jkorous, arphaman, jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D51996
llvm-svn: 342130
client if the client supports this extension
This commit extends the 'textDocument/publishDiagnostics' notification sent from
Clangd to the client. When it's enabled, Clangd sends out the fixits associated
with the appropriate diagnostic in the body of the 'publishDiagnostics'
notification. The client can enable this extension by setting 'clangdFixSupport'
to true in the textDocument capabilities during initialization.
Differential Revision: https://reviews.llvm.org/D50415
llvm-svn: 339454
compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request
This commit allows clangd to use an in-memory compilation database that's
controlled from the LSP client (-compile_args_from=lsp). It extends the
'workspace/didChangeConfiguration' request to allow the client to pass in a
compilation database subset that needs to be updated in the workspace.
Differential Revision: https://reviews.llvm.org/D49758
llvm-svn: 338597
Summary:
That way, as soon as the "initialize" is received by the server, it can start
parsing/indexing with a valid compilation database and not have to wait for a
an initial 'didChangeConfiguration' that might or might not happen.
Then, when the user changes configuration, a didChangeConfiguration can be sent.
Signed-off-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Reviewers: malaperle
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D49833
llvm-svn: 338518
Summary:
An AST-based approach is used to retrieve the document symbols rather than an
in-memory index query. The index is not an ideal fit to achieve this because of
the file-centric query being done here whereas the index is suited for
project-wide queries. Document symbols also includes more symbols and need to
keep the order as seen in the file.
Signed-off-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Subscribers: tomgr, ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47846
llvm-svn: 336386
Summary:
Caching is now handled by ClangdLSPServer and hidden behind the
GlobalCompilationDatabase interface. This simplifies ClangdServer.
This change also removes the SkipCache flag from addDocument,
which is now obsolete.
No behavioral changes are intended, the clangd binary still caches the
compile commands on the first read.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: mgorny, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48068
llvm-svn: 334585
Summary:
The EINTR loop around getline was added to fix an issue with mac gdb, but seems
to loop infinitely in rare cases on linux where the parent editor exits (most
reports with VSCode).
I can't work out how to fix this in a portable way with std::istream, but the
C APIs have clearer contracts and LLVM has a RetryAfterSignal function for use
with them which seems battle-tested.
While here, clean up some inconsistency around \n in log messages (now
add it only after JSON payloads), and reduce the scope of the
long-message handling which was only really added to fight fuzzers.
Reviewers: malaperle, ilya-biryukov
Subscribers: klimek, ioeric, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47643
llvm-svn: 333993
Summary:
This is a basic implementation of the "workspace/symbol" request which is
used to find symbols by a string query. Since this is similar to code completion
in terms of result, this implementation reuses the "fuzzyFind" in order to get
matches. For now, the scoring algorithm is the same as code completion and
improvements could be done in the future.
The index model doesn't contain quite enough symbols for this to cover
common symbols like methods, enum class enumerators, functions in unamed
namespaces, etc. The index model will be augmented separately to achieve this.
Reviewers: sammccall, ilya-biryukov
Reviewed By: sammccall
Subscribers: jkorous, hokein, simark, sammccall, klimek, mgorny, ilya-biryukov, mgrang, jkorous-apple, ioeric, MaskRay, cfe-commits
Differential Revision: https://reviews.llvm.org/D44882
llvm-svn: 330637
Summary:
This patch moves the draft manager closer to the edge of Clangd, from
ClangdServer to ClangdLSPServer. This will make it easier to implement
incremental document sync, by making ClangdServer only deal with
complete documents.
As a result, DraftStore doesn't have to deal with versioning, and thus
its API can be simplified. It is replaced by a StringMap in
ClangdServer holding a current version number for each file.
Signed-off-by: Simon Marchi <simon.marchi@ericsson.com>
Subscribers: klimek, mgorny, ilya-biryukov, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D44408
llvm-svn: 327711
Context can do what Tagged was intended to support (snapshot filesystems),
and less intrusively.
getTaggedFileSystem() no longer needs a filename.
Cleanups while here:
- code-complete now returns errors as Expected, like other functions
- added an alias Callback<T> for the usual callback function type
llvm-svn: 327344
Summary:
The new implementation attaches notes to diagnostic message and shows
the original diagnostics in the message of the note.
Reviewers: hokein, ioeric, sammccall
Reviewed By: sammccall
Subscribers: klimek, mgorny, cfe-commits, jkorous-apple
Differential Revision: https://reviews.llvm.org/D44142
llvm-svn: 327282
Summary:
This subsumes most of the params to ClangdServer and ClangdLSPServer.
Adjacent changes:
- tests use a consistent set of options, except when testing specific options
- tests that previously used synchronous mode for convenience no longer do
- added a runAddDocument helper to SyncAPIs to mitigate the extra code
- rearranged main a bit to follow the structure of the options
Reviewers: ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D44088
llvm-svn: 326719
Summary:
Implementation of DidChangeConfiguration notification handling in
clangd. This currently only supports changing one setting: the path of
the compilation database to be used for the current project. In other
words, it is no longer necessary to restart clangd with a different
command line argument in order to change the compilation database.
Reviewers: malaperle, krasimir, bkramer, ilya-biryukov
Subscribers: jkorous-apple, ioeric, simark, klimek, ilya-biryukov, arphaman, rwols, cfe-commits
Differential Revision: https://reviews.llvm.org/D39571
Signed-off-by: Simon Marchi <simon.marchi@ericsson.com>
Signed-off-by: William Enright <william.enright@polymtl.ca>
llvm-svn: 325784
Summary: Implemention of textDocument/hover as described in LSP definition.
This patch adds a basic Hover implementation. When hovering a variable,
function, method or namespace, clangd will return a text containing the
declaration's scope, as well as the declaration of the hovered entity.
For example, for a variable:
Declared in class Foo::Bar
int hello = 2
For macros, the macro definition is returned.
This patch doesn't include:
- markdown support (the client I use doesn't support it yet)
- range support (optional in the Hover response)
- comments associated to variables/functions/classes
They are kept as future work to keep this patch simpler.
I added tests in XRefsTests.cpp. hover.test contains one simple
smoketest to make sure the feature works from a black box perspective.
Reviewers: malaperle, krasimir, bkramer, ilya-biryukov
Subscribers: sammccall, mgrang, klimek, rwols, ilya-biryukov, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D35894
Signed-off-by: Simon Marchi <simon.marchi@ericsson.com>
Signed-off-by: William Enright <william.enright@polymtl.ca>
llvm-svn: 325395
Summary:
Instead of content-length, we delimit messages with ---.
This also removes the need for (most) dos-formatted test files.
Reviewers: ioeric
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D42919
llvm-svn: 324333
Summary:
Instead of passing Context explicitly around, we now have a thread-local
Context object `Context::current()` which is an implicit argument to
every function.
Most manipulation of this should use the WithContextValue helper, which
augments the current Context to add a single KV pair, and restores the
old context on destruction.
Advantages are:
- less boilerplate in functions that just propagate contexts
- reading most code doesn't require understanding context at all, and
using context as values in fewer places still
- fewer options to pass the "wrong" context when it changes within a
scope (e.g. when using Span)
- contexts pass through interfaces we can't modify, such as VFS
- propagating contexts across threads was slightly tricky (e.g.
copy vs move, no move-init in lambdas), and is now encapsulated in
the threadpool
Disadvantages are all the usual TLS stuff - hidden magic, and
potential for higher memory usage on threads that don't use the
context. (In practice, it's just one pointer)
Reviewers: ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D42517
llvm-svn: 323872
Summary:
Use the YAML-format symbols (generated by the global-symbol-builder tool) to
do the global code completion.
It is **experimental** only , but it allows us to experience global code
completion on a relatively small project.
Tested with LLVM project.
Reviewers: sammccall, ioeric
Reviewed By: sammccall, ioeric
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41668
llvm-svn: 322191
Summary:
We use Diagnostic as a key to find the corresponding FixIt when we do
the "apply-fix", but the "severity" field could be omitted, in some cases,
the codeAction request sent from LSP clients (e.g. VScode) doesn't include the
`severity` field, which makes clangd fail to find the FixIt.
Test the following code in VScode, before the fix, no FixIt shown.
```
void main() {}
^~~~
```
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41280
llvm-svn: 321106
Summary:
- when the diagnostic has an explicit range, we prefer that
- if the diagnostic has a fixit, its RemoveRange is our next choice
- otherwise we try to expand the diagnostic location into a whole token.
(inspired by VSCode, which does this client-side when given an empty range)
- if all else fails, we return the zero-width range as now.
(clients react in different ways to this, highlighting a token or a char)
- this includes the off-by-one fix from D40860, and borrows heavily from it
Reviewers: rwols, hokein
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41118
llvm-svn: 320555
Summary:
Previously, completion options were set per ClangdServer instance.
It will allow to change completion preferences during the lifetime
of a single ClangdServer instance.
Also rewrote ClangdCompletionTest.CompletionOptions to reuse single
ClangdServer instance, the test now runs 2x faster on my machine.
Reviewers: sammccall, ioeric, hokein
Reviewed By: sammccall, ioeric
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D40654
llvm-svn: 319753
Summary: (There must be some reason why D38077 didn't just do this, but I don't get it!)
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D39836
llvm-svn: 318925
Summary:
Make clangd handle "textDocument/rename" request. The rename
functionality comes from the "local-rename" sub-tool of clang-refactor.
Currently clangd only supports local rename (only symbol occurrences in
the main file will be renamed).
Reviewers: sammccall, ilya-biryukov
Reviewed By: sammccall
Subscribers: cfe-commits, ioeric, arphaman, mgorny
Differential Revision: https://reviews.llvm.org/D39676
llvm-svn: 317780
Summary:
When the user selects a fix-it (or any code action with commands), it is
possible to let the client forward the selected command to the server.
When the clangd.applyFix command is handled on the server, it can send a
workspace/applyEdit request to the client. This has the advantage that
the client doesn't explicitly have to know how to handle
clangd.applyFix. Therefore, the code to handle clangd.applyFix in the VS
Code extension (and any other Clangd client) is not required anymore.
Reviewers: ilya-biryukov, sammccall, Nebiroth, hokein
Reviewed By: hokein
Subscribers: ioeric, hokein, rwols, puremourning, bkramer, ilya-biryukov
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D39276
llvm-svn: 317322
Summary:
This changes the onShutdown handler to do essentially nothing (for now), and
instead exits the runloop when we receive the exit notification from the client.
Some clients may wait on the reply from the shutdown request before sending an
exit notification. If we exit the runloop already in the shutdown request, a
client might block forever.
This also gives us the opportunity to do any global cleanups and/or
serializations of PCH preambles to disk, but I've left that out for now.
See the LSP protocol documentation for details.
Reviewers: malaperle, krasimir, bkramer, sammccall, ilya-biryukov
Reviewed By: malaperle, sammccall, ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D38939
llvm-svn: 316564
Summary:
Make the ProtocolHandlers glue between JSONRPCDispatcher and
ClangdLSPServer generic.
Eliminate small differences between methods, de-emphasize the unimportant
distinction between notifications and methods.
ClangdLSPServer is no longer responsible for producing a complete
JSON-RPC response, just the JSON of the result object. (In future, we
should move that JSON serialization out, too).
Handler methods now take a context object that we may hang more
functionality off in the future.
Added documentation to ProtocolHandlers.
Reviewers: ilya-biryukov, bkramer
Reviewed By: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D38464
llvm-svn: 315577
Summary:
The client can send notifications when it detects watched files have
changed. This patch adds the protocol handling for this type of notification.
For now, the notification will be passed down to the ClangdServer, but it will
not be acted upon. However, this will become useful for the indexer to react
to file changes.
The events could also potentially be used to invalidate other caches
(compilation database, etc).
This change also updates the VSCode extension so that it sends the events.
Signed-off-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Reviewers: ilya-biryukov, Nebiroth
Subscribers: ilya-biryukov
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D38422
llvm-svn: 314693
Summary: Adds compileCommands command line argument to specify an absolute path directly to the requested compile_commands.json for flags.
Reviewed By: ilya-biryukov
Differential Revision: https://reviews.llvm.org/D37150
llvm-svn: 314678
Summary:
There doesn't seem to be any real separation between the current three objects.
Feel free to reject this if you find the current style valuable, though.
(Mostly I'm just looking around for cleanups to help me understand the code).
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D38414
llvm-svn: 314587
Sam McCall