This enables unifying command line flags with config options in clangd
internals. This patch changes behaviour in 2 places:
- BackgroundIndex was previously disabled when -remote-index was
provided. After this patch, it will be enabled but all files will have
bkgindex policy set to Skip.
- -index-file was loaded at startup (at least load was initiated), now
the load will happen through ProjectAwareIndex with first index query.
Unfortunately this doesn't simplify any options initially, as
- CompileCommandsDir is also used by clangd --check workflow, which
doesn't use configs.
- EnableBackgroundIndex option controls whether the component will be
created at all, which implies creation of extra threads registering a
listener for compilation database discoveries.
Differential Revision: https://reviews.llvm.org/D98029
Explicit specifier can only be mentioned on the in-line declaration of a
constructor, so don't carry it over to the definition.
Differential Revision: https://reviews.llvm.org/D98164
Also give CanonicalIncludes a less powerful interface (canonicalizes
symbols vs headers separately) so we can cache its results better.
Prior to this:
- path->uri conversions were not consistently cached, this is
particularly cheap when we start from a FileEntry* (which we often can)
- only a small fraction of header-to-include calculation was cached
This is a significant speedup at least for dynamic indexing of preambles.
On my machine, opening XRefs.cpp:
```
PreambleCallback 1.208 -> 1.019 (-15.7%)
BuildPreamble 5.538 -> 5.214 (-5.8%)
```
Differential Revision: https://reviews.llvm.org/D98371
Refactor cross file rename to use a Filesystem instead of a function for getting buffer contents of open files.
Depends on D94554
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D95043
This allows sending requests through CLI and more debugging
opportunities. Example:
```bash
$ grpc_cli ls localhost:50051
clang.clangd.remote.v1.SymbolIndex
grpc.reflection.v1alpha.ServerReflection
grpc.health.v1.Health
```
https://reviews.llvm.org/D94554 introduced code which wont compile with some build flags due to a field having the same identifier as a type.
clang-tools-extra/clangd/DraftStore.h:55:11: error: declaration of ‘clang::clangd::DraftStore::Draft clang::clangd::DraftStore::DraftAndTime::Draft’ changes meaning of ‘Draft’ [-fpermissive]
55 | Draft Draft;
| ^~~~~
clang-tools-extra/clangd/DraftStore.h:30:10: note: ‘Draft’ declared here as ‘struct clang::clangd::DraftStore::Draft’
30 | struct Draft {
| ^~~~~
Create a `ThreadsafeFS` in the `DraftStore` that overlays the dirty file contents over another `ThreadsafeFS`.
This provides a nice thread-safe interface for using dirty file contents throughout the codebase, for example cross file refactoring.
Creating a Filesystem view will overlay a snapshot of the current contents, so if the draft store is updated while the view is being used, it will contain stale contents.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D94554
This was causing TSan failures due to a race on vptr during destruction,
see
https://lab.llvm.org/buildbot/#/builders/131/builds/6579/steps/6/logs/FAIL__Clangd_Unit_Tests__LSPTest_FeatureModulesThr.
The story is, during the execution of a destructor all the virtual
dispatches should resolve to implementations in the class being
destroyed, not the derived ones. And LSPTests will log some stuff during
destruction (we send shutdown/exit requests, which are logged), which is
a virtual dispatch when LSPTest is derived from clang::clangd::Logger.
It is a benign race in our case, as gtests that derive from LSPTest
doesn't override the `log` method. But still during destruction, we
might try to update vtable ptr (due to being done with destruction of
test and starting destruction of LSPTest) and read from it to dispatch a
log message at the same time.
This patch fixes that race by moving `log` out of the vtable of
`LSPTest`.
Differential Revision: https://reviews.llvm.org/D98241
Without this patch the file list of the preamble index contains URIs, but other indexes file lists contain file paths.
This makes `indexedFiles()` always returns `IndexContents::None` for the preamble index, because current implementation expects file paths inside the file list of the index.
This patch fixes this problem and also helps to avoid a lot of URI to path conversions during indexes merge.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D97535
We have no way to reason about the bool returned by try_emplace, so we
simply ignore any std::move()s that happen in a try_emplace argument.
A lot of the time in this situation, the code will be checking the
bool and doing something else if it turns out the value wasn't moved
into the map, and this has been causing false positives so far.
I don't currently have any intentions of handling "maybe move" functions
more generally.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D98034
As pointed out in D96244, "Module" is already pretty overloaded to refer
to clang and llvm modules. (And clangd deals directly with the former).
FeatureModule is a bit of a mouthful but it's pretty self-descriptive.
I think it might be better than "Component" which doesn't really capture
the "common interface" aspect - it's IMO confusing to refer to
"components" but exclude CDB for example.
Differential Revision: https://reviews.llvm.org/D97950
GCC warning:
```
/llvm-project/clang-tools-extra/clangd/SemanticHighlighting.cpp: In function ‘bool clang::clangd::{anonymous}::canHighlightName(clang::DeclarationName)’:
/llvm-project/clang-tools-extra/clangd/SemanticHighlighting.cpp:64:1: warning: control reaches end of non-void function [-Wreturn-type]
64 | }
| ^
```
Often you are only interested in the overall cognitive complexity of a
function and not every individual increment. Thus the flag
'DescribeBasicIncrements' is added. If it is set to 'true', each increment
is flagged. Otherwise, only the complexity of function with complexity
of at least the threshold are flagged.
By default 'DescribeBasisIncrements' is set to 'true', which is the original behavior of the check.
Added a new test for different flag combinations.
(The option to ignore macros which was original part of this patch will be added in another path)
Reviewed By: lebedev.ri
Differential Revision: https://reviews.llvm.org/D96281
Enables transforming loops of the form:
```
for (int i = 0; I != container.size(); ++I) { container[I]...; }
for (int i = 0; I != N; ++I) { FixedArrSizeN[I]...; }
```
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97940
- Create a separate section on silencing erroneous warnings and add more material to it
- Add note that the check is flow-sensitive but not path-sensitive
Clangd can invalidate client state of features like semantic higlighting
without client explicitly triggering, for example after a preamble build
caused by an onSave notification on a different file.
This patch introduces a mechanism to let client know of such actions,
and also calls the workspace/semanticTokens/refresh request to
demonstrate the situation after each preamble build.
Fixes https://github.com/clangd/clangd/issues/699.
Differential Revision: https://reviews.llvm.org/D97548
- highlight references to protocols in class/protocol/extension decls
- support multi-token selector highlights in semantic + xref highlights
(method calls and declarations only)
- In `@interface I(C)`, I now references the interface and C the category
- highlight uses of interfaces as types
- added semantic highlightings of protocol names (as "interface") and
category names (as "namespace").
These are both standard kinds, maybe "extension" will be standardized...
- highlight `auto` as "class" when it resolves to an ObjC pointer
- don't highlight `self` as a variable even though the AST models it as one
Not fixed: uses of protocols in type names (needs some refactoring of
unrelated code first)
Differential Revision: https://reviews.llvm.org/D97617
... For removal in next release cycle.
The clang warning that does the same thing is enabled by default and typically emits better diagnostics making this check surplus to requirements.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97491
After rG5e1801813d93210acae84ff3c68a01512c2df9bc The help command still lists IgnoreImplicitCastsAndParentheses as a valid option.
Reviewed By: aaron.ballman, rsmith
Differential Revision: https://reviews.llvm.org/D97806
Clangd uses codecompletion limit as the limit for workspacesymbols, so
in theory this should only be an order of magnitude slower than a
codecompletion request with empty identifier (as code completion limits
the available symbols).
This is also what LSP suggests "Clients may send an empty string here to request all symbols.".
Clangd doesn't really fulfill the "all" part of that statement, but we
never do unless user set the index query limit to zero explicitly.
Differential Revision: https://reviews.llvm.org/D97773
Make use of the `equalsBoundNode` matcher to ensure Init, Conditon and Increment variables all refer to the same variable during matching.
Reviewed By: steveire
Differential Revision: https://reviews.llvm.org/D97639
This disables the check for false positive cases where implicit type conversion
through either an implicit single argument constructor or a member conversion
operator is triggered when constructing the loop variable.
Fix the test cases that meant to cover these cases.
Differential Revision: https://reviews.llvm.org/D97577
Reviewed-by: hokein
ClangdServer already gets notified of every change, so it makes sense for it to
be the source of truth.
This is a step towards having ClangdServer expose a FS that includes dirty
buffers: D94554
Related changes:
- version is now optional for ClangdServer, to preserve our existing fuzziness
in this area (missing version ==> autoincrement)
- ClangdServer::format{File,Range} are now more regular ClangdServer functions
that don't need the code passed in. While here, combine into one function.
- incremental content update logic is moved from DraftStore to
ClangdLSPServer, with most of the implementation in SourceCode.cpp.
DraftStore is now fairly trivial, and will probably ultimately be
*replaced* by the dirty FS stuff.
Differential Revision: https://reviews.llvm.org/D97738
Browsing macro-generated symbols is confusing.
On the one hand, it seems very *useful* to be able to see the summary of
symbols that were generated.
On the other hand, some macros spew a lot of confusing symbols into the
namespace and when used repeatedly (ABSL_FLAG) can create a lot of spam
that's hard to navigate.
Design constraints:
- the macro expansion tree need not align with the AST, though it often
does in practice.
We address this by defining the nesting based on the *primary*
location of decls, rather than their ranges.
- DocumentSymbol.children[*].range should nest within DocumentSymbol.range
(This constraint is not in LSP "breadcrumbs" breaks without it)
We adjust macro ranges so they cover their "children", rather than
just the macro expansion
- LSP does not have a "macro expansion" symbolkind, nor does it allow a
symbol to have no kind. I've arbitrarily picked "null" as this is
unlikely to conflict with anything useful.
This patch makes all macros and children visible for simplicity+consistency,
though in some cases it may be better to elide the macro node.
We may consider adding heuristics for this in future (e.g. when it expands
to one decl only?) but it doesn't seem clear-cut to me.
Differential Revision: https://reviews.llvm.org/D97615
Don't show negative numbers
Don't show numbers <10 (hex is the same as decimal)
Show numeric enum values in hex too
Differential Revision: https://reviews.llvm.org/D97226
CodeCompletionContext::Kind has 36 Kinds. The completion model used to
support categorical features of 32 cardinality.
Due to this clangd tests were failing asan tests due to overflow.
This patch makes the completion model support 64 cardinality of
categorical features by storing ENUM Features as uint64_t instead of
uint32_t.
Verified that this fixes the asan failures.
Latency: 6.7ms (old) VS 6.8ms (new) per 1000 predictions.
Differential Revision: https://reviews.llvm.org/D97770
CodeCompletionContext::Kind has 36 Kinds. The completion model currently
only handles categorical features of 32 cardinality.
Changing the datatype to uint64_t will solve the problem.
This reverts commit 438b5bb05a.
clang-tools-extra/clangd/benchmarks/CompletionModel/DecisionForestBenchmark.cpp fails to compile since `"CompletionModel.h"` is auto-generated from clang-tools-extra/clangd/quality/model/features.json, which was changed in https://reviews.llvm.org/D94697 to remove `setFilterLength` and `setIsForbidden`, rename `setFileProximityDistance` and `setSymbolScopeDistance`, and add `setNumNameInContext` and `setFractionNameInContext`. This patch removes calls to the two removed functions, updates calls to the two renamed functions, and adds calls to the two new functions. (`20` is an arbitrary choice for the `setNumNameInContext` argument.) It also changes the `FlipCoin` argument from float to double to silence lossy conversion warnings.
Note: I don't use this tool but encountered the build errors and took a shot at fixing them. Please holler if there's another recommended solution. Thanks!
Reviewed By: usaxena95
Differential Revision: https://reviews.llvm.org/D97620
This makes code completion use a Decision Forest based ranking algorithm to rank
completion candidates. [Esitmated 6% accuracy boost]. This was
previously hidden behind the flag --ranking-model=decision_forest. This
patch makes it the default ranking algorithm.
Note: this is a generic model, not specialized for any particular
project. clangd does not collect or upload data to train code completion.
Also treat Keywords separately as they are not recorded by the training set generator.
Differential Revision: https://reviews.llvm.org/D96353
Added an option to control whether to apply the fixes found in notes attached to clang tidy errors or not.
Diagnostics may contain multiple notes each offering different ways to fix the issue, for that reason the default behaviour should be to not look at fixes found in notes.
Instead offer up all the available fix-its in the output but don't try to apply the first one unless `-fix-notes` is supplied.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D84924
Adds an option, `PreferResetCall`, currently defaulted to `false`, to the check.
When `true` the check will refactor by calling the `reset` member function.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97630
If C++17 mode is enabled and the assert doesn't have a string literal, we can emit a static assert with no message in favour of one with an empty message.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97313
Tweak the diagnostics to create small replacements rather than grabbing source text from the lexer.
Also simplified the diagnostic message.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97632
LIBCLANG_INCLUDE_CLANG_TOOLS_EXTRA causes clang-tools-extra tools
to be included in libclang, which caused a dependency cycle. The option
has been off by default for two releases now, and (based on a web search
and mailing list feedback) nobody seems to turn it on. Remove it, like
planned on https://reviews.llvm.org/D79599
Differential Revision: https://reviews.llvm.org/D97693
The interface served a purpose, but since the ability to emit diagnostics when parsing configuration was added, its become mostly redundant. Emitting the diagnostic and removing the boilerplate is much cleaner.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97614
- Categories will now show up as `MyClass(Category)` instead of
`Category` and `MyCategory()` instead of `(anonymous)` in document
symbols
- Methods will now be shown as `-selector:` or `+selector:`
instead of `selector:` to differentiate between instance and class
methods in document symbols
Differential Revision: https://reviews.llvm.org/D96612
Kadir Cetinkaya