Summary:
For completion items that would trigger include insertions (i.e. index symbols
that are not #included yet), add a visual indicator "+" before the completion
label. The inserted headers will appear in the completion detail.
Open to suggestions for better visual indicators; "+" was picked because it
seems cleaner than a few other candidates I've tried (*, #, @ ...).
The displayed header would be like a/b/c.h (without quote) or <vector> for system
headers. I didn't add quotation or "#include" because they can take up limited
space and do not provide additional information after users know what the
headers are. I think a header alone should be obvious for users to infer that
this is an include header..
To align indentation, also prepend ' ' to labels of candidates that would not
trigger include insertions (only for completions where index results are
possible).
Vim:
{F6357587}
vscode:
{F6357589}
{F6357591}
Reviewers: sammccall, ilya-biryukov, hokein
Reviewed By: sammccall
Subscribers: MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48163
llvm-svn: 334828
Summary:
Now we have most of Sema's code completion signals incorporated in Quality,
which will allow us to give consistent ranking to sema/index results.
Therefore we can/should stop using Sema priority as an explicit signal.
This fixes some issues like namespaces always having a terrible score.
The most important missing signals are:
- Really dumb/rarely useful completions like:
SomeStruct().^SomeStruct
SomeStruct().^operator=
SomeStruct().~SomeStruct()
We already filter out destructors, this patch adds injected names and
operators to that list.
- type matching the expression context.
Ilya has a plan to add this in a way that's compatible with indexes
(design doc should be shared real soon now!)
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47871
llvm-svn: 334192
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:
After this commit, clangd will only keep the last 3 accessed ASTs in
memory. Preambles for each of the opened files are still kept in
memory to make completion and AST rebuilds fast.
AST rebuilds are usually fast enough, but having the last ASTs in
memory still considerably improves latency of operations like
findDefinition and documeneHighlight, which are often sent multiple
times a second when moving around the code. So keeping some of the last
accessed ASTs in memory seems like a reasonable tradeoff.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: malaperle, arphaman, klimek, javed.absar, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47063
llvm-svn: 333737
Summary:
clangd will populate #include insertions as addtionalEdits in completion items.
The code completion tests in ClangdServerTest will be added back in D46497.
Reviewers: ilya-biryukov, sammccall
Reviewed By: ilya-biryukov
Subscribers: klimek, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D46676
llvm-svn: 332362
Summary:
The Language Server Protocol unfortunately mandates that locations in files
be represented by line/column pairs, where the "column" is actually an index
into the UTF-16-encoded text of the line.
(This is because VSCode is written in JavaScript, which is UTF-16-native).
Internally clangd treats source files at UTF-8, the One True Encoding, and
generally deals with byte offsets (though there are exceptions).
Before this patch, conversions between offsets and LSP Position pretended
that Position.character was UTF-8 bytes, which is only true for ASCII lines.
Now we examine the text to convert correctly (but don't actually need to
transcode it, due to some nice details of the encodings).
The updated functions in SourceCode are the blessed way to interact with
the Position.character field, and anything else is likely to be wrong.
So I also updated the other accesses:
- CodeComplete needs a "clang-style" line/column, with column in utf-8 bytes.
This is now converted via Position -> offset -> clang line/column
(a new function is added to SourceCode.h for the second conversion).
- getBeginningOfIdentifier skipped backwards in UTF-16 space, which is will
behave badly when it splits a surrogate pair. Skipping backwards in UTF-8
coordinates gives the lexer a fighting chance of getting this right.
While here, I clarified(?) the logic comments, fixed a bug with identifiers
containing digits, simplified the signature slightly and added a test.
This seems likely to cause problems with editors that have the same bug, and
treat the protocol as if columns are UTF-8 bytes. But we can find and fix those.
Reviewers: hokein
Subscribers: klimek, ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D46035
llvm-svn: 331029
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
Request in delimited input ended by EOF shouldn't be an error state.
Comments at the end of test file shouldn't be logged as an error state.
Input mirroring should work for the last request in delimited test file.
llvm-svn: 330608
Summary:
This patch adds support for incremental document syncing, as described
in the LSP spec. The protocol specifies ranges in terms of Position (a
line and a character), and our drafts are stored as plain strings. So I
see two things that may not be super efficient for very large files:
- Converting a Position to an offset (the positionToOffset function)
requires searching for end of lines until we reach the desired line.
- When we update a range, we construct a new string, which implies
copying the whole document.
However, for the typical size of a C++ document and the frequency of
update (at which a user types), it may not be an issue. This patch aims
at getting the basic feature in, and we can always improve it later if
we find it's too slow.
Signed-off-by: Simon Marchi <simon.marchi@ericsson.com>
Reviewers: malaperle, ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: MaskRay, klimek, mgorny, ilya-biryukov, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D44272
llvm-svn: 328500
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:
Changes:
o Store both the original header and the canonical header in LSP command.
o Also check that both original and canonical headers are not already included
by comparing both resolved header path and written literal includes.
This addresses the use case where private IWYU pragma is defined in a private
header while it would still be preferrable to include the private header, in the
internal implementation file. If we have seen that the priviate header is already
included, we don't try to insert the canonical include.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D43510
llvm-svn: 326070
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:
o Collect suitable #include paths for index symbols. This also does smart mapping
for STL symbols and IWYU pragma (code borrowed from include-fixer).
o For global code completion, add a command for inserting new #include in each code
completion item.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, mgorny, ilya-biryukov, jkorous-apple, hintonda, cfe-commits
Differential Revision: https://reviews.llvm.org/D42640
llvm-svn: 325343
Summary:
The chrome trace viewer requires events within a thread to strictly nest.
So we need to record the lifetime of the Span objects, not the contexts.
But we still want to show the relationship between spans where a context crosses
threads, so do this with flow events (i.e. arrows).
Before: https://photos.app.goo.gl/q4Dd9u9xtelaXk1v1
After: https://photos.app.goo.gl/5RNLmAMLZR3unvY83
(This could stand some further improvement, in particular I think we want a
container span whenever we schedule work on a thread. But that's another patch)
Reviewers: ioeric
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D43272
llvm-svn: 325220
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:
This adds checks that our diagnostics emit correct ranges in a bunch of cases,
as promised in D41118.
The diagnostics-preamble test is also converted and extended to be a little more
precise.
diagnostics.test stays around as the smoke test for this feature.
Reviewers: ilya-biryukov
Subscribers: klimek, mgorny, cfe-commits
Differential Revision: https://reviews.llvm.org/D41454
llvm-svn: 323448
Summary:
We will return errors for non-added files for now.
Another alternative for clangd would be to read non-added files from
disk and provide useful features anyway.
There are still some cases that fail with assertion (e.g., code
complete). We should address those too, but they require more subtle
changes to the code and therefore out of scope of this patch.
Reviewers: sammccall, ioeric, hokein
Reviewed By: sammccall
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D42164
llvm-svn: 322637
Summary:
The goal here is again to make it easier to read and write the tests.
I've extracted `parseTextMarker` from CodeCompleteTests into an `Annotations`
class, adding features to it:
- as well as points `^s` it allows ranges `[[...]]`
- multiple points and ranges are supported
- points and ranges may be named: `$name^` and `$name[[...]]`
These features are used for the xrefs tests. This also paves the way for
replacing the lit diagnostics.test with more readable unit tests, using named
ranges.
Alternative considered: `TestSelectionRange` in clang-refactor/TestSupport
Main problems were:
- delimiting the end of ranges is awkward, requiring counting
- comment syntax is long and at least as cryptic for most cases
- no separate syntax for point vs range, which keeps xrefs tests concise
- Still need to convert to Position everywhere
- Still need helpers for common case of expecting exactly one point/range
(I'll probably promote the extra `PrintTo`s from some of the core Protocol types
into `operator<<` in `Protocol.h` itself in a separate, prior patch...)
Reviewers: ioeric
Subscribers: klimek, mgorny, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41432
llvm-svn: 321184
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:
This allows us to limit the number of results we return and still allow them
to be surfaced by refining a query (D39852).
The initial algorithm is very conservative - it accepts a completion if the
filter is any case-insensitive sub-sequence. It does not attempt to rank items
based on match quality.
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D39882
llvm-svn: 319552
Summary:
- JSON<->Obj interface is now ADL functions, so they play nicely with enums
- recursive vector/map parsing and ObjectMapper moved to JSONExpr and tested
- renamed (un)parse to (de)serialize, since text -> JSON is called parse
- Protocol.cpp gets a bit shorter
Sorry for the giant patch, it's prety mechanical though
Reviewers: ilya-biryukov
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D40596
llvm-svn: 319478
Summary:
This makes the parse() functions about as short as they can be given the
current signature, and moves all array-traversal etc code to a
central location.
We keep the ability to distinguish between optional and required fields:
and we don't propagate parse errors for optional fields.
I've made most fields required per the LSP spec - the looseness we had
here was mostly a historical accident I think.
Reviewers: ioeric
Subscribers: klimek, cfe-commits, ilya-biryukov
Differential Revision: https://reviews.llvm.org/D40564
llvm-svn: 319309
Summary:
- Converted Protocol.h parse() functions to take JSON::Expr.
These no longer detect and log unknown fields, as this is not that
useful and no longer free.
I haven't changed the error handling too much: fields that were
treated as optional before are still optional, even when it's wrong.
Exception: object properties with the wrong type are now ignored.
- Made JSONRPCDispatcher parse using json::parse
- The bug where 'method' must come before 'params' in the stream is
fixed as a side-effect. (And the same bug in executeCommand).
- Some parser crashers fixed as a side effect.
e.g. https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=3890
- The debug stream now prettyprints the input messages with --pretty.
- Request params are attached to traces when tracing is enabled.
- Fixed some bugs in tests (errors tolerated by YAMLParser, and
off-by-ones in Content-Length that our null-termination was masking)
- Fixed a random double-escape bug in ClangdLSPServer (it was our last
use of YAMLParser!)
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D40406
llvm-svn: 319159
Summary:
[clangd] Tracing improvements
Compose JSON using JSONExpr
Allow attaching metadata to spans (and avoid it if tracing is off)
Attach IDs and responses of JSON RPCs to their spans
The downside is that large responses make the trace viewer sluggish.
We should make our responses less huge :-) Or fix trace viewer.
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D40132
llvm-svn: 318928
Summary:
This scale is much easier to mix with other signals, such as fuzzy match strength.
Mostly NFC, but it does reorder some low-priority items that get folded together at a score of 0 (see completion-qualifiers.test).
Removed the exact sortText from the testcases, because it's the ranking that we want to test.
Reviewers: hokein
Subscribers: ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D40089
llvm-svn: 318927
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:
All results are scored, we only process CodeCompletionStrings for the winners.
We now return CompletionList rather than CompletionItem[] (both are valid).
sortText is now based on CodeCompletionResult::orderedName (mostly the same).
This is the first clangd-only completion option, so plumbing changed.
It requires a small clangd patch (exposing CodeCompletionResult::orderedName).
(This can't usefully be enabled yet: we don't support server-side filtering)
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D39852
llvm-svn: 318287
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
Eric Liu