locateSymbolAt (used in goToDeclaration) follows the
deduced type instead of failing to locate the declaration.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D92977
Boost in its infinite wisdom considers C: a parent of C:\, and we've
inherited that. This breaks the assumption that after canonicalizing a
path, the path parents are the directory's parents.
How about add hover information for `this` expr?
It seems useful to show related information about the class for `this` expr sometimes.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D92041
Using a MemoryBufferRef, If there is an error parsing, we can point the user to the location of the file.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D93024
SmallVector<T> with default size is now the recommended version (D92522).
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D92788
Figuring out whether the server is responding and debugging issues with remote
index setup is no easy task: add verbose logging for client side RPC requests
to relieve some pain.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D92181
This is a step towards making compile_commands.json reloadable.
The idea is:
- in addition to rare CDB loads we're soon going to have somewhat-rare CDB
reloads and fairly-common stat() of files to validate the CDB
- so stop doing all our work under a big global lock, instead using it to
acquire per-directory structures with their own locks
- each directory can be refreshed from disk every N seconds, like filecache
- avoid locking these at all in the most common case: directory has no CDB
Differential Revision: https://reviews.llvm.org/D92381
No changes to the tests themselves, other than some auto -> const auto
diagnostic fixes and formatting.
Differential Revision: https://reviews.llvm.org/D92939
Allow a `std::unique_ptr` to be moved into the an `IntrusiveRefCntPtr`,
and remove a couple of now-unnecessary `release()` calls.
Differential Revision: https://reviews.llvm.org/D92888
We can't expand lambda types anyway. Now we simply not offer the code
action instead of showing it and then returning an error in apply().
Differential Revision: https://reviews.llvm.org/D92847
This should address the FIXME about clang3.9 dervied to base unique_ptr constructor not working.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D91925
apply() will fail in those cases, so it's better to detect it in
prepare() already and hide code action from the user.
This was especially annoying on code bases that use a lot of
RETURN_IF_ERROR-like macros.
Differential Revision: https://reviews.llvm.org/D92408
We don't act as a language server for these files (e.g. don't get open/close
notifications for them), but just blindly publish diagnostics for them.
This works reasonably well in coc.nvim and vscode: they show up in the
workspace diagnostic list and when you open the file.
The only update after the file is reparsed, not as you type which is a bit
janky, but seems a lot better than nothing.
Fixes https://github.com/clangd/clangd/issues/614
Differential Revision: https://reviews.llvm.org/D92704
This can happen when, for example, merging results from an external
index that generates IncludeHeaders with full URI rather than just
literal include.
Differential Revision: https://reviews.llvm.org/D92494
The static_assert in "libcxx/include/memory" was the main offender here,
but then I figured I might as well `git grep -i instantat` and fix all
the instances I found. One was in user-facing HTML documentation;
the rest were in comments or tests.
This cache went away in 73fdd99870
This time, the cache is periodically validated against disk, so config
is still mostly "live".
The per-file cache reuses FileCache, but the tree-of-file-caches is
duplicated from ConfigProvider. .clangd, .clang-tidy, .clang-format, and
compile_commands.json all have this pattern, we should extract it at some point.
TODO for now though.
Differential Revision: https://reviews.llvm.org/D92133
Makes it easier to diagnose remote index issues with --debug-origins flag.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D92202
In some cases system includes extractions is not enough, we also need target specific defines.
The problems appears when clang default target is not the same as toolchain's one (GCC cross-compiler, MinGW on Windows).
After this patch `query-driver` also extracts target and adds `--target=<extracted target>` compile option.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D92012
Added some new ClangTidyOptionsProvider like classes designed for clangd work flow.
These providers are designed to source the options on the worker thread but in a thread safe manner.
This is done through making the options getter take a pointer to the filesystem used by the worker thread which natuarally is from a ThreadsafeFS.
Internal caching in the providers is also guarded.
The providers don't inherit from `ClangTidyOptionsProvider` instead they share a base class which is able to create a provider for the `ClangTidyContext` using a specific FileSystem.
This approach means one provider can be used for multiple contexts even though `ClangTidyContext` owns its provider.
Depends on D90531
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D91029
If the enum is a dependent type, we would crash somewhere in
getIntWidth(). -Wswitch diagnostic doesn't work on dependent enums
either.
Differential Revision: https://reviews.llvm.org/D92051
The plan is to use this to use this for .clang-format, .clang-tidy, and
compile_commands.json. (Currently the former two are reparsed every
time, and the latter is cached forever and changes are never seen).
Differential Revision: https://reviews.llvm.org/D88172
When type/function is defined in the middle of the file, previuosly we
would sometimes insert a "using" line before that definition, leading to
a compilation error. With this fix, we pick a point after such
definition in translation unit.
This is not a perfect solution. For example, it still doesn't handle
"using namespace" directives. It is, however, a significant improvement.
Differential Revision: https://reviews.llvm.org/D92053
Seeing an implicit this in the AST is pretty confusing I think.
While here, also mention when `this` is const.
Differential Revision: https://reviews.llvm.org/D91868
This was confusing, as testRoot on windows results in C:\\clangd-test
and testPath generated with posix explicitly still contained backslashes.
This patch ensures not only the relative part, but the whole final result
respects passed in Style.
Differential Revision: https://reviews.llvm.org/D91947
D71880 makes this dependency redundant and we can safely remove it. Tested for
both shared lib build and static lib build.
Reviewed By: hokein
Differential Revision: https://reviews.llvm.org/D91951
This patch introduces new canonicalization rules which are used for AST-based
rename in Clangd. By comparing two canonical declarations of inspected nodes,
Clangd determines whether both of them belong to the same entity user would
like to rename. Such functionality is relatively concise compared to the
Clang-Rename API that is used right now. It also helps to overcome the
limitations that Clang-Rename originally had and helps to eliminate several
classes of bugs.
Clangd AST-based rename currently relies on Clang-Rename which has design
limitations and also lacks some features. This patch breaks this dependency and
significantly reduces the amount of code to maintain (Clang-Rename is ~2000 LOC,
this patch is just <30 LOC of replacement code).
We eliminate technical debt by simultaneously
* Maintaining feature parity and ensuring no regressions
* Opening a straightforward path to improving existing rename bugs
* Making it possible to add more capabilities to rename feature which would not
be possible with Clang-Rename
Reviewed By: hokein
Differential Revision: https://reviews.llvm.org/D71880
Put project-aware-index between command-line specified static index and
ClangdServer indexes.
This also moves remote-index dependency from clangDaemon to ClangdMain
in an attempt to prevent cyclic dependency between clangDaemon and
remote-index-marshalling.
Differential Revision: https://reviews.llvm.org/D91860
An index implementation that can dispatch to a variety of indexes
depending on the file path. Enables clangd to work with multiple indexes in the
same instance, configured via config files.
Depends on D90749, D90746
Differential Revision: https://reviews.llvm.org/D90750
Compilation logic for External blocks. A few of the high level points:
- Requires exactly one-of File/Server at a time:
- Server is ignored in case of both, with a warning.
- Having none is an error, would render ExternalBlock void.
- Ensures mountpoint is an absolute path:
- Interprets it as relative to FragmentDirectory.
- Defaults to FragmentDirectory when empty.
- Marks Background as Skip.
Depends on D90748.
Differential Revision: https://reviews.llvm.org/D90749
Enable configuration of remote and static indexes through config files
in addition to command line arguments.
Differential Revision: https://reviews.llvm.org/D90748
Some of the buildbots were failing due to what seems to be them using a non c++14 compilant std::string implementation.
Since c++14 std::basic_string::append(const basic_string, size_t, size_t) has a defaulted 3rd paramater, but some of the build bots were reporting that it wasn't defaulted in their implementation.
First step of implementing clang-tidy configuration into clangd config.
This is just adding support for reading and verifying the clang tidy options from the config fragments.
No support is added for actually using the options within clang-tidy yet.
That will be added in a follow up as its a little more involved.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D90531
This is a mass-market version of the "dump AST" tweak we have behind
-hidden-features.
I think in this friendlier form it'll be useful for people outside clang
developers, which would justify making it a real feature.
It could be useful as a step towards lightweight clang-AST tooling in clangd
itself (like matcher-based search).
Advantages over the tweak:
- simplified information makes it more accessible, likely somewhat useful
without learning too much clang internals
- can be shown in a tree view
- structured information gives some options for presentation (e.g.
icon + two text colors + tooltip in vscode)
- clickable nodes jump to the corresponding code
Disadvantages:
- a bunch of code to handle different node types
- likely missing some important info vs dump-ast due to brevity/oversight
- may end up chasing/maintaining support for the long tail of nodes
Demo with VSCode support: https://imgur.com/a/6gKfyIV
Differential Revision: https://reviews.llvm.org/D89571
If there is a "-verify" flag in the compile command, clangd will crash
(hit the assertion) inside the `~VerifyDiagnosticConsumer` (Looks like our
compiler invocation doesn't setup correctly?).
This patch disables the verify mode as it is rarely useful in clangd.
Differential Revision: https://reviews.llvm.org/D91777
Address Sanitizer crashes on large allocations:
```c++
// Try to crash rather than hang on large allocation.
ScopedMemoryLimit MemLimit(1000 * 1024 * 1024); // 1GB
```
This adds a couple of missed tests from existing clang-rename ones and
introduces several new ones (e.g. static class member). This patch is required
to ensure feature parity for migration off Clang-Rename API D71880.
Reviewed By: hokein
Differential Revision: https://reviews.llvm.org/D91337
- Add verbose logging of payloads
- Add public logging of request summaries
- fix non-logging of messages in request scopes (oops!)
- add test for public/non-public logging, extending pipeline_helper a bit.
We've accumulated quite a lot of duplication in the request handlers by now.
I should factor that out, but not in this patch...
Differential Revision: https://reviews.llvm.org/D90654
Avoid requiring an actual MemoryBuffer in ComputePreambleBounds, when
a MemoryBufferRef will do just fine.
Differential Revision: https://reviews.llvm.org/D90890
Previously a corrupted index shard could cause us to resize arrays to an
arbitrary int32. This tends to be a huge number, and can render the
system unresponsive.
Instead, cap this at the amount of data that might reasonably be read
(e.g. the #bytes in the file). If the specified length is more than that,
assume the data is corrupt.
Differential Revision: https://reviews.llvm.org/D91258
This is a try to improve clangd-indexer tool performance:
- avoid processing already processed files.
- use different mutexes for different entities (e.g. do not block insertion of references while symbols are inserted)
Results for LLVM project indexing:
- before: ~30 minutes
- after: ~10 minutes
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D91051
https://reviews.llvm.org/D89670 changed the Ref structure, we need to
bump the version to invalidate all stored stale data, otherwise we will
get ` Error while reading shard: malformed or truncated refs` when
building the background index.
Differential Revision: https://reviews.llvm.org/D91131
* Even though remote index is still somewhat experimental, it can now be
used withing clangd itself: this should be the primary way of trying
it out
* Remove `protobuf-compiler` from list of needed Debian packages as it
`protobuf-compiler-grpc` already depends on it
* Bump recommended gRPC version to 1.32.3
We plan to eliminate error-prone and obsolete Clang-Rename API from Clangd. To
do that, we will introduce Decl canonicalization rules that will make renaming
code simpler and easier to maintain (D71880).
To ensure smooth transition to the new implementation, many Clang-Rename tests
will be adopted in Clangd to prevent any possible regressions. This patch is
the first in the chain of test migration patches. It improves existing tests
and adopts tests from Clang-Rename's alias, class and enum testing files.
Reviewed By: hokein
Differential Revision: https://reviews.llvm.org/D91102
With this patch, we reject the rename if the new name would conflict with
any other decls in the decl context of the renamed decl.
Differential Revision: https://reviews.llvm.org/D89790
There is not reason to check `std::make_unique<...>(..)` return value,
but `clangd::clang::loadIndex()` returns `nullptr` if an index file could not be loaded (e.g. incorrect version).
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D91049
If an enum has different names for the same constant, make sure only the first one declared gets added into the switch. Failing to do so results in a compiler error as 2 case labels can't represent the same value.
```
lang=c
enum Numbers{
One,
Un = One,
Two,
Deux = Two,
Three,
Trois = Three
};
// Old behaviour
switch (<Number>) {
case One:
case Un:
case Two:
case Duex:
case Three:
case Trois: break;
}
// New behaviour
switch (<Number>) {
case One:
case Two:
case Three: break;
}
```
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D90555
While here, clean up ParsedAST::build a bit:
- remove FIXMEs that were fixed long ago by ReplayPreamble
- remove redundant if, ClangTidyContext is not actually optional
Differential Revision: https://reviews.llvm.org/D90975
Probably not essential as afaik only one check uses this field. but still good to have consistent behaviour.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D90552
RemoteIndexClient implementations only depends on clangdSupport for
logging functionality and has no dependence on clangDeamon itself. This clears
out that link time dependency and enables depending on it in clangDeamon itself,
so that we can have other index implementations that makes use of the
RemoteIndex.
Differential Revision: https://reviews.llvm.org/D90746
This enables using the arrow operator to access members of the contained item.
```lang=c++
Located<std::string> X;
const char* CStr = X->c_str();
```
This is inline with how classes like `Optional` handle the arrow operator.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D90682
This introduces a mechanism for providers to interpret paths specified
in a fragment either as absolute or relative to fragment location.
This information should be used during compile stage to handle blocks correctly.
Differential Revision: https://reviews.llvm.org/D90270
The index server has access to potentially-sensitive information, e.g. a
sequence of fuzzyFind requests reveals details about code completions in the
editor which in turn reveals details about the code being edited.
This information is necessary to provide the service, and our intention[1] is it
should never be retained beyond the scope of the request (e.g. not logged).
At the same time, some log messages should be exposed:
- server startup/index reloads etc that don't pertain to a user request
- basic request logs (method, latency, #results, error code) for monitoring
- errors while handling requests, without request-specific data
The -log=public design accommodates these by allowing three types of logs:
- those not associated with any user RPC request (via context-propagation)
- those explicitly tagged as [public] in the log line
- logging of format strings only, with no interpolated data (error level only)
[1] Specifically: Google is likely to run public instances of this server
for LLVM and potentially other projects, they will run in this configuration.
The details agreed in a Google-internal privacy review.
As clangd developers, we'd encourage others to use this configuration for public
instances too.
Differential Revision: https://reviews.llvm.org/D90526
- pass required=False to use_clang(), as we don't need it
- fix required=False (which was unused and rotted):
- make derived substitutions conditional on it
- add a feature so we can disable tests that need it
- conditionally disable our one test that depends on %resource_dir.
This doesn't seem right from first principles, but isn't a big deal.
Differential Revision: https://reviews.llvm.org/D90528
The vendor will be prefixed to the "clangd" and can be an arbitrary
string, so account for it in the test.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D90517
We were default initializing SymbolIDs before, which would leave
indeterminate values in underlying std::array.
This patch updates the underlying data initalization to be value-init and adds a
way to check for validness of a SymbolID.
Differential Revision: https://reviews.llvm.org/D90397
Introduce a separate thread that will kill `clangd-index-server` after 10 seconds regardless. This helps shut down the test if the server hangs and `stderr.readline()` does not contain inititalizatiton message. It prevents "necessary" waiting delay for the server warm-up and only introduces additional delay if the test fails.
It also makes use of `subprocess.Popen.kill()` which is a portable way of handling process shutdown and avoids using signals.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D90590
Google test matcher `DeclKind` uses `NamedDecl::getDeclKindName()` to compare its result with expected declaration name.
Both, returned value of this function and the expected kind name argument have type `const char *`, so this matcher effectively
compares two pointers instead of the respective strings.
The test was passing on most platforms because compilers mostly were able to coalesce these string literals.
Patch By: Ilya Golovenko
Reviewed By: hokein
Differential Revision: https://reviews.llvm.org/D90384
With every incremental change, one needs to check-in new model upstream.
This also significantly increases the size of the git repo with every
new model.
Testing and comparing the old and previous model is also not possible as
we run only a single model at any point.
One solution is to have a "staging" decision forest which can be
injected into clangd without pushing it to upstream. Compare the
performance of the staging model with the live model. After a couple of
enhancements have been done to staging model, we can then replace the
live model upstream with the staging model. This reduces upstream churn
and also allows us to compare models with current baseline model.
This is done by having a callback in CodeCompleteOptions which is called
only when we want to use a decision forest ranking model. This allows us
to inject different completion model internally.
Differential Revision: https://reviews.llvm.org/D90014
This is an initial attempt to start using Syntax Trees in clangd while improving state of folding ranges feature and experimenting with Syntax Tree capabilities.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D88553
Both `SymbolKind` and `indexSymbolKindToSymbolKind` support constructors and
separate them into a different category from regular methods.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D89935
Previous attempt (15f6bad6d7) introduced
add_dependencies but unfortunately it does not actually add a dependency
between RemoteIndexProto and RemoteIndexServiceProto. This is likely due
to some requirements of it that clang_add_library violates.
As a workaround, we will link RemoteIndexProto library to
RemoteIndexServiceProto which is logical because the library can not be
without linking to RemoteIndexProto anyway.
TestWorkspace allows easily writing tests involving multiple
files that can have inclusion relationships between them.
BackgroundIndexTest.RelationsMultiFile is refactored to use
TestWorkspace, and moved to FileIndexTest as it no longer
depends on BackgroundIndex.
Differential Revision: https://reviews.llvm.org/D89297
In `ReplayPreamble::replay`, use `getFileRef` instead of `getFile`, and
then use that `FileEntryRef` later to avoid needing
`FileEntryRef::FileEntryRef`. The latter is going to become private to
`FileManager` in a later commit.
We only need to version these messages if they actually diverge.
Unlike the service, the namespace name isn't part of the wire format.
clangd-index-server was broken by 81e5f298c4
as the namespace names weren't updated there, this fixes it (by adding
them for the service, and not requiring them elsewhere).
This allows it to have a separate namespace (grpc versioned service) without
putting versioning info on all of the other protos (before we need it).
clang-index-server is still broken (from 81e5f298c4).
Differential Revision: https://reviews.llvm.org/D90031
`llvm::sys::path` is used a lot in the remote index marshalling code. We can save space by avoiding spelling it out explicitly for most functions and times.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D90016
This allows us to check whether enum field is actually sent over the wire or missing.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D89882
And also introduce Protobuf package versioning, it will help to deal
with breaking changes. Inroducing package version itself is a breaking
change, clients and servers need to be updated.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D89862
Nullability annotations are implmented using attributes; previusly
clangd would skip over AttributedTypeLoc since their location
points to the attribute instead of the modified type.
Also add some test cases for this.
Differential Revision: https://reviews.llvm.org/D89579
The patch adjusts the existing `llvm::DenseMap<unsigned, T>` and
`llvm::DenseSet<unsigned>` objects that store source locations, so
that they use `SourceLocation` directly instead of `unsigned`.
This patch relies on the `DenseMapInfo` trait added in D89719.
It also replaces the construction of `SourceLocation` objects from
the constants -1 and -2 with calls to the trait's methods `getEmptyKey`
and `getTombstoneKey` where appropriate.
Reviewed By: dexonsmith
Differential Revision: https://reviews.llvm.org/D69840
Without this patch 6 marshalling tests fail on Windows.
This patch contains the following changes:
- Allow paths with Windows slashes (convert to the POSIX style instead of assertion)
- Add support for URI with Windows path.
- Change the value of the second parameter of several `llvm::sys::path::convert_to_slash()` calls: we should use `windows` instead of `posix` to ensure UNIX slashes in the path.
- Port `RemoteMarshallingTest::IncludeHeaderURI` test to Windows.
Reviewed By: kbobyrev
Differential Revision: https://reviews.llvm.org/D89529
Update `Lexer` / `Lexer::Lexer` to use `MemoryBufferRef` instead of
`MemoryBuffer*`. Callers that were acquiring a `MemoryBuffer*` via
`SourceManager::getBuffer` were updated, such that if they checked
`Invalid` they use `getBufferOrNone` and otherwise `getBufferOrFake`.
Differential Revision: https://reviews.llvm.org/D89398
This was causing duplicate `symbols` components on the path as both the
edge from an index to filesymbols and filesymbols to symbolslabs were named
symbols.
Differential Revision: https://reviews.llvm.org/D89685
Performs a detailed profiling of clangd lsp server and conveys the
result to the client as a json object. It is of the form:
{
"_self": 0,
"_total": 8,
"child1": {
"_self": 4,
"_total": 4,
}
"child2": {
"_self": 2,
"_total": 4,
"child_deep": {
"_self": 2,
"_total": 2,
}
}
}
Differential Revision: https://reviews.llvm.org/D89277
Update clang-tools-extra, clang/tools, clang/unittests to migrate from
`SourceManager::getBuffer`, which returns an always dereferenceable
`MemoryBuffer*`, to `getBufferOrNone` or `getBufferOrFake`, both of
which return a `MemoryBufferRef`, depending on whether the call site was
checking for validity of the buffer. No functionality change intended.
Differential Revision: https://reviews.llvm.org/D89416
so that we could start experiment for C.
Previously, these flags in clangd were only meaningful for C++. We need
to flip them for C, this patch repurpose these flags.
- if true, just set it.
- if false, just respect the value in clang.
this would allow us to keep flags on for C++, and optionally flip them on for C.
Differential Revision: https://reviews.llvm.org/D89233
Given the following VarTemplateDecl AST,
```
VarTemplateDecl col:26 X
|-TemplateTypeParmDecl typename depth 0 index 0
`-VarDecl X 'bool' cinit
`-CXXBoolLiteralExpr 'bool' true
```
previously, we returned the VarDecl as the top-level decl, which was not
correct, the top-level decl should be VarTemplateDecl.
Differential Revision: https://reviews.llvm.org/D89098
This will enable queries like "clangd::" to find symbols under clangd
namespace, without requiring full "clang::clangd::" qualification.
Since Fuzzyfind performs the search under all scopes and only boosts the symbols
from relevant namespaces, we might get symbols from non-matching namespaces.
This patch chooses to drop those as they clearly do not match the query.
Fixes https://github.com/clangd/clangd/issues/550.
Differential Revision: https://reviews.llvm.org/D88814
This patch introduces hoisting detection logic into prepare state with
a partial AST traversal of the enclosing function.
We had some complaints from the users about this code action being almost always
available but failing most of the time. Hopefully this should reduce that noise.
The latency/correctness tradeoff is a bunch of hand-waving, but at least today
we don't have any other actions that are available on selection of statements,
so when we get to do the traversal it is quite likely that all the other checks
will bail out early. But this is still up for discussion, I am happy to abandon
the patch if you believe this is not practical.
Differential Revision: https://reviews.llvm.org/D85354
Unreachable file distances are represented as
`std::numeric_limits<unsigned>::max()`.
The previous dataset recorded the signals as `signed int` capturing this default
value as `-1`.
A new dataset was regenerated and a new model is trained that
interprets this unreachable as the intended value.
Distribution of `SymbolScopeDistance`:
Value Normalised Frequency
0 46.6184
4294967295 29.5342
6 14.5666
4 6.4433
2 1.4534
8 0.5760
10 0.3581
....
Distribution of `FileProximityDistance`:
Value Normalised Frequency
4294967295 39.9378
12 5.1997
14 4.9828
15 4.4221
16 4.3820
13 4.2765
17 3.8957
11 3.6387
19 3.4799
18 3.4076
....
Differential Revision: https://reviews.llvm.org/D89035
If the NewName is provided, prepareRename would perform a name
validation.
The motivation is to allow our internal embeder implement the customized
"canRenameInto" functionality on top of prepareRename.
Differential Revision: https://reviews.llvm.org/D88881
Up until now, we relied on matching the filename.
This depends on unstable details of libstdc++ and doesn't work well on other
stdlibs. Also we'd like to remove it (see D88204).
Differential Revision: https://reviews.llvm.org/D88885
The default value is 1.3f, but it was cast to true, which is not a good
base for code completion score.
Differential Revision: https://reviews.llvm.org/D88970
With this patch, we don't treat `using ns::X` as a first-class declaration like `using Z = ns::Y`, reference to X that goes through this using-decl is considered a direct reference (without the Underlying bit).
Fix the workaround in https://reviews.llvm.org/D87225 and https://reviews.llvm.org/D74054.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D88472
The protocol doesn't really incorporate ranking.
As with code completion, most clients respect what the server sends, but
VSCode re-ranks items, with predictable results.
See https://github.com/clangd/vscode-clangd/issues/81
There's no filterText field so we may be unable to construct a good workaround.
But expose the score so we may be able to do this on the client in future.
Differential Revision: https://reviews.llvm.org/D88844
several changes:
- return a structure result in rename API;
- prepareRename now returns more information (main-file occurrences);
- remove the duplicated detecting-touch-identifier code in prepareRename (which is implemented in rename API);
Differential Revision: https://reviews.llvm.org/D88634
It fixes the -Wswitch warning, though we mark it as a fix even if that is off.
This makes it the "recommended" action on an incomplete switch, which seems OK.
Differential Revision: https://reviews.llvm.org/D88726
Intent was a nice idea but it ends up being a bit awkward/heavyweight
without adding much.
In particular, it makes it hard to implement `CodeActionParams.only` properly
(there's an inheritance hierarchy for kinds).
Differential Revision: https://reviews.llvm.org/D88427
This allows us MSAN to instrument this function. Previous version is not
instrumentable due to it shear volume.
Differential Revision: https://reviews.llvm.org/D88536
This is a tool to simply parse a file as clangd would, and run some
common features (code actions, go-to-definition, hover) in an attempt to
trigger or reproduce crashes, error diagnostics, etc.
This is easier and more predictable than loading the file in clangd, because:
- there's no editor/plugin variation to worry about
- there's no accidental variation of user behavior or other extraneous requests
- we trigger features at every token, rather than guessing
- everything is synchronoous, logs are easier to reason about
- it's easier to (get users to) capture logs when running on the command-line
This is a fairly lightweight variant of this idea.
We could do a lot more with it, and maybe we should.
But I can't in the near future, and experience will tell us if we made
the right tradeoffs and if it's worth investing further.
Differential Revision: https://reviews.llvm.org/D88338
Relative paths received from the server are always in posix style. So
we need to ensure they are relative using that style, and not the native one.
Differential Revision: https://reviews.llvm.org/D88507
In preparation for making moving TweakFilter from ClangdServer::Options to
a ClangdLSPServer option, and letting it vary per-request.
(In order to implement CodeActionParams.only)
Also a general overdue cleanup.
Differential Revision: https://reviews.llvm.org/D88470
Extend the Trivial setter documentation to support cases where the value is moved into a field using `std::move`.
Reviewed By: sammccall, kadircet
Differential Revision: https://reviews.llvm.org/D88297
As @kadircet mentions in D84912#2184144, `findNearbyIdentifier()` traverses the whole file if there is no identifier for the word.
This patch ensures give up after 2^N lines in any case.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D87891
MSAN build times out for generated DecisionForest inference runtime.
A solution worth trying is splitting the function into 300 smaller
functions and then re-enable msan.
For now we are disabling instrumentation for the generated function.
Differential Revision: https://reviews.llvm.org/D88495
Improve the recently-added PopulateSwitch tweak to work on non-empty switches.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D88434
Since we have 2 scoring functions (heuristics and decision forest),
renaming the existing evaluate() function to be more descriptive of the
Heuristics being evaluated in it.
Differential Revision: https://reviews.llvm.org/D88431
By default clangd will score a code completion item using heuristics model.
Scoring can be done by Decision Forest model by passing `--ranking_model=decision_forest` to
clangd.
Features omitted from the model:
- `NameMatch` is excluded because the final score must be multiplicative in `NameMatch` to allow rescoring by the editor.
- `NeedsFixIts` is excluded because the generating dataset that needs 'fixits' is non-trivial.
There are multiple ways (heuristics) to combine the above two features with the prediction of the DF:
- `NeedsFixIts` is used as is with a penalty of `0.5`.
Various alternatives of combining NameMatch `N` and Decision forest Prediction `P`
- N * scale(P, 0, 1): Linearly scale the output of model to range [0, 1]
- N * a^P:
- More natural: Prediction of each Decision Tree can be considered as a multiplicative boost (like NameMatch)
- Ordering is independent of the absolute value of P. Order of two items is proportional to `a^{difference in model prediction score}`. Higher `a` gives higher weightage to model output as compared to NameMatch score.
Baseline MRR = 0.619
MRR for various combinations:
N * P = 0.6346, advantage%=2.5768
N * 1.1^P = 0.6600, advantage%=6.6853
N * **1.2**^P = 0.6669, advantage%=**7.8005**
N * **1.3**^P = 0.6668, advantage%=**7.7795**
N * **1.4**^P = 0.6659, advantage%=**7.6270**
N * 1.5^P = 0.6646, advantage%=7.4200
N * 1.6^P = 0.6636, advantage%=7.2671
N * 1.7^P = 0.6629, advantage%=7.1450
N * 2^P = 0.6612, advantage%=6.8673
N * 2.5^P = 0.6598, advantage%=6.6491
N * 3^P = 0.6590, advantage%=6.5242
N * scaled[0, 1] = 0.6465, advantage%=4.5054
Differential Revision: https://reviews.llvm.org/D88281
Replaces the dummy CodeCompletion model with a trained DecisionForest
model.
The features.json needs to be manually curated specifying the features
to be used. This is a one-time cost and does not change if the model
changes until we decide to add/remove features.
Differential Revision: https://reviews.llvm.org/D88071
Add a tweak that populates an empty switch statement of an enumeration type with all of the enumerators of that type.
Before:
```
enum Color { RED, GREEN, BLUE };
void f(Color color) {
switch (color) {}
}
```
After:
```
enum Color { RED, GREEN, BLUE };
void f(Color color) {
switch (color) {
case RED:
case GREEN:
case BLUE:
break;
}
}
```
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D88383
Translating between JSON objects and C++ strutctures is common.
From experience in clangd, fromJSON/ObjectMapper work well and save a lot of
code, but aren't adopted elsewhere at least partly due to total lack of error
reporting beyond "ok"/"bad".
The recently-added error model should be rich enough for most applications.
It requires tracking the path within the root object and reporting local
errors at appropriate places.
To do this, we exploit the fact that the call graph of recursive
parse functions mirror the structure of the JSON itself.
The current path is represented as a linked list of segments, each of which is
on the stack as a parameter. Concretely, fromJSON now looks like:
bool fromJSON(const Value&, T&, Path);
Beyond the signature change, this is reasonably unobtrusive: building
the path segments is mostly handled by ObjectMapper and the vector<T> fromJSON.
However the root caller of fromJSON must now create a Root object to
store the errors, which is a little clunky.
I've added high-level parse<T>(StringRef) -> Expected<T>, but it's not
general enough to be the primary interface I think (at least, not usable in
clangd).
All existing users (mostly just clangd) are updated in this patch,
making this change backwards-compatible is a bit hairy.
Differential Revision: https://reviews.llvm.org/D88103
Current implementation of heuristic-based scoring function also contains
computation of derived signals (e.g. whether name contains a word from
context, computing file distances, scope distances.)
This is an attempt to separate out the logic for computation of derived
signals from the scoring function.
This will allow us to have a clean API for scoring functions that will
take only concrete code completion signals as input.
Differential Revision: https://reviews.llvm.org/D88146
We intend to replace heuristics based code completion ranking with a Decision Forest Model.
This patch introduces a format for representing the model and an inference runtime that is code-generated at build time.
- Forest.json contains all the trees as an array of trees.
- Features.json describes the features to be used.
- Codegen file takes the above two files and generates CompletionModel containing Feature struct and corresponding Evaluate function.
The Evaluate function maps a feature to a real number describing the relevance of this candidate.
- The codegen is part of build system and these files are generated at build time.
- Proposes a way to test the generated runtime using a test model.
- Replicates the model structure in unittests.
- unittest tests both the test model (for correct tree traversal) and the real model (for sanity).
This reverts commit 549e55b3d5.
Kirill Bobyrev