Summary:
This runs checks over a restricted subset of the TU:
- preprocessor callbacks just receive the truncated PP events that
occur when a preamble is used.
- ASTMatchers run only over the top-level decls in the main-file
This patch just turns on one simple check (bugprone-sizeof-expression)
with no configuration. Configuration is complex enough to warrant a separate patch
This depends on a patch allowing traversal to be restricted to a scope.
Reviewers: hokein
Subscribers: srhines, mgorny, ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54204
llvm-svn: 347036
Add support for "polymorphic" types to YAMLIO.
PolymorphicTraits can dynamically switch between other traits (Scalar, Map, or
Sequence). When inputting, the PolymorphicTraits type is told which type to
become, and when outputting the PolymorphicTraits type is asked which type it
currently is.
Also add support for TaggedScalarTraits to allow dynamically differentiating
between multiple scalar types using YAML tags.
Serialize empty maps as "{}" and empty sequences as "[]", so that types
are preserved when round-tripping PolymorphicTraits. This change has
equivalent semantics, but may break e.g. tests which compare output
verbatim.
Differential Revision: https://reviews.llvm.org/D48144
llvm-svn: 346884
Summary:
This would save us 8 bytes per ref, and buy us ~40MB in total
for llvm index (from ~300MB to ~260 MB).
The char pointer must be null-terminated, and llvm::StringSaver
guarantees it.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53427
llvm-svn: 346852
Summary:
Previously code completion did not work well for Objective-C methods
which contained multiple arguments as clangd did not expect to see
multiple typed-text chunks when handling code completion.
Note that even with this change, we do not consider selector fragments
from previous arguments to be part of the signature (although we
could in the future).
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, jfb, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53934
llvm-svn: 346836
Summary:
These get passed to HandleTopLevelDecl() if they happen to have been
deserialized for any reason. We don't want to treat them as part of the
main file.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54303
llvm-svn: 346503
Our testing didn't reflect reality: live clangd almost always uses a
preamble, and sometimes the preamble behaves differently.
This patch fixes a common test helper to be more realistic.
Preamble doesn't preserve information about which tokens come from the
command-line (this gets inlined into a source file). So remove logic
that attempts to treat symbols with such names differently.
A SymbolCollectorTest tries to verify that locals in headers are not
indexed, with preamble enabled this is only meaningful for locals of
auto-typed functions (otherwise the bodies aren't parsed).
Tests were relying on the fact that the findAnyDecl helper actually did expose
symbols from headers. Resolve by making all these functions consistently
able to find symbols in headers/preambles.
llvm-svn: 346488
Summary:
Clang's hierarchy is CompilerInstance -> DiagnosticsEngine -> DiagnosticConsumer.
(Ownership is optional/shared, but this structure is fairly clear).
Currently ClangTidyDiagnosticConsumer *owns* the DiagnosticsEngine:
- this inverts the hierarchy, which is confusing
- this means ClangTidyDiagnosticConsumer() mutates the passed-in context, which
is both surprising and limits flexibility
- it's not possible to use a different DiagnosticsEngine with ClangTidy
This means a little bit more code in the places ClangTidy is used standalone,
but more flexibility in using ClangTidy with other diagnostics configurations.
Reviewers: hokein
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D54033
llvm-svn: 346418
Summary:
Namespace references is less useful compared with other symbols, and
they contribute large part of the index. This patch drops them.
The number of refs is reduced from 5.4 million to 4.7 million.
| | Before | After |
|file size | 78 MB | 71MB |
|memory | 330MB | 300MB|
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54202
llvm-svn: 346319
Summary:
For example, when anonymous namespace is present, duplicated namespaces might be
generated for the enclosing namespace.
Reviewers: ilya-biryukov
Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54105
llvm-svn: 346224
Summary:
This allows us to deduplicate header symbols across TUs. File digests
are collects when collecting symbols/refs. And the index store deduplicates
file symbols based on the file digest.
Reviewers: sammccall, hokein
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53433
llvm-svn: 346221
Summary:
The new implementation is a GlobalCompilationDatabase that overlays a base.
Normally this is the directory-based CDB.
To preserve the behavior of compile_args_from=LSP, the base may be null.
The OverlayCDB is always present, and so the extensions to populate it
are always supported.
It also allows overriding the flags of the fallback command. This is
just unit-tested for now, but the plan is to expose this as an extension
on the initialize message. This addresses use cases like
https://github.com/thomasjo/atom-ide-cpp/issues/16
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53687
llvm-svn: 345970
Summary:
Currently ClangTidyContext::diag() sends the diagnostics to a
DiagnosticsEngine, which probably delegates to a ClangTidyDiagnosticsConsumer,
which is supposed to go back and populate ClangTidyContext::Errors.
After this patch, the diagnostics are stored in the ClangTidyDiagnosticsConsumer
itself and can be retrieved from there.
Why?
- the round-trip from context -> engine -> consumer -> context is confusing
and makes it harder to establish layering between these things.
- context does too many things, and makes it hard to use clang-tidy as a library
- everyone who actually wants the diagnostics has access to the ClangTidyDiagnosticsConsumer
The most natural implementation (ClangTidyDiagnosticsConsumer::take()
finalizes diagnostics) causes a test failure: clang-tidy-run-with-database.cpp
asserts that clang-tidy exits successfully when trying to process a file
that doesn't exist.
In clang-tidy today, this happens because finish() is never called, so the
diagnostic is never flushed. This looks like a bug to me.
For now, this patch carefully preserves that behavior, but I'll ping the
authors to see whether it's deliberate and worth preserving.
Reviewers: hokein
Subscribers: xazax.hun, cfe-commits, alexfh
Differential Revision: https://reviews.llvm.org/D53953
llvm-svn: 345961
Summary:
Add granular options for AST dumping, text printing and diagnostics.
This makes it possible to
* Have both diag and dump active at once
* Extend the output with other queryable content in the future.
Reviewers: aaron.ballman, pcc, ioeric, ilya-biryukov, klimek, sammccall
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52857
llvm-svn: 345522
Summary:
To enable this, TUScheduler has to provide a way to run async tasks without
needing a preamble or AST!
Reviewers: ilya-biryukov
Subscribers: javed.absar, ioeric, MaskRay, jkorous, arphaman, jfb, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53644
llvm-svn: 345268
Summary: This will make it possible to add non-exclusive mode output.
Reviewers: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D53501
llvm-svn: 345194
Summary: Future development can then dump other content than AST.
Reviewers: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D53500
llvm-svn: 345193
Summary:
These are available via qualifiers, but signal to noise level is low.
Keep required quailifier machinery around though, for cross-ns completion.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53571
llvm-svn: 345141
Summary:
CodeAction provides us with a standard way of representing fixes inline, so
use it, replacing our existing ad-hoc extension.
After this, it's easy to serialize diagnostics using the structured
toJSON/Protocol.h mechanism rather than assembling JSON ad-hoc.
Reviewers: hokein, arphaman
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53391
llvm-svn: 345119
Summary:
The goal is 8 bytes, which has a nonzero risk of collisions with huge indexes.
This patch should shake out any issues with truncation at all, we can lower
further later.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53587
llvm-svn: 345113
Summary:
See http://lists.llvm.org/pipermail/clangd-dev/2018-October/000171.html for context.
I kept the error (instead of downgrading to a log message) since the range lengths differing does indicate either a bug in the client or server range calculation or the buffers being out of sync (which both seems serious enough to me to be an error). If any existing clients aside from VSCode break they should only break when accidentally typing a Unicode character which should only be a minor nuisance for a little while until the bug is fixed in the respective client.
Patch by Daan De Meyer!
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53527
llvm-svn: 345020
Summary:
For example:
```
namespace util { class Base; }
namespace new {
namespace util { class Internal; }
}
namespace old {
util::Base b1;
}
```
When changing `old::` to `new::`, `util::` in namespace "new::" will conflict
with "new::util::" unless a leading "::" is added.
Reviewers: hokein
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D53489
llvm-svn: 344897
Standardize on the most common namespace setup in our *.cpp files:
using namespace llvm;
namespace clang {
namespace clangd {
void foo(StringRef) { ... }
And remove redundant llvm:: qualifiers. (Except for cases like
make_unique where this causes problems with std:: and ADL).
This choice is pretty arbitrary, but some broad consistency is nice.
This is going to conflict with everything. Sorry :-/
Squash the other configurations:
A)
using namespace llvm;
using namespace clang;
using namespace clangd;
void clangd::foo(StringRef);
This is in some of the older files. (It prevents accidentally defining a
new function instead of one in the header file, for what that's worth).
B)
namespace clang {
namespace clangd {
void foo(llvm::StringRef) { ... }
This is fine, but in practice the using directive often gets added over time.
C)
namespace clang {
namespace clangd {
using namespace llvm; // inside the namespace
This was pretty common, but is a bit misleading: name lookup preferrs
clang::clangd::foo > clang::foo > llvm:: foo (no matter where the using
directive is).
llvm-svn: 344850
Summary:
This is useful if using clang-query -f with a file containing multiple
matchers.
Reviewers: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52859
llvm-svn: 344840
Summary:
Rename instance variable to WorkspaceRoot to match what we call it internally.
Add fixme to set it automatically. Don't do it yet, clients have assumptions
that the constructor won't access the FS.
Don't second-guess the provided root.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53404
llvm-svn: 344787
Summary:
These are often not expected to be used directly e.g.
```
TEST_F(Fixture, X) {
^ // "Fixture_X_Test" expanded in the macro should be down ranked.
}
```
Only doing this for sema for now, as such symbols are mostly coming from sema
e.g. gtest macros expanded in the main file. We could also add a similar field
for the index symbol.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53374
llvm-svn: 344736
Summary:
This would buy us more memory. Using a 32-bits integer is enough for
most human-readable source code (up to 4M lines and 4K columns).
Previsouly, we used 8 bytes for a position, now 4 bytes, it would save
us 8 bytes for each Ref and each Symbol instance.
For LLVM-project binary index file, we save ~13% memory.
| Before | After |
| 412MB | 355MB |
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53363
llvm-svn: 344735
Makes bitcode tests line up with what's actually called in the tool.
Should fix the failing bot.
Also fixes a warning that was being thrown about initialization braces.
Differential Revision: https://reviews.llvm.org/D53381
llvm-svn: 344707
Summary:
This should make all-scope completion more usable. Scope proximity for
indexes will be added in followup patch.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53131
llvm-svn: 344688
Summary:
Add a flag to SymbolCollector to collect refs fdrom headers.
Note that we collect refs from headers in static index, and we don't do it for
dynamic index because of the preamble (we skip function body in preamble,
collecting it will result incomplete results).
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53322
llvm-svn: 344678
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
Add unit tests for Markdown generation.
This is part of a move to convert clang-doc's tests to a more
maintainable unit test framework, with a smaller number of integration
tests to maintain and more granular failure feedback.
Differential Revision: https://reviews.llvm.org/D53085
llvm-svn: 344654
Adds unit tests for the YAML generator library.
This is part of a move to convert clang-doc's tests to a more
maintainable unit test framework, with a smaller number of integration
tests to maintain and more granular failure feedback.
Differential Revision: https://reviews.llvm.org/D53084
llvm-svn: 344653
Adds unit tests for the merging logic in Respresentation.cpp.
This is part of a move to convert clang-doc's tests to a more
maintainable unit test framework, with a smaller number of integration
tests to maintain and more granular failure feedback.
Differential Revision: https://reviews.llvm.org/D53083
llvm-svn: 344652
Adds unit tests for the BitcodeWriter and BitcodeReader libraries.
This is part of a move to convert clang-doc's tests to a more
maintainable unit test framework, with a smaller number of integration
tests to maintain and more granular failure feedback.
Differential Revision: https://reviews.llvm.org/D53082
llvm-svn: 344651
Adds unit tests for the Serialize library.
This is part of a move to convert clang-doc's tests to a more
maintainable unit test framework, with a smaller number of integration
tests to maintain and more granular failure feedback.
Differential Revision: https://reviews.llvm.org/D53081
llvm-svn: 344650
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:
This is useful for symbo scope proximity, where down traversals from
the global scope if not desired.
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53317
llvm-svn: 344604
Summary:
One relatively boring bug: forgot to notify the CV after enqueue.
One much more fun bug: the thread member could access instance variables before
they were initialized. Although the thread was last in the init list, QueueCV
etc were listed after Thread in the class, so their default constructors raced
with the thread itself.
We have to get very unlucky to lose this race, I saw it 0.02% of the time.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D53313
llvm-svn: 344595
Summary:
Reuse the old -use-dex-index experiment flag for this.
To avoid breaking the tests, make Dex deduplicate symbols, addressing an old FIXME.
Reviewers: hokein
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53288
llvm-svn: 344594
Summary:
See tinyurl.com/clangd-automatic-index for design and goals.
Lots of limitations to keep this patch smallish, TODOs everywhere:
- no serialization to disk
- no changes to dynamic index, which now has a much simpler job
- no partitioning of symbols by file to avoid duplication of header symbols
- no reindexing of edited files
- only a single worker thread
- compilation database is slurped synchronously (doesn't scale)
- uses memindex, rebuilds after every file (should be dex, periodically)
It's not hooked up to ClangdServer/ClangdLSPServer yet: the layering
isn't clear (it should really be in ClangdServer, but ClangdLSPServer
has all the CDB interactions).
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D53032
llvm-svn: 344513
Summary:
Previously, SymbolCollector postfilters all references at the end to
find all references of interesting symbols.
It was incorrect when indxing main AST where we don't see locations
of symbol declarations and definitions in the main AST (as those are in
preamble AST).
The fix is to do earily check during collecting references.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53273
llvm-svn: 344507
This patch moves the virtual file system form clang to llvm so it can be
used by more projects.
Concretely the patch:
- Moves VirtualFileSystem.{h|cpp} from clang/Basic to llvm/Support.
- Moves the corresponding unit test from clang to llvm.
- Moves the vfs namespace from clang::vfs to llvm::vfs.
- Formats the lines affected by this change, mostly this is the result of
the added llvm namespace.
RFC on the mailing list:
http://lists.llvm.org/pipermail/llvm-dev/2018-October/126657.html
Differential revision: https://reviews.llvm.org/D52783
llvm-svn: 344140
Summary: Also add a few new test cases and a special case into handling of empty fixit ranges that collides with location of a diag.
Reviewers: sammccall, ilya-biryukov
Reviewed By: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D52889
llvm-svn: 344025
Calling getMacroArgExpansionLocation too early was causing
Lexer::getRawToken to do the wrong thing - lexing the macro name instead
of the arg contents.
Differential Revision: https://reviews.llvm.org/D52928
llvm-svn: 343844
Summary:
The bug being fixed: when a posting list doesn't exist in the index, it
was previously just dropped from the query rather than being treated as
empty. Now that we have the FALSE iterator, we can use it instead.
The query tree logic previously had a bunch of special cases to detect whether
subtrees are empty. Now we just naively build the whole tree, and rely
on the query optimizations to drop the trivial parts.
Finally, there was a bug in trigram generation: the empty query would
generate a single trigram "$$$" instead of no trigrams.
This had no effect (there was no posting list, so the other bug
cancelled it out). But we now have to fix this bug too.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52796
llvm-svn: 343802
Summary:
This allows inheriting from it, so index() can ga away and allowing
TestTU::index) to be fixed.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52250
llvm-svn: 343780
Summary:
Currently queries like "ab" can match identifiers like a_yellow_bee.
The value of allowing this for exactly one segment but no more seems dubious.
It costs ~3% of overall ram (~9% of posting list ram) and some quality.
Reviewers: ilya-biryukov, ioeric
Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52885
llvm-svn: 343777
Summary:
1) Instead of x$$ for a short-query trigram, just use x
2) Make rules more coherent: prefixes of length 1-2, and first char + next head
3) Fix Dex::fuzzyFind to mark results as incomplete, because
short-trigram rules only yield a subset of results.
Reviewers: ioeric
Subscribers: ilya-biryukov, jkorous, mgrang, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52808
llvm-svn: 343775
Summary:
The FALSE iterator will be used in a followup patch to fix a logic bug in Dex
(currently, tokens that don't have posting lists in the index are simply dropped
from the query, changing semantics).
It can usually be optimized away, so added the following opmitizations:
- simplify booleans inside AND/OR
- replace effectively-empty AND/OR with booleans
- flatten nested AND/ORs
While working on this, found a bug in the AND iterator: its constructor sync()
assumes that ReachedEnd is set if applicable, but the constructor never sets it.
This crashes if a non-first iterator is nonempty.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52789
llvm-svn: 343774
Summary:
handleDeclOccurrencce reports a canonical declartion, so stick to use
canonical declarations to determine whether a declaration is in the
target set.
Also fix a previous ref test which misses a matched label (it fails without this
patch).
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52871
llvm-svn: 343763
Summary:
It is possible to pass a file of commands to clang-query using the
command line option -f or --preload. Make it possible to write comments
in such files.
Reviewers: aaron.ballman
Reviewed By: aaron.ballman
Subscribers: mgorny, cfe-commits
Differential Revision: https://reviews.llvm.org/D52752
llvm-svn: 343666
Declaring a field with the same name as a type causes GCC to error out:
Dex.h:104:10: error: declaration of 'clang::clangd::dex::Corpus clang::clangd::dex::Dex::Corpus' [-fpermissive]
Corpus Corpus;
^
Iterator.h:127:7: error: changes meaning of 'Corpus' from 'class clang::clangd::dex::Corpus' [-fpermissive]
class Corpus {
llvm-svn: 343610
Summary:
- Corpus avoids having to pass size to the true iterator, and (soon) any
iterator that might optimize down to true.
- Shorten names of factory functions now they're scoped to the Corpus.
intersect() and unionOf() rather than createAnd() or createOr() as this
seems to read better to me, and fits with other short names. Opinion wanted!
- DEFAULT_BOOST_SCORE --> 1. This is a multiplier, don't obfuscate identity.
- Simplify variadic templates in Iterator.h
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52711
llvm-svn: 343589
Summary:
This makes it suitable for logging (which immediately found a bug, to
be fixed in the next patch...)
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52715
llvm-svn: 343580
Summary:
The file stats can be reused when preamble is reused (e.g. code
completion). It's safe to assume that cached status is not outdated as we
assume preamble files to remain unchanged.
On real file system, this made code completion ~20% faster on a measured file
(with big preamble). The preamble build time doesn't change much.
Reviewers: sammccall, ilya-biryukov
Reviewed By: sammccall
Subscribers: mgorny, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52419
llvm-svn: 343576
The `let` command was added in commit 045c15ba (Add new 'let' command to
bind arbitrary values into constants., 2014-04-23).
The `let` command and the non-existant `l` command were documented in
commit 233092a0 (Add 'let' to the help message., 2015-02-27).
Implement the `l` command now for completeness.
llvm-svn: 343533
Conditionally compile the parts of clang-tidy which depend on the static
analyzer.
Funnily enough, I made the patch to exclude this from the build in 2013,
and it was committed with the comment that the tool should not be fully
excluded, but only the parts of it which depend on the analyzer should
be excluded.
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20130617/081797.html
This commit implements that idea.
Reviewed By: aaron.ballman
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52334
llvm-svn: 343528
Summary:
When no scope qualifier is specified, allow completing index symbols
from any scope and insert proper automatically. This is still experimental and
hidden behind a flag.
Things missing:
- Scope proximity based scoring.
- FuzzyFind supports weighted scopes.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: kbobyrev, ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52364
llvm-svn: 343248
Summary:
If we have some range information coming from clang diagnostic, promote
that one even if it doesn't contain diagnostic location inside.
Reviewers: sammccall, ioeric
Reviewed By: ioeric
Subscribers: ilya-biryukov, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D52544
llvm-svn: 343197
Summary:
Interface is in one file, implementation in two as they have little in common.
A couple of ad-hoc YAML functions left exposed:
- symbol -> YAML I expect to keep for tools like dexp
- YAML -> symbol is used for the MR-style indexer, I think we can eliminate
this (merge-on-the-fly, else use a different serialization)
Reviewers: kbobyrev
Subscribers: mgorny, ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52453
llvm-svn: 342999
This patch implements Variable-length Byte compression of `PostingList`s
to sacrifice some performance for lower memory consumption.
`PostingList` compression and decompression was extensively tested using
fuzzer for multiple hours and runnning significant number of realistic
`FuzzyFindRequests`. AddressSanitizer and UndefinedBehaviorSanitizer
were used to ensure the correct behaviour.
Performance evaluation was conducted with recent LLVM symbol index (292k
symbols) and the collection of user-recorded queries (7751
`FuzzyFindRequest` JSON dumps):
| Metrics | Before| After | Change (%)
| ----- | ----- | ----- | -----
| Memory consumption (posting lists only), MB | 54.4 | 23.5 | -60%
| Time to process queries, sec | 7.70 | 9.4 | +25%
Reviewers: sammccall, ioeric
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D52300
llvm-svn: 342965
`Dex` should utilize `FuzzyFindRequest.RestrictForCodeCompletion` flags
and omit symbols not meant for code completion when asked for it.
The measurements below were conducted with setting
`FuzzyFindRequest.RestrictForCodeCompletion` to `true` (so that it's
more realistic). Sadly, the average latency goes down, I suspect that is
mostly because of the empty queries where the number of posting lists is
critical.
| Metrics | Before | After | Relative difference
| ----- | ----- | ----- | -----
| Cumulative query latency (7000 `FuzzyFindRequest`s over LLVM static index) | 6182735043 ns | 7202442053 ns | +16%
| Whole Index size | 81.24 MB | 81.79 MB | +0.6%
Out of 292252 symbols collected from LLVM codebase 136926 appear to be
restricted for code completion.
Reviewers: ioeric
Differential Revision: https://reviews.llvm.org/D52357
llvm-svn: 342866
Summary:
Pros:
o Loading macros from preamble for every completion is slow (see profile).
o Calculating macro USR is also slow (see profile).
o Sema can provide a lot of macro completion results (e.g. when filter is empty,
60k for some large TUs!).
Cons:
o Slight memory increase in dynamic index (~1%).
o Some extra work during preamble build (should be fine as preamble build and
indexAST is way slower).
Before:
{F7195645}
After:
{F7195646}
Reviewers: ilya-biryukov, sammccall
Reviewed By: sammccall
Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52078
llvm-svn: 342529
Summary:
FileIndex now provides explicit interfaces for preamble and main file updates.
This avoids growing parameter list when preamble and main symbols diverge
further (e.g. D52078). This also gets rid of the hack in `indexAST` that
inferred main file index based on `TopLevelDecls`.
Also separate `indexMainDecls` from `indexAST`.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52222
llvm-svn: 342460
Summary:
To stay fast, enable single-file-mode instead. This is fine since completions
in the preamble are simple.
The net effect for now is to suppress the spurious TopLevel completions when
completing inside the preamble.
Once Sema has include directive completion, this will be more important.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52071
llvm-svn: 342230
This patch abstracts `PostingList` interface and reuses existing
implementation. It will be used later to test different `PostingList`
representations.
No functionality change is introduced, this patch is mostly refactoring
so that the following patches could focus on functionality while not
being too hard to review.
Reviewed By: sammccall, ioeric
Differential Revision: https://reviews.llvm.org/D51982
llvm-svn: 342155
As discussed during D51860 review, it is better to use `llvm::Optional`
here as it has clear semantics which reflect intended behavior.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D52028
llvm-svn: 342138
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
Summary:
This is 2/2 of moving ExprMutationAnalyzer from clangtidy to clang/Analysis.
ExprMutationAnalyzer is moved to clang/Analysis in D51948.
This diff migrates existing usages within clangtidy to point to the new
location and remove the old copy of ExprMutationAnalyzer.
Reviewers: george.karpenkov, JonasToth
Reviewed By: george.karpenkov
Subscribers: mgorny, a.sidorin, Szelethus, cfe-commits
Differential Revision: https://reviews.llvm.org/D51950
llvm-svn: 342006
Summary:
This handles cases like this:
```
typedef int& IntRef;
void mutate(IntRef);
void f() {
int x;
mutate(x);
}
```
where the param type is a sugared type (`TypedefType`) instead of a
reference type directly.
Note that another category of similar but different cases are already
handled properly before:
```
typedef int Int;
void mutate(Int&);
void f() {
int x;
mutate(x);
}
```
Reviewers: aaron.ballman, alexfh, george.karpenkov
Subscribers: xazax.hun, a.sidorin, Szelethus, cfe-commits
Differential Revision: https://reviews.llvm.org/D50953
llvm-svn: 341986
Summary:
For smart pointers like std::unique_ptr which uniquely owns the
underlying object, treat the mutation of the pointee as mutation of the
smart pointer itself.
This gives better behavior for cases like this:
```
void f(std::vector<std::unique_ptr<Foo>> v) { // undesirable analyze result of `v` as not mutated.
for (auto& p : v) {
p->mutate(); // only const member function `operator->` is invoked on `p`
}
}
```
Reviewers: hokein, george.karpenkov
Subscribers: xazax.hun, a.sidorin, Szelethus, cfe-commits
Differential Revision: https://reviews.llvm.org/D50883
llvm-svn: 341967
This is the same as D50619 plus fixes for buildbot failures on windows.
The test failures on windows are caused by -fdelayed-template-parsing
and is fixed by forcing -fno-delayed-template-parsing on test cases that
requires AST for uninstantiated templates.
llvm-svn: 341891
Summary:
I have hit this the rough way, while trying to use this in D51870.
There is no particular point in storing the pointers, and moreover
the pointers are assumed to be non-null, and that assumption is not
enforced. If they are null, it won't be able to do anything good
with them anyway.
Initially i thought about simply adding asserts() that they are
not null, but taking/storing references looks like even cleaner solution?
Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=38888 | PR38888 ]]
Reviewers: JonasToth, shuaiwang, alexfh, george.karpenkov
Reviewed By: shuaiwang
Subscribers: xazax.hun, a.sidorin, Szelethus, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D51884
llvm-svn: 341854
Summary:
- If a function is unresolved, assume it mutates its arguments
- Follow unresolved member expressions for nested mutations
Reviewers: aaron.ballman, JonasToth, george.karpenkov
Subscribers: xazax.hun, a.sidorin, cfe-commits
Differential Revision: https://reviews.llvm.org/D50619
llvm-svn: 341848
Currently, `SymbolIndex::estimateMemoryUsage()` returns the "overhead"
estimate, i.e. the estimate of the Index data structure excluding
backing data (such as Symbol Slab and Reference Slab). This patch
propagates information about paired data size where necessary.
Reviewed By: ioeric, sammccall
Differential Revision: https://reviews.llvm.org/D51539
llvm-svn: 341800
This patch introduces `PathURI` Search Token kind and utilizes it to
uprank symbols which are defined in files with small distance to the
directory where the fuzzy find request is coming from (e.g. files user
is editing).
Reviewed By: ioeric
Reviewers: ioeric, sammccall
Differential Revision: https://reviews.llvm.org/D51481
llvm-svn: 341542
Summary:
GoToDefinition returns all declaration results (implicit/explicit) that are
in the same location, and the results are returned in arbitrary order.
Some LSP clients defaultly take the first result as the final result, which
might present a bad result (implicit decl) to users.
This patch ranks the result based on whether the declarations are
referenced explicitly/implicitly. We put explicit declarations first.
This also improves the "hover" (which just take the first result) feature
in some cases.
Reviewers: ilya-biryukov
Subscribers: kadircet, ioeric, MaskRay, jkorous, mgrang, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50438
llvm-svn: 341463
Summary:
This is intended to replace the current YAML format for general use.
It's ~10x more compact than YAML, and ~40% more compact than gzipped YAML:
llvmidx.riff = 20M, llvmidx.yaml = 272M, llvmidx.yaml.gz = 32M
It's also simpler/faster to read and write.
The format is a RIFF container (chunks of (type, size, data)) with:
- a compressed string table
- simple binary encoding of symbols (with varints for compactness)
It can be extended to include occurrences, Dex posting lists, etc.
There's no rich backwards-compatibility scheme, but a version number is included
so we can detect incompatible files and do ad-hoc back-compat.
Alternatives considered:
- compressed YAML or JSON: bulky and slow to load
- llvm bitstream: confusing model and libraries are hard to use. My attempt
produced slightly larger files, and the code was longer and slower.
- protobuf or similar: would be really nice (esp for back-compat) but the
dependency is a big hassle
- ad-hoc binary format without a container: it seems clear we're going
to add posting lists and occurrences here, and that they will benefit
from sharing a string table. The container makes it easy to debug
these pieces in isolation, and make them optional.
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, mgrang, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D51585
llvm-svn: 341375
Summary:
A few things that I noticed while merging the SwapIndex patch:
- SymbolOccurrences and particularly SymbolOccurrenceSlab are unwieldy names,
and these names appear *a lot*. Ref, RefSlab, etc seem clear enough
and read/format much better.
- The asymmetry between SymbolSlab and RefSlab (build() vs freeze()) is
confusing and irritating, and doesn't even save much code.
Avoiding RefSlab::Builder was my idea, but it was a bad one; add it.
- DenseMap<SymbolID, ArrayRef<Ref>> seems like a reasonable compromise for
constructing MemIndex - and means many less wasted allocations than the
current DenseMap<SymbolID, vector<Ref*>> for FileIndex, and none for
slabs.
- RefSlab::find() is not actually used for anything, so we can throw
away the DenseMap and keep the representation much more compact.
- A few naming/consistency fixes: e.g. Slabs,Refs -> Symbols,Refs.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, mgrang, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D51605
llvm-svn: 341368
Summary:
- DynamicIndex doesn't implement ParsingCallbacks, to make its role clearer.
ParsingCallbacks is a separate object owned by the receiving TUScheduler.
(I tried to get rid of the "index-like-object that doesn't implement index"
but it was too messy).
- Clarified(?) docs around DynamicIndex - fewer details up front, more details
inside.
- Exposed dynamic index from ClangdServer for memory monitoring and more
direct testing of its contents (actual tests not added here, wanted to get
this out for review)
- Removed a redundant and sligthly confusing filename param in a callback
Reviewers: ilya-biryukov
Subscribers: javed.absar, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D51221
llvm-svn: 341325
Summary:
This is now handled by a wrapper class SwapIndex, so MemIndex/DexIndex can be
immutable and focus on their job.
Old and busted:
I have a MemIndex, which holds a shared_ptr<vector<Symbol*>>, which keeps the
symbol slab alive. I update by calling build(shared_ptr<vector<Symbol*>>).
New hotness: I have a SwapIndex, which holds a unique_ptr<SymbolIndex>, which
holds a MemIndex, which holds a shared_ptr<void>, which keeps backing
data alive.
I update by building a new MemIndex and calling SwapIndex::reset().
Reviewers: kbobyrev, ioeric
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, mgrang, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D51422
llvm-svn: 341318
Summary:
Currently, a symbol can have only one #include header attached, which
might not work well if the symbol can be imported via different #includes depending
on where it's used. This patch stores multiple #include headers (with # references)
for each symbol, so that CodeCompletion can decide which include to insert.
In this patch, code completion simply picks the most popular include as the default inserted header. We also return all possible includes and their edits in the `CodeCompletion` results.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: mgrang, ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D51291
llvm-svn: 341304
SymbolCollector will be used for two cases:
- collect Symbol type only, used for indexing preamble AST.
- collect Symbol and SymbolOccurrences, used for indexing main AST.
For finding local references from the AST, we will implement it in other ways.
llvm-svn: 341208
Summary:
After code completion inserts a header, running signature help using the old
preamble will usually fail. So we add support for consistent preamble reads.
Reviewers: ilya-biryukov
Subscribers: javed.absar, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D51438
llvm-svn: 341076
Summary: Only accessible via the C++ API at the moment.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D51437
llvm-svn: 341065
This patch introduces iterator cost concept to improve the performance
of Dex query iterators (mainly, AND iterator). Benchmarks show that the
queries become ~10% faster.
Before
```
-------------------------------------------------------
Benchmark Time CPU Iteration
-------------------------------------------------------
DexAdHocQueries 5883074 ns 5883018 ns 117
DexRealQ 959904457 ns 959898507 ns 1
```
After
```
-------------------------------------------------------
Benchmark Time CPU Iteration
-------------------------------------------------------
DexAdHocQueries 5238403 ns 5238361 ns 130
DexRealQ 873275207 ns 873269453 ns 1
```
Reviewed by: sammccall
Differential Revision: https://reviews.llvm.org/D51310
llvm-svn: 341057
This patch introduces LIMIT iterator, which is very important for
improving the quality of search query. LIMIT iterators can be applied on
top of BOOST iterators to prevent populating query request with a huge
number of low-quality symbols.
Reviewed by: sammccall
Differential Revision: https://reviews.llvm.org/D51029
llvm-svn: 340605
Summary:
For index-based code completion, send an asynchronous speculative index
request, based on the index request for the last code completion on the same
file and the filter text typed before the cursor, before sema code completion
is invoked. This can reduce the code completion latency (by roughly latency of
sema code completion) if the speculative request is the same as the one
generated for the ongoing code completion from sema. As a sequence of code
completions often have the same scopes and proximity paths etc, this should be
effective for a number of code completions.
Trace with speculative index request:{F6997544}
Reviewers: hokein, ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: javed.absar, jfb, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D50962
llvm-svn: 340604
This patch prints information about built index size estimation to
verbose logs. This is useful for optimizing memory usage of DexIndex and
comparisons with MemIndex.
Reviewed by: sammccall
Differential Revision: https://reviews.llvm.org/D51154
llvm-svn: 340601
Summary:
Currently we match an include only if we are inside filename, with this patch we
will match whenever we are on the starting line of the include.
Reviewers: ilya-biryukov, hokein, ioeric
Reviewed By: ilya-biryukov
Subscribers: MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D51163
llvm-svn: 340539
Summary:
Whenever a code-completion is triggered within a class/struct/union looks at
base classes and figures out non-overriden virtual functions. Than suggests
completions for those.
Reviewers: ilya-biryukov, hokein, ioeric
Reviewed By: hokein
Subscribers: MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50898
llvm-svn: 340530
Summary:
We were handling the EnableFunctionArgSnippets only when we are producing LSP
response. Move that code into CompletionItem generation so that internal clients
can benefit from that as well.
Reviewers: ilya-biryukov, ioeric, hokein
Reviewed By: ilya-biryukov
Subscribers: MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D51102
llvm-svn: 340527
Some of them timeout on our buildbots in certain configurations.
The timeouts are there to avoid hanging indefinitely on deadlocks, so
the exact number we put there does not matter.
llvm-svn: 340523
This patch introduces BOOST iterator - a substantial block for efficient
and high-quality symbol retrieval. The concept of boosting allows
performing computationally inexpensive scoring on the query side so that
the final (expensive) scoring can only be applied on the items with the
highest preliminary score while eliminating the need to score too many
items.
Reviewed by: ilya-biryukov
Differential Revision: https://reviews.llvm.org/D50970
llvm-svn: 340409
Summary:
Will be used for updating the dynamic index on updates to the open files.
Currently we collect only information coming from the preamble
AST. This has a bunch of limitations:
- Dynamic index misses important information from the body of the
file, e.g. locations of definitions.
- XRefs cannot be collected at all, since we can only obtain full
information for the current file (preamble is parsed with skipped
function bodies, therefore not reliable).
This patch only adds the new callback, actually updates to the index
will be done in a follow-up patch.
Reviewers: hokein
Reviewed By: hokein
Subscribers: kadircet, javed.absar, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50847
llvm-svn: 340401
This patch is a proof-of-concept Dex index implementation. It has
several flaws, which don't allow replacing static MemIndex yet, such as:
* Not being able to handle queries of small size (less than 3 symbols);
a way to solve this is generating trigrams of smaller size and having
such incomplete trigrams in the index structure.
* Speed measurements: while manually editing files in Vim and requesting
autocompletion gives an impression that the performance is at least
comparable with the current static index, having actual numbers is
important because we don't want to hurt the users and roll out slow
code. Eric (@ioeric) suggested that we should only replace MemIndex as
soon as we have the evidence that this is not a regression in terms of
performance. An approach which is likely to be successful here is to
wait until we have benchmark library in the LLVM core repository, which
is something I have suggested in the LLVM mailing lists, received
positive feedback on and started working on. I will add a dependency as
soon as the suggested patch is out for a review (currently there's at
least one complication which is being addressed by
https://github.com/google/benchmark/pull/649). Key performance
improvements for iterators are sorting by cost and the limit iterator.
* Quality measurements: currently, boosting iterator and two-phase
lookup stage are not implemented, without these the quality is likely to
be worse than the current implementation can yield. Measuring quality is
tricky, but another suggestion in the offline discussion was that the
drop-in replacement should only happen after Boosting iterators
implementation (and subsequent query enhancement).
The proposed changes do not affect Clangd functionality or performance,
`DexIndex` is only used in unit tests and not in production code.
Reviewed by: ioeric
Differential Revision: https://reviews.llvm.org/D50337
llvm-svn: 340175
Proposed changes:
* Cleanup comments in `clangd/index/dex/Iterator.h`: Vim's `gq`
formatting added redundant spaces instead of newlines in few
places
* Few comments in `OrIterator` are wrong
* Use `EXPECT_TRUE(Condition)` instead of
`EXPECT_THAT(Condition, true)` (same with `EXPECT_FALSE`)
* Don't expose `dump()` method to the public by misplacing
`private:`
This patch does not affect functionality.
Reviewed by: ioeric
Differential Revision: https://reviews.llvm.org/D50956
llvm-svn: 340157
Summary:
Currently we only add parantheses to the functions if snippets are
enabled, which also inserts snippets for parameters into parantheses. Adding a
new option to put only parantheses. Also it moves the cursor within parantheses
or at the end of them by looking at whether completion item has any parameters
or not. Still requires snippets support on the client side.
Reviewers: ioeric, ilya-biryukov, hokein
Reviewed By: ioeric
Subscribers: MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50835
llvm-svn: 340040
Summary: This is a patch of add a testcase for https://reviews.llvm.org/D50628.
Reviewers: ilya-biryukov
Subscribers: javed.absar, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50627
llvm-svn: 340035
Summary:
Sema can only be used for documentation in the current file, other doc
comments should be fetched from the index.
Reviewers: hokein, ioeric, kadircet
Reviewed By: hokein, kadircet
Subscribers: MaskRay, jkorous, mgrang, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50727
llvm-svn: 340005
Summary:
Fix an inconsistent behavior of using `LastBuiltPreamble`/`NewPreamble`
in TUScheduler (see the test for details), AST should always use
NewPreamble. This patch makes LastBuiltPreamble always point to
NewPreamble.
Preamble rarely fails to build, even there are errors in headers, so we
assume it would not cause performace issue for code completion.
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: javed.absar, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50695
llvm-svn: 340001
This patch improves `dex::Iterator` string representation by
incorporating the information about the element which is currently being
pointed to by the `DocumentIterator`.
Reviewed by: ioeric
Differential Revision: https://reviews.llvm.org/D50689
llvm-svn: 339877
Summary:
To avoid producing very verbose output in substitutions involving
typedefs, e.g.
T -> std::vector<std::string>::iterator
gets turned into an unreadable mess when printed out for libstdc++,
result contains internal types (std::__Vector_iterator<...>) and
expanded well-defined typedefs (std::basic_string<char>).
Until we improve the presentation code in clang, going with
non-instantiated decls looks like a better UX trade-off.
Reviewers: hokein, ioeric, kadircet
Reviewed By: hokein
Subscribers: MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50645
llvm-svn: 339665
This patch handles trigram generation "short" identifiers and queries.
Trigram generator produces incomplete trigrams for short names so that
the same query iterator API can be used to match symbols which don't
have enough symbols to form a trigram and correctly handle queries which
also are not sufficient for generating a full trigram.
Reviewed by: ioeric
Differential revision: https://reviews.llvm.org/D50517
llvm-svn: 339548
Summary:
When compile_commands.json contains some source files expressed as
relative paths, we can get duplicate responses to findDefinitions. The
responses only differ by the URI, which are different versions of the
same file:
"result": [
{
...
"uri": "file:///home/emaisin/src/ls-interact/cpp-test/build/../src/first.h"
},
{
...
"uri": "file:///home/emaisin/src/ls-interact/cpp-test/src/first.h"
}
]
In getAbsoluteFilePath, we try to obtain the realpath of the FileEntry
by calling tryGetRealPathName. However, this can fail and return an
empty string. It may be bug a bug in clang, but in any case we should
fall back to computing it ourselves if it happens.
I changed getAbsoluteFilePath so that if tryGetRealPathName succeeds, we
return right away (a real path is always absolute). Otherwise, we try
to build an absolute path, as we did before, but we also call
VFS->getRealPath to make sure to get the canonical path (e.g. without
any ".." in it).
Reviewers: malaperle
Subscribers: hokein, ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48687
llvm-svn: 339483
This patch modifies `consume` function to allow retrieval of limited
number of symbols. This is the "cheap" implementation of top-level
limiting iterator. In the future we would like to have a complete limit
iterator implementation to insert it into the query subtrees, but in the
meantime this version would be enough for a fully-functional
proof-of-concept Dex implementation.
Reviewers: ioeric, ilya-biryukov
Reviewed by: ioeric
Differential Revision: https://reviews.llvm.org/D50500
llvm-svn: 339426
Summary:
This allows implementations like different symbol indexes to know what
the current active file is. For example, some customized index implementation
might decide to only return results for some files.
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: javed.absar, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50446
llvm-svn: 339320
Summary:
This is the first step of implementing Xrefs in clangd:
- add index interfaces, and related data structures.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D49658
llvm-svn: 339011
The diagnostic messages that are sent to the client from Clangd are now always
capitalized.
Differential Revision: https://reviews.llvm.org/D50154
llvm-svn: 338919
Summary:
After r338256, clangd stopped reporting diagnostics if WantDiags::No request
is followed by a WantDiags::Yes request but the AST can be reused.
Reviewers: ioeric
Reviewed By: ioeric
Subscribers: javed.absar, MaskRay, jkorous, arphaman, jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D50045
llvm-svn: 338361
The original Dex Iterators patch (https://reviews.llvm.org/rL338017)
caused problems for Clang 3.6 and Clang 3.7 due to the compiler bug
which prevented inferring template parameter (`Size`) in create(And|Or)?
functions. It was reverted in https://reviews.llvm.org/rL338054.
In this revision the mentioned helper functions were replaced with
variadic templated versions.
Proposed changes were tested on multiple compiler versions, including
Clang 3.6 which originally caused the failure.
llvm-svn: 338116
This patch introduces three essential types of query iterators:
`DocumentIterator`, `AndIterator`, `OrIterator`. It provides a
convenient API for query tree generation and serves as a building block
for the next generation symbol index - Dex. Currently, many
optimizations are missed to improve code readability and to serve as the
reference implementation. Potential improvements are briefly mentioned
in `FIXME`s and will be addressed in the following patches.
Dex RFC in the mailing list:
http://lists.llvm.org/pipermail/clangd-dev/2018-July/000022.html
Iterators, their applications and potential extensions are explained in
detail in the design proposal:
https://docs.google.com/document/d/1C-A6PGT6TynyaX4PXyExNMiGmJ2jL1UwV91Kyx11gOI/edit#heading=h.903u1zon9nkj
Reviewers: ioeric, sammccall, ilya-biryukov
Subscribers: cfe-commits, klimek, jfb, mgrang, mgorny, MaskRay, jkorous,
arphaman
Differential Revision: https://reviews.llvm.org/D49546
llvm-svn: 338017
Summary:
If the contents are the same, the update most likely comes from the
fact that compile commands were invalidated. In that case we want to
avoid rebuilds in case the compile commands are actually the same.
Reviewers: ioeric
Reviewed By: ioeric
Subscribers: simark, javed.absar, MaskRay, jkorous, arphaman, jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D49783
llvm-svn: 338012
Summary:
This has a shape to similar logarithm function but grows much slower for
large #usages.
Metrics: https://reviews.llvm.org/P8096
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: MaskRay, jkorous, arphaman, cfe-commits, sammccall
Differential Revision: https://reviews.llvm.org/D49780
llvm-svn: 337907
This patch introduces the core building block of the next-generation
Clangd symbol index - Dex. Search tokens are the keys in the inverted
index and represent a characteristic of a specific symbol: examples of
search token types (Token Namespaces) are
* Trigrams - these are essential for unqualified symbol name fuzzy
search * Scopes for filtering the symbols by the namespace * Paths, e.g.
these can be used to uprank symbols defined close to the edited file
This patch outlines the generic for such token namespaces, but only
implements trigram generation.
The intuition behind trigram generation algorithm is that each extracted
trigram is a valid sequence for Fuzzy Matcher jumps, proposed
implementation utilize existing FuzzyMatcher API for segmentation and
trigram extraction.
However, trigrams generation algorithm for the query string is different
from the previous one: it simply yields sequences of 3 consecutive
lowercased valid characters (letters, digits).
Dex RFC in the mailing list:
http://lists.llvm.org/pipermail/clangd-dev/2018-July/000022.html
The trigram generation techniques are described in detail in the
proposal:
https://docs.google.com/document/d/1C-A6PGT6TynyaX4PXyExNMiGmJ2jL1UwV91Kyx11gOI/edit#heading=h.903u1zon9nkj
Reviewers: sammccall, ioeric, ilya-biryukovA
Subscribers: cfe-commits, klimek, mgorny, MaskRay, jkorous, arphaman
Differential Revision: https://reviews.llvm.org/D49591
llvm-svn: 337901
Summary:
The following are metrics for explicit member access completions. There is no
noticeable impact on other completion types.
Before:
EXPLICIT_MEMBER_ACCESS
Total measurements: 24382
All measurements: MRR: 62.27 Top10: 80.21% Top-100: 94.48%
Full identifiers: MRR: 98.81 Top10: 99.89% Top-100: 99.95%
0-5 filter len:
MRR: 13.25 46.31 62.47 67.77 70.40 81.91
Top-10: 29% 74% 84% 91% 91% 97%
Top-100: 67% 99% 99% 99% 99% 100%
After:
EXPLICIT_MEMBER_ACCESS
Total measurements: 24382
All measurements: MRR: 63.18 Top10: 80.58% Top-100: 95.07%
Full identifiers: MRR: 98.79 Top10: 99.89% Top-100: 99.95%
0-5 filter len:
MRR: 13.84 48.39 63.55 68.83 71.28 82.64
Top-10: 30% 75% 84% 91% 91% 97%
Top-100: 70% 99% 99% 99% 99% 100%
* Top-N: wanted result is found in the first N completion results.
* MRR: Mean reciprocal rank.
Remark: the change seems to have minor positive impact. Although the improvement
is relatively small, down-ranking non-instance members in instance member access
should reduce noise in the completion results.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D49543
llvm-svn: 337681
Summary: This is intended to be used for indexing, e.g. in D49417
Reviewers: ioeric, omtcyfz
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D49540
llvm-svn: 337527
Having `using qualified::name;` for some symbol is an important signal
for clangd code completion as the user is more likely to use such
symbol. This patch helps to uprank the relevant symbols by saving
UsingShadowDecl in the new field of CodeCompletionResult and checking
whether the corresponding UsingShadowDecl is located in the main file
later in ClangD code completion routine. While the relative importance
of such signal is a subject to change in the future, this patch simply
bumps DeclProximity score to the value of 1.0 which should be enough for
now.
The patch was tested using
`$ ninja check-clang check-clang-tools`
No unexpected failures were noticed after running the relevant testsets.
Reviewers: sammccall, ioeric
Subscribers: MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D49012
llvm-svn: 336810
Summary:
Sema code complete in the recovery mode is generally useless. For many
cases, sema first completes in recovery context and then recovers to more useful
context, in which it's favorable to ignore results from recovery (as results are
often bad e.g. all builtin symbols and top-level symbols). There is also case
where only sema would fail to recover e.g. completions in excluded #if block.
Sema would try to give results, but the results are often useless (see the updated
excluded #if block test).
Reviewers: sammccall, ilya-biryukov
Subscribers: MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D49175
llvm-svn: 336801
Summary: This is not enabled in the global-symbol-builder or dynamic index yet.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D49028
llvm-svn: 336553
Summary:
The library has graduated from clangd to llvm/Support.
This is a mechanical change to move to the new API and remove the old one.
Main API changes:
- namespace clang::clangd::json --> llvm::json
- json::Expr --> json::Value
- Expr::asString() etc --> Value::getAsString() etc
- unsigned longs need a cast (due to r336541 adding lossless integer support)
Reviewers: ilya-biryukov
Subscribers: mgorny, ioeric, MaskRay, jkorous, omtcyfz, cfe-commits
Differential Revision: https://reviews.llvm.org/D49077
llvm-svn: 336549
Summary:
To avoid doing extra work of processing headers in the preamble
mutilple times in parallel.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: javed.absar, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48940
llvm-svn: 336538
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:
Following D48903 ([VirtualFileSystem] InMemoryFileSystem::status: Return
a Status with the requested name), the paths output by clang-move in the
FileToReplacements map may contain leading "./". For example, where we
would get "foo.h", we'll now get "./foo.h". This breaks the tests,
because we are doing exact string lookups in the FileToFileID and
Results maps (they contain "foo.h", but we search for "./foo.h").
To mitigate this, try to normalize a little bit the paths output by
clang-move to remove that leading "./".
This patch should be safe to merge before D48903, remove_dots will just
be a no-op.
Reviewers: ilya-biryukov, hokein
Reviewed By: hokein
Subscribers: ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D48951
llvm-svn: 336358
Summary: Surface it in the completion items C++ API, and when a flag is set.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48938
llvm-svn: 336309
Summary:
We now compute a distance from the main file to the symbol header, which
is a weighted count of:
- some number of #include traversals from source file --> included file
- some number of FS traversals from file --> parent directory
- some number of FS traversals from parent directory --> child file/dir
This calculation is performed in the appropriate URI scheme.
This means we'll get some proximity boost from header files in main-file
contexts, even when these are in different directory trees.
This extended file proximity model is not yet incorporated in the index
interface/implementation.
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48441
llvm-svn: 336177
Summary:
This allows hovering on keywords that refer to deduced types.
This should cover most useful cases. Not covered:
- auto template parameters: Since this can be instantiated with many types,
it would not be practical to show the types.
- Structured binding: This could be done later to show multiple deduced types
in the hover.
- auto:: (part of concepts): Outside the scope of this patch.
Signed-off-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48159
llvm-svn: 336119
Summary:
This provides more structured information that embedders can use for rendering.
ClangdLSPServer continues to call render(), so NFC.
The patch is:
- trivial changes to ClangdServer/ClangdLSPServer
- mostly-mechanical updates to CodeCompleteTests etc for the new API
- new direct tests of render() in CodeCompleteTests
- tiny cleanups to CodeCompletionItem (operator<< and missing initializers)
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48821
llvm-svn: 336094
Summary:
(Originally started as a clang-tidy check but there's already D45444 so shifted to just adding ExprMutationAnalyzer)
`ExprMutationAnalyzer` is a generally useful helper that can be used in different clang-tidy checks for checking whether a given expression is (potentially) mutated within a statement (typically the enclosing compound statement.)
This is a more general and more powerful/accurate version of isOnlyUsedAsConst, which is used in ForRangeCopyCheck, UnnecessaryCopyInitialization.
It should also be possible to construct checks like D45444 (suggest adding const to variable declaration) or https://bugs.llvm.org/show_bug.cgi?id=21981 (suggest adding const to member function) using this helper function.
This function is tested by itself and is intended to stay generally useful instead of tied to any particular check.
Reviewers: hokein, alexfh, aaron.ballman, ilya-biryukov, george.karpenkov
Reviewed By: aaron.ballman
Subscribers: lebedev.ri, shuaiwang, rnkovacs, hokein, alexfh, aaron.ballman, a.sidorin, Eugene.Zelenko, xazax.hun, JonasToth, klimek, mgorny, cfe-commits
Tags: #clang-tools-extra
Patch by Shuai Wang.
Differential Revision: https://reviews.llvm.org/D45679
llvm-svn: 335736
Summary:
Injected names being ranked too high was just a bug.
The high boost for keywords was intended, but was too much given how useless
keywords are. We should probably boost them on a case-by-case basis eventually.
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48368
llvm-svn: 335723
Summary:
Comments from namespaces that clangd produces are too noisy and often
not useful.
Namespaces have too many redecls and we don't have a good way of
determining which of the comments are relevant and which should be
ignored (e.g. because they come from code generators like the protobuf
compiler).
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48211
llvm-svn: 335718
Summary:
Instead of checking symbol name and container (scope) separately, check the
qualified name instead. This is much shorter and similar to how it is done
in the SymbolCollector tests.
Signed-off-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Reviewers: simark
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47847
llvm-svn: 335624
Summary:
Previously, the strings matched LSP completion pretty closely.
The completion label was a single string, for instance. This made
implementing completion itself easy but makes it hard to use the names
in other way, e.g. pretty-printed name in synthesized
documentation/hover.
It also limits our introspection into completion items, which can only
be as precise as the indexed symbols. This change is a prerequisite to
improvements to overload bundling which need to inspect e.g. signature
structure.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48475
llvm-svn: 335360
Summary:
The qualified name can be used to match a completion item to its corresponding
symbol. This can be useful for tools that measure code completion quality.
Qualified names are not precise for identifying symbols; we need to figure out a
better way to identify completion items.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48425
llvm-svn: 335334
Summary:
It's almost always identical to Name, and in fact we never used it (we used name
instead).
The only case where they differ is objc method selectors (foo: vs foo:bar:).
We can live with the latter for both name and filterText, so I've made that
change too.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48375
llvm-svn: 335321
Summary: This allows tools to examine symbols that would be collected in a symbol index. For example, a tool that measures index-based completion quality would be interested in references to symbols that are collected in the index.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48418
llvm-svn: 335218
Summary:
Some URI schemes require a hint path to be provided, and workspace root
path seems to be a good fit.
Reviewers: sammccall, malaperle
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48290
llvm-svn: 335035
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:
Adds a CodeCompleteOption to folds together compatible function/method overloads
into a single item. This feels pretty good (for editors with signatureHelp
support), but has limitations.
This happens in the code completion merge step, so there may be inconsistencies
(e.g. if only one overload made it into the index result list, no folding).
We don't want to bundle together completions that have different side-effects
(include insertion), because we can't constructo a coherent CompletionItem.
This may be confusing for users, as the reason for non-bundling may not
be immediately obvious. (Also, the implementation seems a little fragile)
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47957
llvm-svn: 334822
Summary:
Also move unittest: URI scheme to TestFS so that it can be shared by
different tests.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47935
llvm-svn: 334810
Summary:
This allows dynamic index to have consistent URI schemes with the
static index which can have customized URI schemes, which would make file
proximity scoring based on URIs easier.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47931
llvm-svn: 334809
Summary:
Like the following:
// -------
// =======
// *******
It does not cover all the cases, but those are definitely not very
useful.
Reviewers: sammccall, ioeric, hokein
Reviewed By: sammccall
Subscribers: MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48171
llvm-svn: 334807
Summary:
This is a small code change but vastly reduces noise in code completion results.
The intent of allowing this was to let [sc] ~ "strncpy" and [strcpy] ~ "strncpy"
however the benefits for unsegmented names aren't IMO worth the costs.
Test cases should be representative of the changes here.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47950
llvm-svn: 334712
Summary: These have few signals other than being keywords, so the boost is high.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48083
llvm-svn: 334711
ExprMutationAnalyzer is a generally useful helper that can be used in different clang-tidy checks for checking whether a given expression is (potentially) mutated within a statement (typically the enclosing compound statement.) This is a more general and more powerful/accurate version of isOnlyUsedAsConst, which is used in ForRangeCopyCheck, UnnecessaryCopyInitialization.
Patch by Shuai Wang
llvm-svn: 334604
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: Macros are terribly spammy at the moment and this offers some relief.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47936
llvm-svn: 334287
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: Fix a couple of bugs in tests an in Quality to keep tests passing.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47815
llvm-svn: 334089
Summary:
This signal is considered a relevance rather than a quality signal because it's
dependent on the query (the fact that it's completion, and implicitly the query
context).
This is part of the effort to reduce reliance on Sema priority, so we can have
consistent ranking between Index and Sema results.
Reviewers: ioeric
Subscribers: klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47762
llvm-svn: 334026
Summary:
This adds more symbols to the index:
- member variables and functions
- enum constants in scoped enums
The code completion behavior should remain intact but workspace symbols should
now provide much more useful symbols.
Other symbols should be considered such as the ones in "main files" (files not
being included) but this can be done separately as this introduces its fair
share of problems.
Signed-off-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Reviewers: ioeric, sammccall
Reviewed By: ioeric, sammccall
Subscribers: hokein, sammccall, jkorous, klimek, ilya-biryukov, jkorous-apple, ioeric, MaskRay, cfe-commits
Differential Revision: https://reviews.llvm.org/D44954
llvm-svn: 334017
Summary:
These decls are sometime used as the canonical declarations (e.g. for go-to-def),
which seems to be bad.
- friend decls that are not definitions should be ignored for indexing purposes
- this means they should never be selected as canonical decl
- if the friend decl is the only decl, then the symbol should not be indexed
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: mgorny, klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47623
llvm-svn: 333885
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:
Also fix USR generation for classes in unit tests. The previous USR
only works for class members, which happens to work when completing class name
inside the class, where constructors are suggested by sema.
Reviewers: sammccall, ilya-biryukov
Subscribers: klimek, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47466
llvm-svn: 333519
Summary:
They cause lots of deserialization and are not actually used.
The ParsedAST accessor that previously returned those was renamed from
getTopLevelDecls to getLocalTopLevelDecls in order to avoid
confusion.
This change should considerably improve the speed of findDefinition
and other features that try to find AST nodes, corresponding to the
locations in the source code.
Reviewers: ioeric, sammccall
Reviewed By: sammccall
Subscribers: klimek, mehdi_amini, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47331
llvm-svn: 333371
The commit includes two changes:
1. Set DisableFree to false when building the ParsedAST.
This is sane default, since clangd never wants to leak the AST.
2. Make sure CompilerInstance created in code completion is passed to
the FrontendAction::BeginSourceFile call.
We have to do this to make sure the memory buffers of remapped
files are properly freed.
Our tests do not produce any warnings under asan anymore.
The changes are mostly trivial, just moving the code around. So
sending without review.
llvm-svn: 333370
This fix is still quite fragile, the underlying problem is that the
code should not rely on source ranges coming from the preamble to be
correct when reading from the text buffers.
This is probably not possible to achieve in practice, so we would
probably have to keep the contents of old headers around for the
lifetime of the preamble.
llvm-svn: 333369
It turns out that our fix did not solve the problem completely and the
crash due to stale preamble is still there under asan.
Disabling the test for now, will reenable it when landing a proper fix
for the problem.
llvm-svn: 333280
Summary:
This is more efficient and avoids data races when reading files that
come from the preamble. The staleness can occur when reading a file
from disk that changed after the preamble was built. This can lead to
crashes, e.g. when parsing comments.
We do not to rely on symbols from the main file anyway, since any info
that those provide can always be taken from the AST.
Reviewers: ioeric, sammccall
Reviewed By: ioeric
Subscribers: malaperle, klimek, javed.absar, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47272
llvm-svn: 333196
Summary:
To fix a crash in code completion that occurrs when reading doc
comments from files that were updated after the preamble was
computed. In that case, the files on disk could've been changed and we
can't rely on finding the comment text with the same range anymore.
The current workaround is to not provide comments from the headers at
all and rely on the dynamic index instead.
A more principled solution would be to store contents of the files
read inside the preamble, but it is way harder to implement properly,
given that it would definitely increase the sizes of the preamble.
Together with D47272, this should fix all preamble-related crashes
we're aware of.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47274
llvm-svn: 333189
Summary:
This assumes that .inc files are supposed to be included via headers
that include them.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47187
llvm-svn: 333188
Summary:
Currently, we only handle the first callback from sema code completion
and ignore results from potential following callbacks. This causes
causes loss of completion results when multiple contexts are tried by Sema.
For example, we wouldn't get any completion result in the following completion
as the first attemped context is natural language which has no
candidate. The parser would backtrack and tried a completion with AST
semantic, which would find candidate "::x".
```
void f(const char*, int);
#define F(x) f(#x, x)
int x;
void main() {
F(::^);
}
```
To fix this, we only process a sema callback when it gives completion results or
the context supports index-based completion.
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: klimek, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47256
llvm-svn: 333174
Summary:
And add tests for the comment extraction code.
clangd will now show non-doxygen comments in completion for results
coming from Sema and Dynamic index.
Static index does not include the comments yet, I will enable it in
a separate commit after investigating which implications it has for
the size of the index.
Reviewers: sammccall, hokein, ioeric
Reviewed By: sammccall
Subscribers: klimek, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D46002
llvm-svn: 332460
Summary:
Previous implementation used to extract brief text from doxygen comments.
Brief text parsing slows down completion and is not suited for
non-doxygen comments.
This commit switches to providing comments that mimic the ones
originally written in the source code, doing minimal reindenting and
removing the comments markers to make the output more user-friendly.
It means we lose support for doxygen-specific features, e.g. extracting
brief text, but provide useful results for non-doxygen comments.
Switching the doxygen support back is an option, but I suggest to see
whether the current approach gives more useful results.
Reviewers: sammccall, hokein, ioeric
Reviewed By: sammccall
Subscribers: klimek, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D45999
llvm-svn: 332459
Summary:
This uses heuristics to identify private proto symbols. For example,
top-level symbols whose name contains "_" are considered private. These symbols
are not expected to be used by users.
Reviewers: ilya-biryukov, malaperle
Reviewed By: ilya-biryukov
Subscribers: sammccall, klimek, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D46751
llvm-svn: 332456
Summary: D46524 (rL332378) introduced a link failure when built with
`-DSHARED_LIB=ON`, which this patch fixes.
Reviewers: ioeric
Subscribers: klimek, mgorny, ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D46906
llvm-svn: 332438
Summary:
Code completion scoring was embedded in CodeComplete.cpp, which is bad:
- awkward to test. The mechanisms (extracting info from index/sema) can be
unit-tested well, the policy (scoring) should be quantitatively measured.
Neither was easily possible, and debugging was hard.
The intermediate signal struct makes this easier.
- hard to reuse. This is a bug in workspaceSymbols: it just presents the
results in the index order, which is not sorted in practice, it needs to rank
them!
Also, index implementations care about scoring (both query-dependent and
independent) in order to truncate result lists appropriately.
The main yak shaved here is the build() function that had 3 variants across
unit tests is unified in TestTU.h (rather than adding a 4th variant).
Reviewers: ilya-biryukov
Subscribers: klimek, mgorny, ioeric, MaskRay, jkorous, mgrang, cfe-commits
Differential Revision: https://reviews.llvm.org/D46524
llvm-svn: 332378
Summary:
o Remove IncludeInsertion LSP command.
o Populate include insertion edits synchromously in completion items.
o Share the code completion compiler instance and precompiled preamble to get existing inclusions in main file.
o Include insertion logic lives only in CodeComplete now.
o Use tooling::HeaderIncludes for inserting new includes.
o Refactored tests.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D46497
llvm-svn: 332363
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:
We used to query the index when completing after class qualifiers,
e.g. 'ClassName::^'. We should not do that for the same reasons we
don't query the index for member access expressions.
Reviewers: sammccall, ioeric
Reviewed By: sammccall
Subscribers: klimek, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D46795
llvm-svn: 332226
That broke every single .clang-tidy config out there
which happened to specify AnalyzeTemporaryDtors option:
YAML:5:24: error: unknown key 'AnalyzeTemporaryDtors'
AnalyzeTemporaryDtors: false
^~~~~
Error parsing <...>/.clang-tidy: Invalid argument
More so, that error isn't actually a error, the
clang-tidy does not exit with $? != 0, it continues
with the default config.
Surely this breakage isn't the intended behavior.
But if it is, feel free to revert this commit.
llvm-svn: 331822
Remove the `AnalyzeTemporaryDtors` option, since the corresponding
`cfg-temporary-dtors` option of the Static Analyzer defaults to `true` since
r326461.
llvm-svn: 331456
Summary:
This patch adds index support for GoToDefinition -- when we don't get the
definition from local AST, we query our index (Static&Dynamic) index to
get it.
Since we currently collect top-level symbol in the index, it doesn't support all
cases (e.g. class members), we will extend the index to include more symbols in
the future.
Reviewers: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, MaskRay, cfe-commits
Differential Revision: https://reviews.llvm.org/D45717
llvm-svn: 331189
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
We do now both:
- stop reformatting a sequence after a closing brace in more cases, in
order to not misindent after an incorrect closing brace
- format the closing brace when formatting the line containing the
opening brace
llvm-svn: 330580
Summary: This makes C++/objC not totally broken, without hurting C files too much.
Reviewers: ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D45442
llvm-svn: 330418
Summary:
When running the FileIndexTest, it shows "Failed to create an URI
for file XXX: not a valid absolute file path" warnings, and the
generated Symbols is missing Location information.
Reviewers: ioeric
Subscribers: klimek, ilya-biryukov, jkorous-apple, MaskRay, cfe-commits
Differential Revision: https://reviews.llvm.org/D45692
llvm-svn: 330182
Summary:
Previsouly, class completions items from the index were missing
template parameters in both the snippet and the label.
Reviewers: sammccall, hokein
Reviewed By: sammccall
Subscribers: klimek, jkorous-apple, ioeric, MaskRay, cfe-commits
Differential Revision: https://reviews.llvm.org/D45482
llvm-svn: 330004
Summary:
LSP is using Line & column as symbol position, clangd needs to transfer file
offset to Line & column when sending results back to LSP client, which is a high
cost, especially for finding workspace symbol -- we have to read the file
content from disk (if it isn't loaded in memory).
Saving these information in the index will make the clangd life eaiser.
Reviewers: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, MaskRay, cfe-commits
Differential Revision: https://reviews.llvm.org/D45513
llvm-svn: 329997
Summary:
By converting Replacements by AtomicChange, clang-apply-replacements is able like clang-tidy to automatically cleanup and format changes.
This should permits to close this ticket: https://bugs.llvm.org/show_bug.cgi?id=35051 and attempt to follow hints from https://reviews.llvm.org/D43500 comments.
Reviewers: klimek, ioeric
Reviewed By: ioeric
Subscribers: malcolm.parsons, mgorny, cfe-commits
Differential Revision: https://reviews.llvm.org/D43764
Patch by Jeremy Demeule.
llvm-svn: 329813
Summary:
It is possible that there will be two different instantiations of
the printer template for a given type and some tests could end up calling the
wrong (default) one. For example, it was seen in CodeCompleteTests.cpp when
printing CompletionItems that it would use the wrong printer because the default
is also instantiated in ClangdTests.cpp.
With this change, objects that were previously printed with a custom Printer now
get printed through the operator<< which is declared alongside the class.
This rule of the thumb should make it less error-prone.
Reviewers: simark, ilya-biryukov, sammccall
Reviewed By: simark, ilya-biryukov, sammccall
Subscribers: bkramer, hokein, sammccall, klimek, ilya-biryukov, jkorous-apple, ioeric, MaskRay, cfe-commits
Differential Revision: https://reviews.llvm.org/D44764
llvm-svn: 329725
LLVM_ON_WIN32 is set exactly with MSVC and MinGW (but not Cygwin) in
HandleLLVMOptions.cmake, which is where _WIN32 defined too. Just use the
default macro instead of a reinvented one.
See thread "Replacing LLVM_ON_WIN32 with just _WIN32" on llvm-dev and cfe-dev.
No intended behavior change.
llvm-svn: 329695
Explicitly include and build lib/Testing/Support from LLVM sources when
doing a stand-alone build. This is necessary since clangd tests started
to depend on LLVMTestingSupport library which is neither installed
by LLVM, nor built by clang itself.
Since completely separate build of clang-tools-extra is not supported,
this relies on variables set by clang CMakeLists.
Differential Revision: https://reviews.llvm.org/D45409
llvm-svn: 329594
Summary:
Calculating the include path from absolute file path does not always
work for all build system, e.g. bazel uses symlink as the build working
directory. The absolute file path from editor and clang is diverged from
each other. We need to address it properly in build sysmtem integration.
This patch worksarounds the issue by providing a hook in URI which allows
clients to provide their customized include path.
Reviewers: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, MaskRay, cfe-commits
Differential Revision: https://reviews.llvm.org/D45426
llvm-svn: 329578
Summary:
Fix bugs:
- don't count occurrences of decls where we don't spell the name
- findDefinitions at MACRO(^X) goes to the definition of MACRO
Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, MaskRay, cfe-commits
Differential Revision: https://reviews.llvm.org/D45356
llvm-svn: 329571
Summary:
Currently if a fix is attached directly to a diagnostic, we repeat the
diagnostic message as the fix message. From eyeballing the top diagnostics,
it seems describing the textual replacement would be much clearer.
e.g.
error: use of undeclared identifier 'goo'; did you mean 'foo'?
action before: use of undeclared identifier 'goo'; did you mean 'foo'?
action after: change 'goo' to 'foo'
Reviewers: ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, MaskRay, cfe-commits
Differential Revision: https://reviews.llvm.org/D45069
llvm-svn: 329090
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:
To implement incremental document syncing, we want to verify that the
ranges provided by the front-end are valid. Currently, positionToOffset
deals with invalid Positions by returning 0 or Code.size(), which are
two valid offsets. Instead, return an llvm:Expected<size_t> with an
error if the position is invalid.
According to the LSP, if the character value exceeds the number of
characters of the given line, it should default back to the end of the
line. It makes sense in some contexts to have this behavior, and does
not in other contexts. The AllowColumnsBeyondLineLength parameter
allows to decide what to do in that case, default back to the end of the
line, or return an error.
Reviewers: ilya-biryukov
Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D44673
llvm-svn: 328100
Summary:
When parser backtracks, we might receive multiple code completion
callbacks.
Previously we had a failing assertion there, now we take first results
and hope they are good enough.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D44567
llvm-svn: 327717
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
Summary:
Previously, the matcher matches a function call/ref multiple times, one
for each decl ancestor. This might cause problems. For example, in the following
case, `func()` would be matched once (with namespace context) before using decl is
seen and once after using decl is seeing, which would result in different conflicting
replacements as the first match would replace `func` with "ns::func" as it doesn't
know about the using decl.
```
namespace x {
namespace {
using ::ns::func;
void f() { func(); }
}
}
```
Switching from `hasDescendant` matching to `hasAncestor` matching solves the
problem.
Reviewers: hokein
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D44517
llvm-svn: 327629
Summary:
It was previously an easy way to concurrently access a mutable vfs,
which is a recipe for disaster.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, jkorous-apple, cfe-commits, ioeric
Differential Revision: https://reviews.llvm.org/D44463
llvm-svn: 327537
Summary:
Potential use case: argument go-to-definition result with symbol
information (e.g. function definition in cc file) that might not be in the AST.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D44305
llvm-svn: 327487
Summary: This also aligns with the behavior of declarations.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D44423
llvm-svn: 327401
Summary:
DeclrationAndMacrosFinder will find some declarations (not macro!) that are
referened inside the macro somehow, isSearchedLocation() is not sufficient, we
don't know whether the searched source location is macro or not.
Reviewers: ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D44293
llvm-svn: 327387
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 is an important ranking signal.
It's off for the dynamic index for now. Correspondingly, tell the index
infrastructure only to report declarations for the dynamic index.
Reviewers: ioeric, hokein
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D44315
llvm-svn: 327275
Summary: This also matches the range in symbol index.
Reviewers: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D44247
llvm-svn: 327129
Summary:
These have different USRs than the underlying entity, but are not typically
interesting in their own right and can be numerous (e.g. generated traits).
Reviewers: ioeric
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D44298
llvm-svn: 327127
Summary:
The intent was that [ar] doesn't match "FooBar"; the first character must match
a Head character (hard requirement, not just a low score).
This matches VSCode, and was "tested" but the tests were defective.
The tests expected matches("FooBar") to fail for lack of a match. But instead
it fails because the string should be annotated - matches("FooB[ar]").
This patch makes matches("FooBar") ignore annotations, as was intended.
Fixing the code to reject weak matches for the first char causes problems:
- [bre] no longer matches "HTMLBRElement".
We allow matching against an uppercase char even if we don't think it's head.
Only do this if there's at least one lowercase, to avoid triggering on MACROS
- [print] no longer matches "sprintf".
This is hard to fix without false positives (e.g. [int] vs "sprintf"])
This patch leaves this case broken. A future patch will add a dictionary
providing custom segmentation to common names from the standard library.
Fixed a couple of index tests that indirectly relied on broken fuzzy matching.
Added const in a couple of missing places for consistency with new code.
Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D44003
llvm-svn: 326721
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:
Don't actually start building ASTs for new revisions until either:
- 500ms have passed since the last revision, or
- we actually need the revision for something (or to unblock the queue)
In practice, this avoids the "first keystroke results in diagnostics" problem.
This is kind of awkward to test, and the test is pretty bad.
It can be observed nicely by capturing a trace, though.
Reviewers: hokein, ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D43648
llvm-svn: 326546
Summary:
Symbols with different canonical includes might be defined in the same header
(e.g. symbols defined in STL <iosfwd>). This patch adds support for mapping from
qualified symbol names to canonical headers and special mapping for symbols in <iosfwd>
Reviewers: sammccall, hokein
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D43869
llvm-svn: 326456
Summary:
Currently, we pick the first declaration of a symbol in a TU, which is considered
canonical in the clangIndex, as the canonical declaration in clangd. This causes
forward declarations that might appear in a random header to be used as a
canonical declaration, which is not desirable for features like go-to-declaration
or include insertion.
For example, for class X, we would consider the forward declaration in fwd.h to
be the canonical declaration, while the preferred canonical declaration should
be the actual definition in x.h.
```
// fwd.h
class X; // forward decl
// x.h
class X {};
```
This patch fixes the issue by making symbol collector favor the actual definition of
a TagDecl (i.e. class/struct/enum/union) found in a header file over the first seen
declarations in a TU. Other symbol types like functions are not handled because
using the first seen declarations as canonical declarations is usually a good
heuristic for them.
Reviewers: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D43823
llvm-svn: 326313
Summary:
I'm not sure whether there are any principal reasons why it returns raw owning pointer,
or it is just a old code that was not updated post-C++11.
I'm not too sure what testing i should do, because `check-all` is not error clean here for some reason,
but it does not //appear// asif those failures are related to these changes.
This is Clang-tools-extra part.
Clang part is D43779.
Reviewers: klimek, bkramer, alexfh, pcc
Reviewed By: alexfh
Subscribers: ioeric, jkorous-apple, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D43780
llvm-svn: 326202
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:
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:
Through the C++ API, we support for a given snapshot version:
- Yes: make sure we generate diagnostics for exactly this version
- Auto: generate eventually-consistent diagnostics for at least this version
- No: don't generate diagnostics for this version
Eventually auto should be debounced for better UX.
Through LSP, we force diagnostics for initial load (bypassing future debouncing)
and all updates follow the "auto" policy.
This is complicated to implement under the CancellationFlag design, so
rewrote that part to just inspect the queue instead.
It turns out we never pass None to the diagnostics callback, so remove Optional
from the signature. The questionable behavior of not invoking the callback at
all if CppFile::rebuild fails is not changed.
Reviewers: ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D43518
llvm-svn: 325774
Summary:
The new behaviors introduced by this patch:
o When include collection is enabled, we always set IncludeHeader field in Symbol
even if it's the same as FileURI in decl.
o Disable include collection in FileIndex which is currently only used to build
dynamic index. We should revisit when we actually want to use FileIndex to global
index.
o Code-completion only uses IncludeHeader to insert headers but not FileURI in
CanonicalDeclaration. This ensures that inserted headers are always canonicalized.
Note that include insertion can still be triggered for symbols that are already
included if they are merged from dynamic index and static index, but we would
only use includes that are already canonicalized (e.g. from static index).
Reason for change:
Collecting header includes in dynamic index enables inserting includes for headers
that are not indexed but opened in the editor. Comparing to inserting includes for
symbols in global/static index, this is nice-to-have but would probably require
non-trivial amount of work to get right. For example:
o Currently it's not easy to fully support CanonicalIncludes in dynamic index, given the way
we run dynamic index.
o It's also harder to reason about the correctness of include canonicalization for dynamic index
(i.e. symbols in the current file/TU) than static index where symbols are collected
offline and sanity check is possible before shipping to production.
o We have less control/flexibility over symbol info in the dynamic index
(e.g. URIs, path normalization), which could be used to help make decision when inserting includes.
As header collection (especially canonicalization) is relatively new, and enabling
it for dynamic index would immediately affect current users with only dynamic
index support, I propose we disable it for dynamic index for now to avoid
compromising other hot features like code completion and only support it for
static index where include insertion would likely to bring more value.
Reviewers: ilya-biryukov, sammccall, hokein
Subscribers: klimek, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D43550
llvm-svn: 325764
Summary:
o Avoid inserting a header include into the header itself.
o Avoid inserting non-header files (by not indexing symbols in main
files at all).
o Canonicalize include paths for symbols in dynamic index.
Reviewers: sammccall, ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: klimek, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D43462
llvm-svn: 325523
Summary:
This has a bit of a blast radius, but I think there's enough value in "forcing"
us to give names to these async tasks for debugging. Guessing about what
multithreaded code is doing is so unfun...
The "file" param attached to the tasks may seem to be redundant with the thread
names, but note that thread names are truncated to 15 chars on linux!
We'll be lucky to get the whole basename...
Reviewers: ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D43388
llvm-svn: 325480
Summary:
D42640 adds calls to `Preprocessor::addCommentHandler` in
`unittests/clangd/SymbolCollectorTests.cpp` and
`clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp` but does not
link `clangLex` library. This causes undefined reference errors when
built with `-DBUILD_SHARED_LIBS=ON`.
Reviewers: ioeric
Subscribers: klimek, mgorny, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D43437
llvm-svn: 325458
Sam McCall