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
Summary: Implemention of textDocument/hover as described in LSP definition.
This patch adds a basic Hover implementation. When hovering a variable,
function, method or namespace, clangd will return a text containing the
declaration's scope, as well as the declaration of the hovered entity.
For example, for a variable:
Declared in class Foo::Bar
int hello = 2
For macros, the macro definition is returned.
This patch doesn't include:
- markdown support (the client I use doesn't support it yet)
- range support (optional in the Hover response)
- comments associated to variables/functions/classes
They are kept as future work to keep this patch simpler.
I added tests in XRefsTests.cpp. hover.test contains one simple
smoketest to make sure the feature works from a black box perspective.
Reviewers: malaperle, krasimir, bkramer, ilya-biryukov
Subscribers: sammccall, mgrang, klimek, rwols, ilya-biryukov, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D35894
Signed-off-by: Simon Marchi <simon.marchi@ericsson.com>
Signed-off-by: William Enright <william.enright@polymtl.ca>
llvm-svn: 325395
Summary:
o Collect suitable #include paths for index symbols. This also does smart mapping
for STL symbols and IWYU pragma (code borrowed from include-fixer).
o For global code completion, add a command for inserting new #include in each code
completion item.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, mgorny, ilya-biryukov, jkorous-apple, hintonda, cfe-commits
Differential Revision: https://reviews.llvm.org/D42640
llvm-svn: 325343
Summary:
As a consequence, all LSP operations are now handled asynchronously,
i.e. they never block the main processing thread. However, if
-run-synchronously flag is specified, clangd still runs everything on
the main thread.
Reviewers: sammccall, ioeric, hokein
Reviewed By: sammccall, ioeric
Subscribers: klimek, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D43227
llvm-svn: 325233
Summary:
The chrome trace viewer requires events within a thread to strictly nest.
So we need to record the lifetime of the Span objects, not the contexts.
But we still want to show the relationship between spans where a context crosses
threads, so do this with flow events (i.e. arrows).
Before: https://photos.app.goo.gl/q4Dd9u9xtelaXk1v1
After: https://photos.app.goo.gl/5RNLmAMLZR3unvY83
(This could stand some further improvement, in particular I think we want a
container span whenever we schedule work on a thread. But that's another patch)
Reviewers: ioeric
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D43272
llvm-svn: 325220
Prior to this patch, same instance of VFS was shared for concurrent
processing of the files in ClangdThreadingTest.StressTest.
It caused a data race as the same instance of InMemoryFileSystem was
mutated from multiple threads using setCurrentWorkingDirectory().
llvm-svn: 325132
Summary:
Some of the existing structs had primimtive fields that were
not explicitly initialized on construction.
After this commit every struct consistently sets a defined value for
every field when default-initialized.
Reviewers: hokein, ioeric, sammccall
Reviewed By: sammccall
Subscribers: klimek, cfe-commits, jkorous-apple
Differential Revision: https://reviews.llvm.org/D43230
llvm-svn: 325113
Relative paths could be returned in some cases, e.g. when relative
path is used in compilation arguments. This led to crash when trying
to convert the path to URI.
llvm-svn: 325029
Summary:
LSP has asynchronous semantics, being able to block on an async operation
completing is unneccesary and leads to tighter coupling with the threading.
In practice only tests depend on this, so we add a general-purpose "block until
idle" function to the scheduler which will work for all operations.
To get this working, fix a latent condition-variable bug in ASTWorker, and make
AsyncTaskRunner const-correct.
Reviewers: ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D43127
llvm-svn: 324990
Summary:
Before the fix, if clang-move decides to move the following macro statement, it only moves the first line `DEFINE(A,`.
```
DEFINE(A,
B);
```
Reviewers: ioeric
Reviewed By: ioeric
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D43174
llvm-svn: 324886
Summary:
It was deprecated and callback version and is used everywhere.
Only changes to the testing code were needed.
Reviewers: hokein, ioeric, sammccall
Reviewed By: sammccall
Subscribers: mgorny, klimek, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D43068
llvm-svn: 324883
Within a TU:
- as now, collect a declaration from the first occurrence of a symbol
(taking clang's canonical declaration)
- when we first see a definition occurrence, copy the symbol and add it
Across TUs/sources:
- mergeSymbol in Merge.h is responsible for combining matching Symbols.
This covers dynamic/static merges and cross-TU merges in the static index.
- it prefers declarations from Symbols that have a definition.
- GlobalSymbolBuilderMain is modified to use mergeSymbol as a reduce step.
Random cleanups (can be pulled out):
- SymbolFromYAML -> SymbolsFromYAML, new singular SymbolFromYAML added
- avoid uninit'd SymbolLocations. Add an idiomatic way to check "absent".
- CanonicalDeclaration (as well as Definition) are mapped as optional in YAML.
- added operator<< for Symbol & SymbolLocation, for debugging
Reviewers: ioeric, hokein
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D42942
llvm-svn: 324735
Summary:
This can happen if the CompileCommand provided by compilation database
is malformed.
Reviewers: hokein, ioeric, sammccall
Reviewed By: sammccall
Subscribers: klimek, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D43122
llvm-svn: 324732
Initially submitted as r324356 and reverted in r324386.
This change additionally contains a fix to crashes of the buildbots.
The source of the crash was undefined behaviour caused by
std::future<> whose std::promise<> was destroyed without calling
set_value().
llvm-svn: 324575
Summary:
In the new threading model clangd creates one thread per file to manage
the AST and one thread to process each of the incoming requests.
The number of actively running threads is bounded by the semaphore to
avoid overloading the system.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, mgorny, jkorous-apple, ioeric, hintonda, cfe-commits
Differential Revision: https://reviews.llvm.org/D42573
llvm-svn: 324356
Summary:
Some STL symbols are defined in inline namespaces. For example,
```
namespace std {
inline namespace __cxx11 {
typedef ... string;
}
}
```
Currently, this will be `std::__cxx11::string`; however, `std::string` is desired.
Inline namespaces are treated as transparent scopes. This
reflects the way they're most commonly used for lookup. Ideally we'd
include them, but at query time it's hard to find all the inline
namespaces to query: the preamble doesn't have a dedicated list.
Reviewers: sammccall, hokein
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D42796
llvm-svn: 324065
Summary:
Instead of passing Context explicitly around, we now have a thread-local
Context object `Context::current()` which is an implicit argument to
every function.
Most manipulation of this should use the WithContextValue helper, which
augments the current Context to add a single KV pair, and restores the
old context on destruction.
Advantages are:
- less boilerplate in functions that just propagate contexts
- reading most code doesn't require understanding context at all, and
using context as values in fewer places still
- fewer options to pass the "wrong" context when it changes within a
scope (e.g. when using Span)
- contexts pass through interfaces we can't modify, such as VFS
- propagating contexts across threads was slightly tricky (e.g.
copy vs move, no move-init in lambdas), and is now encapsulated in
the threadpool
Disadvantages are all the usual TLS stuff - hidden magic, and
potential for higher memory usage on threads that don't use the
context. (In practice, it's just one pointer)
Reviewers: ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D42517
llvm-svn: 323872
Summary:
For symbols defined inside macros:
* use expansion location, if the symbol is formed via macro concatenation.
* use spelling location, otherwise.
This will fix some symbols that have ill-format location (especial invalid filepath).
Reviewers: ioeric
Reviewed By: ioeric
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D42575
llvm-svn: 323867
Summary:
Previously, we assume only old.cc includes "old.h", which would
introduce incorrect fixes for the cases where old.h also includes `#include "old.h"`
Although it should not be occurred in real projects, clang-move should handle this.
Old.h:
```
class Foo {};
```
after moving to a new old.h:
```
class Foo {};
```
Reviewers: ioeric
Reviewed By: ioeric
Subscribers: cfe-commits, klimek
Differential Revision: https://reviews.llvm.org/D42639
llvm-svn: 323865
Summary:
We now provide an abstraction of Scheduler that abstracts threading
and resource management in ClangdServer.
No changes to behavior are intended with an exception of changed error
messages.
This patch is preliminary work to allow a revamped threading
implementation that will move the threading code out of CppFile.
Reviewers: sammccall, bkramer, jkorous-apple
Reviewed By: sammccall
Subscribers: hokein, mgorny, hintonda, ioeric, jkorous-apple, cfe-commits, klimek
Differential Revision: https://reviews.llvm.org/D42174
llvm-svn: 323851
Summary:
o Replace the existing clangd::URI with a wrapper of FileURI which also
carries a resolved file path.
o s/FileURI/URI/
o Get rid of the URI hack in vscode extension.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D42419
llvm-svn: 323660
Summary:
This is probably the right behavior for distributed tracers, and makes unpaired
begin-end events impossible without requiring Spans to be bound to a thread.
The API is conceptually clean but syntactically awkward. As discussed offline,
this is basically a naming problem and will go away if (when) we use TLS to
store the current context.
The apparently-unrelated change to onScopeExit are because its move semantics
broken if Func is POD-like since r322838. This is true of function pointers,
and the lambda I use here that captures two pointers only.
I've raised this issue on llvm-dev and will revert this part if we fix it in
some other way.
Reviewers: ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D42499
llvm-svn: 323511
Summary:
This adds checks that our diagnostics emit correct ranges in a bunch of cases,
as promised in D41118.
The diagnostics-preamble test is also converted and extended to be a little more
precise.
diagnostics.test stays around as the smoke test for this feature.
Reviewers: ilya-biryukov
Subscribers: klimek, mgorny, cfe-commits
Differential Revision: https://reviews.llvm.org/D41454
llvm-svn: 323448
Summary:
* truncate symbols from static/dynamic index to the limited number
(which would save lots of cost in constructing the merged symbols).
* add an CLI option allowing to limit the number of returned completion results.
(default to 100)
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D42484
llvm-svn: 323408
Summary:
* For qualified completion (foo::a^)
* unresolved qualifier - use global namespace ("::")
* resolved qualifier - use all accessible namespaces inside the resolved qualifier.
* For unqualified completion (vec^), use scopes that are accessible from the
scope from which code completion occurs.
Reviewers: sammccall, ilya-biryukov
Reviewed By: sammccall
Subscribers: jkorous-apple, ioeric, klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D42073
llvm-svn: 323189
Summary: I will replace the existing URI struct in Protocol.h with the new URI and rename FileURI to URI in a followup patch.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: jkorous-apple, klimek, mgorny, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41946
llvm-svn: 323101
Global scope is "" (was "")
Top-level namespace scope is "ns::" (was "ns")
Nested namespace scope is "ns::ns::" (was "ns::ns")
This composes more naturally:
- qname = scope + name
- full scope = resolved scope + unresolved scope (D42073 was the trigger)
It removes a wart from the old way: "foo::" has one more separator than "".
Another alternative that has these properties is "::ns", but that lacks
the property that both the scope and the name are substrings of the
qname as produced by clang.
This is re-landing r322996 which didn't build.
llvm-svn: 323000
Global scope is "" (was "")
Top-level namespace scope is "ns::" (was "ns")
Nested namespace scope is "ns::ns::" (was "ns::ns")
This composes more naturally:
- qname = scope + name
- full scope = resolved scope + unresolved scope (D42073 was the trigger)
It removes a wart from the old way: "foo::" has one more separator than "".
Another alternative that has these properties is "::ns", but that lacks
the property that both the scope and the name are substrings of the
qname as produced by clang.
llvm-svn: 322996
Summary:
- we match on USR, and do a field-by-field merge if both have results
- scoring is post-merge, with both sets of information available
(for now, sema priority is used if available, static score for index results)
- limit is applied to the complete result set (previously index ignored limit)
- CompletionItem is only produces for the returned results
- If the user doesn't type a scope, we send the global scope for completion
(we can improve this after D42073)
Reviewers: ioeric
Subscribers: klimek, ilya-biryukov, mgrang, cfe-commits
Differential Revision: https://reviews.llvm.org/D42181
llvm-svn: 322945
Summary:
This makes performance slower but more predictable (it always processes
every symbol). We need to find ways to make this fast, possibly by precomputing
short queries or capping the number of scored results. But our current approach
is too naive.
It also no longer returns results in a "good" order. In fact it's pathological:
the top N results are ranked from worst to best. Indexes aren't responsible for
ranking and MergedIndex can't do a good job, so I'm pleased that this will make
any hidden assumptions we have more noticeable :-)
Reviewers: hokein
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D42060
llvm-svn: 322821
Summary:
This test dominates our unit test runtime, and the change speeds it up by 10x.
We lose coverage of some combinations of flags, but I'm not sure that's finding
many bugs.
3300 -> 300ms on my machine (3800 -> 800ms for the whole of CompletionTest).
Reviewers: ilya-biryukov
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D42063
llvm-svn: 322547
Summary:
We now hide the static/dynamic split from the code completion, behind a
new implementation of the SymbolIndex interface. This will reduce the
complexity of the sema/index merging that needs to be done by
CodeComplete, at a fairly small cost in flexibility.
Reviewers: hokein
Subscribers: klimek, mgorny, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D42049
llvm-svn: 322480
Summary:
To stay fast, it avoids deserializing anything outside the current file, by
disabling the LoadExternal code completion option added in r322377, when the
index is enabled.
Reviewers: hokein
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41996
llvm-svn: 322387
Summary:
For some cases, GoToDefinition will navigate to the forward class
declaration, we should always navigate to the class definition.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41661
llvm-svn: 322370
Summary:
o We only collect symbols in namespace or translation unit scopes.
o Add an option to only collect symbols in included headers.
Reviewers: hokein, ilya-biryukov
Reviewed By: hokein, ilya-biryukov
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D41823
llvm-svn: 322193
Summary:
Use the YAML-format symbols (generated by the global-symbol-builder tool) to
do the global code completion.
It is **experimental** only , but it allows us to experience global code
completion on a relatively small project.
Tested with LLVM project.
Reviewers: sammccall, ioeric
Reviewed By: sammccall, ioeric
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41668
llvm-svn: 322191
Summary:
This patch removes hidden items from code completion.
Items can be hidden, e.g., by other items in the child scopes.
This patch addresses a particular problem of a duplicate completion
item for the class in the following example:
struct Adapter { void method(); };
void Adapter::method() {
Adapter^
}
We should probably investigate if there are other duplicates in
completion and remove them, possibly adding assertions that it never
happens.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: cfe-commits, klimek
Differential Revision: https://reviews.llvm.org/D41901
llvm-svn: 322185
Summary:
This enables more execution modes like standalone and Mapreduce-style execution.
See also D41729
Reviewers: hokein, sammccall
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41730
llvm-svn: 322084
Summary:
We currently only collect external-linkage symbols in the collector,
which results in missing some typical symbols (like no-linkage type alias symbols).
This patch relaxes the constraint a bit to allow collecting more symbols.
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D41759
llvm-svn: 322067
Summary:
This improves a few things:
- the insert -> freeze -> read sequence is now enforced/communicated by the
type system
- SymbolSlab::const_iterator iterates over symbols, not over id-symbol pairs
- we avoid permanently storing a second copy of the IDs, and the
string map's hashtable
The slab size is now down to 21.8MB for the LLVM project.
Of this only 2.7MB is strings, the rest is #symbols * `sizeof(Symbol)`.
`sizeof(Symbol)` is currently 96, which seems too big - I think
SymbolInfo isn't efficiently packed. That's a topic for another patch!
Also added simple API to see the memory usage/#symbols of a slab, since
it seems likely we will continue to care about this.
Reviewers: ilya-biryukov
Subscribers: klimek, mgrang, cfe-commits
Differential Revision: https://reviews.llvm.org/D41506
llvm-svn: 321412
Summary:
Symbols are not self-contained - it's only safe to hand them out if you
guarantee the lifetime of the underlying data.
Before this lands, I'm going to measure the before/after memory usage of the
LLVM index loaded into memory in a single slab.
Reviewers: hokein
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41483
llvm-svn: 321272
Summary:
The goal here is again to make it easier to read and write the tests.
I've extracted `parseTextMarker` from CodeCompleteTests into an `Annotations`
class, adding features to it:
- as well as points `^s` it allows ranges `[[...]]`
- multiple points and ranges are supported
- points and ranges may be named: `$name^` and `$name[[...]]`
These features are used for the xrefs tests. This also paves the way for
replacing the lit diagnostics.test with more readable unit tests, using named
ranges.
Alternative considered: `TestSelectionRange` in clang-refactor/TestSupport
Main problems were:
- delimiting the end of ranges is awkward, requiring counting
- comment syntax is long and at least as cryptic for most cases
- no separate syntax for point vs range, which keeps xrefs tests concise
- Still need to convert to Position everywhere
- Still need helpers for common case of expecting exactly one point/range
(I'll probably promote the extra `PrintTo`s from some of the core Protocol types
into `operator<<` in `Protocol.h` itself in a separate, prior patch...)
Reviewers: ioeric
Subscribers: klimek, mgorny, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41432
llvm-svn: 321184
Summary:
- Moved these functions to SourceCode.h
- added unit tests
- fix off by one in positionToOffset: Offset - 1 in final calculation was wrong
- fixed formatOnType which had an equal and opposite off-by-one
- positionToOffset and offsetToPosition both consistently clamp to beginning/end
of file when input is out of range
- gave variables more descriptive names
- removed windows line ending fixmes where there is nothing to fix
- elaborated on UTF-8 fixmes
This will conflict with Eric's D41281, but in a pretty easy-to-resolve way.
Reviewers: ioeric
Subscribers: klimek, mgorny, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41351
llvm-svn: 321073
Summary: When scopes are specified, only match symbols from scopes.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41367
llvm-svn: 321067
Summary: This will be used together with D40548 for the global index source (experimental).
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, mgorny, ilya-biryukov, cfe-commits, ioeric
Differential Revision: https://reviews.llvm.org/D41178
llvm-svn: 320694
Summary:
o Index interfaces to support using different index sources (e.g. AST index, global index) for code completion, cross-reference finding etc. This patch focuses on code completion.
The following changes in the original patch has been split out.
o Implement an AST-based index.
o Add an option to replace sema code completion for qualified-id with index-based completion.
o Implement an initial naive code completion index which matches symbols that have the query string as substring.
Reviewers: malaperle, sammccall
Reviewed By: sammccall
Subscribers: hokein, klimek, malaperle, mgorny, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D40548
llvm-svn: 320688
Summary:
* The "Symbol" class represents a C++ symbol in the codebase, containing all the
information of a C++ symbol needed by clangd. clangd will use it in clangd's
AST/dynamic index and global/static index (code completion and code
navigation).
* The SymbolCollector (another IndexAction) will be used to recollect the
symbols when the source file is changed (for ASTIndex), or to generate
all C++ symbols for the whole project.
In the long term (when index-while-building is ready), clangd should share a
same "Symbol" structure and IndexAction with index-while-building, but
for now we want to have some stuff working in clangd.
Reviewers: ioeric, sammccall, ilya-biryukov, malaperle
Reviewed By: sammccall
Subscribers: malaperle, klimek, mgorny, cfe-commits
Differential Revision: https://reviews.llvm.org/D40897
llvm-svn: 320486
Summary:
It will be used to pass around things like Logger and Tracer throughout
clangd classes.
Reviewers: sammccall, ioeric, hokein, bkramer
Reviewed By: sammccall
Subscribers: klimek, bkramer, mgorny, cfe-commits
Differential Revision: https://reviews.llvm.org/D40485
llvm-svn: 320468
We currently use target_link_libraries without an explicit scope
specifier (INTERFACE, PRIVATE or PUBLIC) when linking executables.
Dependencies added in this way apply to both the target and its
dependencies, i.e. they become part of the executable's link interface
and are transitive.
Transitive dependencies generally don't make sense for executables,
since you wouldn't normally be linking against an executable. This also
causes issues for generating install export files when using
LLVM_DISTRIBUTION_COMPONENTS. For example, clang has a lot of LLVM
library dependencies, which are currently added as interface
dependencies. If clang is in the distribution components but the LLVM
libraries it depends on aren't (which is a perfectly legitimate use case
if the LLVM libraries are being built static and there are therefore no
run-time dependencies on them), CMake will complain about the LLVM
libraries not being in export set when attempting to generate the
install export file for clang. This is reasonable behavior on CMake's
part, and the right thing is for LLVM's build system to explicitly use
PRIVATE dependencies for executables.
Unfortunately, CMake doesn't allow you to mix and match the keyword and
non-keyword target_link_libraries signatures for a single target; i.e.,
if a single call to target_link_libraries for a particular target uses
one of the INTERFACE, PRIVATE, or PUBLIC keywords, all other calls must
also be updated to use those keywords. This means we must do this change
in a single shot. I also fully expect to have missed some instances; I
tested by enabling all the projects in the monorepo (except dragonegg),
and configuring both with and without shared libraries, on both Darwin
and Linux, but I'm planning to rely on the buildbots for other
configurations (since it should be pretty easy to fix those).
Even after this change, we still have a lot of target_link_libraries
calls that don't specify a scope keyword, mostly for shared libraries.
I'm thinking about addressing those in a follow-up, but that's a
separate change IMO.
Differential Revision: https://reviews.llvm.org/D40823
llvm-svn: 319840
Summary:
Previously, completion options were set per ClangdServer instance.
It will allow to change completion preferences during the lifetime
of a single ClangdServer instance.
Also rewrote ClangdCompletionTest.CompletionOptions to reuse single
ClangdServer instance, the test now runs 2x faster on my machine.
Reviewers: sammccall, ioeric, hokein
Reviewed By: sammccall, ioeric
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D40654
llvm-svn: 319753
Summary:
Common parts are mostly FS related, so pulled out TestFS.h for the common stuff.
Deliberately resisted cleaning up much here, so this is pretty mechanical.
Reviewers: hokein
Subscribers: klimek, mgorny, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D40784
llvm-svn: 319741
Summary:
- GlobalCompilationDatabase now returns a single command (that's all we use)
- fallback flags are now part of the GlobalCompilationDatabase.
There's a default implementation that they can optionally customize.
- this allows us to avoid invoking the fallback logic on two separate codepaths
- race on extra flags fixed by locking the mutex
- made GCD const-correct (DBGCD does have mutating methods)
Reviewers: hokein
Subscribers: klimek, cfe-commits, ilya-biryukov
Differential Revision: https://reviews.llvm.org/D40733
llvm-svn: 319647
Summary:
This will be used for rescoring code completion results based on partial
identifiers.
Short-term use:
- we want to limit the number of code completion results returned to
improve performance of global completion. The scorer will be used to
rerank the results to return when the user has applied a filter.
Long-term use case:
- ranking of completion results from in-memory index
- merging of completion results from multiple sources (merging usually
works best when done at the component-score level, rescoring the
fuzzy-match quality avoids different backends needing to have
comparable scores)
Reviewers: ilya-biryukov
Subscribers: cfe-commits, mgorny
Differential Revision: https://reviews.llvm.org/D40060
llvm-svn: 319557
Summary:
This allows us to limit the number of results we return and still allow them
to be surfaced by refining a query (D39852).
The initial algorithm is very conservative - it accepts a completion if the
filter is any case-insensitive sub-sequence. It does not attempt to rank items
based on match quality.
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D39882
llvm-svn: 319552
Summary:
- JSON<->Obj interface is now ADL functions, so they play nicely with enums
- recursive vector/map parsing and ObjectMapper moved to JSONExpr and tested
- renamed (un)parse to (de)serialize, since text -> JSON is called parse
- Protocol.cpp gets a bit shorter
Sorry for the giant patch, it's prety mechanical though
Reviewers: ilya-biryukov
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D40596
llvm-svn: 319478
Summary:
- Converted Protocol.h parse() functions to take JSON::Expr.
These no longer detect and log unknown fields, as this is not that
useful and no longer free.
I haven't changed the error handling too much: fields that were
treated as optional before are still optional, even when it's wrong.
Exception: object properties with the wrong type are now ignored.
- Made JSONRPCDispatcher parse using json::parse
- The bug where 'method' must come before 'params' in the stream is
fixed as a side-effect. (And the same bug in executeCommand).
- Some parser crashers fixed as a side effect.
e.g. https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=3890
- The debug stream now prettyprints the input messages with --pretty.
- Request params are attached to traces when tracing is enabled.
- Fixed some bugs in tests (errors tolerated by YAMLParser, and
off-by-ones in Content-Length that our null-termination was masking)
- Fixed a random double-escape bug in ClangdLSPServer (it was our last
use of YAMLParser!)
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D40406
llvm-svn: 319159
Summary:
Noticed this when I tried to port the Protocol.h parsers.
And tests for the inspect API, which caught a small bug.
Reviewers: ioeric
Subscribers: ilya-biryukov
Differential Revision: https://reviews.llvm.org/D40399
llvm-svn: 319157
Summary:
[clangd] Tracing improvements
Compose JSON using JSONExpr
Allow attaching metadata to spans (and avoid it if tracing is off)
Attach IDs and responses of JSON RPCs to their spans
The downside is that large responses make the trace viewer sluggish.
We should make our responses less huge :-) Or fix trace viewer.
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D40132
llvm-svn: 318928
Summary: (There must be some reason why D38077 didn't just do this, but I don't get it!)
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D39836
llvm-svn: 318925
Summary:
This will replace the places where we're using YAMLParser to parse JSON now:
- the new marshalling code (T::parse()) should handle fewer cases and require
fewer explicit casts
- we'll early-reject invalid JSON that YAMLParser accepts
- we'll be able to fix protocol-parsing bugs caused by the fact that YAML can
only parse forward
I plan to do the conversion as soon as this lands, but I don't want it in one
patch as the protocol.cpp changes are conflict-prone.
Reviewers: ioeric
Subscribers: ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D40182
llvm-svn: 318774
Summary:
All results are scored, we only process CodeCompletionStrings for the winners.
We now return CompletionList rather than CompletionItem[] (both are valid).
sortText is now based on CodeCompletionResult::orderedName (mostly the same).
This is the first clangd-only completion option, so plumbing changed.
It requires a small clangd patch (exposing CodeCompletionResult::orderedName).
(This can't usefully be enabled yet: we don't support server-side filtering)
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D39852
llvm-svn: 318287
Summary:
This form can be created with a nice clang-format-friendly literal syntax,
and gets escaping right. It knows how to call unparse() on our Protocol types.
All the places where we pass around JSON internally now use this type.
Object properties are sorted (stored as std::map) and so serialization is
canonicalized, with optional prettyprinting (triggered by a -pretty flag).
This makes the lit tests much nicer to read and somewhat nicer to debug.
(Unfortunately the completion tests use CHECK-DAG, which only has
line-granularity, so pretty-printing is disabled there. In future we
could make completion ordering deterministic, or switch to unittests).
Compared to the current approach, it has some efficiencies like avoiding copies
of string literals used as object keys, but is probably slower overall.
I think the code/test quality benefits are worth it.
This patch doesn't attempt to do anything about JSON *parsing*.
It takes direction from the proposal in this doc[1], but is limited in scope
and visibility, for now.
I am of half a mind just to use Expr as the target of a parser, and maybe do a
little string deduplication, but not bother with clever memory allocation.
That would be simple, and fast enough for clangd...
[1] https://docs.google.com/document/d/1OEF9IauWwNuSigZzvvbjc1cVS1uGHRyGTXaoy3DjqM4/edit
+cc d0k so he can tell me not to use std::map.
Reviewers: ioeric, malaperle
Subscribers: bkramer, ilya-biryukov, mgorny, klimek
Differential Revision: https://reviews.llvm.org/D39435
llvm-svn: 317486
Summary:
This lets you visualize clangd's activity on different threads over time,
and understand critical paths of requests and object lifetimes.
The data produced can be visualized in Chrome (at chrome://tracing), or
in a standalone copy of catapult (http://github.com/catapult-project/catapult)
This patch consists of:
- a command line flag "-trace" that causes clangd to emit JSON trace data
- an API (in Trace.h) allowing clangd code to easily add events to the stream
- several initial uses of this API to capture JSON-RPC requests, builds, logs
Example result: https://photos.app.goo.gl/12L9swaz5REGQ1rm1
Caveats:
- JSON serialization is ad-hoc (isn't it everywhere?) so the API is
limited to naming events rather than attaching arbitrary metadata.
I'd like to fix this (I think we could use a JSON-object abstraction).
- The recording is very naive: events are written immediately by
locking a mutex. Contention on the mutex might disturb performance.
- For now it just traces instants or spans on the current thread.
There are other things that make sense to show (cross-thread flows,
non-thread resources such as ASTs). But we have to start somewhere.
Reviewers: ioeric, ilya-biryukov
Subscribers: cfe-commits, mgorny
Differential Revision: https://reviews.llvm.org/D39086
llvm-svn: 317193
Summary:
This is part 3 of 3 of a series of changes to improve Objective-C
linting in clang-tidy.
This adds a new clang-tidy check `objc-forbidden-subclassing` which
ensures clients do not create subclasses of Objective-C classes which
are not designed to be subclassed.
(Note that for code under your control, you should use
__attribute__((objc_subclassing_restricted)) instead -- this
is intended for third-party APIs which cannot be modified.)
By default, the following classes (which are publicly documented
as not supporting subclassing) are forbidden from subclassing:
ABNewPersonViewController
ABPeoplePickerNavigationController
ABPersonViewController
ABUnknownPersonViewController
NSHashTable
NSMapTable
NSPointerArray
NSPointerFunctions
NSTimer
UIActionSheet
UIAlertView
UIImagePickerController
UITextInputMode
UIWebView
Clients can set a CheckOption
`objc-forbidden-subclassing.ClassNames` to a semicolon-separated
list of class names, which overrides this list.
Test Plan: `ninja check-clang-tools`
Patch by Ben Hamilton!
Reviewers: hokein, alexfh
Reviewed By: hokein
Subscribers: saidinwot, Wizard, srhines, mgorny, xazax.hun
Differential Revision: https://reviews.llvm.org/D39142
llvm-svn: 316744
Summary:
This is part 2 of 3 of a series of changes to improve
Objective-C linting in clang-tidy.
Currently, `clang::tidy::test::runCheckOnCode()` assumes all files
are C++ and unconditionally adds `-std=c++11` to the list of
`clang-tidy` options.
This updates the logic to check the extension of the source file
and only add `-std=c++11` if the extension indicates C++ or
Objective-C++.
Depends On D39188
Test Plan:
ninja ClangTidyTests && \
./tools/clang/tools/extra/unittests/clang-tidy/ClangTidyTests
Patch by Ben Hamilton!
Reviewers: hokein, alexfh
Reviewed By: hokein
Subscribers: Wizard
Differential Revision: https://reviews.llvm.org/D39189
llvm-svn: 316645
Summary:
This is part 1 of 3 of a series of changes to improve Objective-C
linting in clang-tidy.
This introduces a new clang-tidy module, `objc`, specifically for
Objective-C / Objective-C++ checks.
The module is currently empty; D39142 adds the first check.
Test Plan: `ninja check-clang-tools`
Patch by Ben Hamilton!
Reviewers: hokein, alexfh
Reviewed By: hokein
Subscribers: Wizard, mgorny
Differential Revision: https://reviews.llvm.org/D39188
llvm-svn: 316643
Summary:
ClangdServer now provides async code completion API.
It is still used synchronously by ClangdLSPServer, more work is needed
to allow processing other requests in parallel while completion (or
any other request) is running.
Reviewers: klimek, bkramer, krasimir
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D38583
llvm-svn: 314989
Summary:
Small extension to LSP to allow clients to use clangd to switch between C header files and source files.
Final version will use the completed clangd indexer to use the index of symbols to be able to switch from header to source file when the file names don't match.
Reviewers: malaperle, krasimir, bkramer, ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: ilya-biryukov, cfe-commits, arphaman
Patch by: William Enright
Differential Revision: https://reviews.llvm.org/D36150
llvm-svn: 314377
Summary:
Calls to onDiagnosticsReady were done concurrently before. This sometimes
led to older versions of diagnostics being reported to the user after
the newer versions.
Reviewers: klimek, bkramer, krasimir
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D38032
llvm-svn: 313754
Support running the extra clang tool tests when the static analyzer
is disabled. Disable the relevant clang-tidy tests and one include-fixer
test that require it to work.
Previously, the tests were disabled entirely with
CLANG_ENABLE_STATIC_ANALYZER being false. Now, the tests are being
enabled and the relevant tests are excluded and marked unsupported
appropriately.
In order to disable clang-tidy tests, the whole test directory is added
to the exclude lists, to avoid having to explicitly add 'REQUIRES' line
to every single test. If the other solution is preferable, I can update
the patch.
The yamldb_plugin include-fixer test is also updated to be disabled
without static analyzer. It fails in that case because clang is not
outputting a replacement suggestion -- but I don't know the exact
reason why it does not do that.
Differential Revision: https://reviews.llvm.org/D37188
llvm-svn: 311983
This is a follow up to https://reviews.llvm.org/D30567 where I overlooked that
LLVM YAML parser doesn't support multiline literal folding.
llvm-svn: 310491
Summary:
To get properly integration Clang-Tidy with CLion IDE, next things were implemented:
* Preserve `Message`, `FileOffset`, `FilePath` in the clang-tidy output.
* Export all diagnostics, not just the ones with fixes
* Test-cases
Reviewers: alexfh, ilya-biryukov
Subscribers: mgorny, JDevlieghere, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Patch by Vladimir Plyashkun!
Differential Revision: https://reviews.llvm.org/D35349
llvm-svn: 308015
Summary:
A common source of security bugs is code that opens a file descriptors without using the O_CLOEXEC flag. (Without that flag, an opened sensitive file would remain open across a fork+exec to a lower-privileged SELinux domain, leaking that sensitive data.).
Add a new Android module and one checks in clang-tidy.
-- open(), openat(), and open64() should include O_CLOEXEC in their flags argument. [android-file-open-flag]
Links to part2 and part3:
https://reviews.llvm.org/D33745https://reviews.llvm.org/D33747
Reviewers: chh, alexfh, aaron.ballman, hokein
Reviewed By: alexfh, hokein
Subscribers: jbcoe, joerg, malcolm.parsons, Eugene.Zelenko, srhines, mgorny, xazax.hun, cfe-commits, krytarowski
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D33304
llvm-svn: 306165
Summary:
ClangdServer owned objects passed to it in constructor for no good reason.
Lots of stuff was moved from the heap to the stack thanks to this change.
Reviewers: krasimir
Reviewed By: krasimir
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D34148
llvm-svn: 305298
Summary:
This is a reapplied r305280 with a fix to the crash found by build bots
(StringRef to an out-of-scope local std::string).
Reviewers: krasimir
Reviewed By: krasimir
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D34146
llvm-svn: 305291
Summary:
The use-case is when renaming a widely used name, like a lower-level
class in a codebase and clang-rename is simply invoked for each
translation unit based on the compile database. In this case it's not
interesting to show errors: not finding the symbol means there is
simply nothing to do.
Reviewers: klimek
Reviewed By: klimek
Differential Revision: https://reviews.llvm.org/D32403
llvm-svn: 304534
Summary:
This allows an implementation of FileSystemProvider that can track which vfs::FileSystem
were used for each of the requests.
Reviewers: bkramer, krasimir
Reviewed By: bkramer
Subscribers: klimek, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D33678
llvm-svn: 304214
Summary:
Custom vfs::FileSystem is currently used for unit tests.
This revision depends on https://reviews.llvm.org/D33397.
Reviewers: bkramer, krasimir
Reviewed By: bkramer, krasimir
Subscribers: klimek, cfe-commits, mgorny
Differential Revision: https://reviews.llvm.org/D33416
llvm-svn: 303977
Summary:
The patch adds a new feature for renaming qualified symbol references.
Unlike orginal clang-rename behavior, when renaming a qualified symbol to a new
qualified symbol (e.g "A::Foo" => "B::Bar"), this new rename behavior will
consider the prefix qualifiers of the symbol, and calculate the new prefix
qualifiers. It aims to add as few additional qualifiers as possible.
As this is an early version (only supports renaming classes), I don't change
current clang-rename interfaces at the moment, and would like to keep its
(command-line tool) behavior. So I added new interfaces for the prototype.
In the long run, these interfaces should be unified.
No functionality changes in original clang-rename command-line tool.
This patch also contains a few bug fixes of clang-rename which are discovered by
the new unittest:
* fix a potential nullptr accessment when class declaration doesn't have definition.
* add USRs of nested declartaions in "getNamedDeclFor".
Reviewers: ioeric
Reviewed By: ioeric
Subscribers: alexfh, cfe-commits, mgorny
Differential Revision: https://reviews.llvm.org/D31176
llvm-svn: 299419
Summary:
This furtherly improves r295303: [clang-tidy] Ignore spaces between globs in the Checks option.
Trims all whitespaces and not only spaces and correctly computes the offset of the checks list (taking the size before trimming).
Reviewers: alexfh
Reviewed By: alexfh
Subscribers: cfe-commits, JDevlieghere
Differential Revision: https://reviews.llvm.org/D30567
llvm-svn: 298621
Summary:
When changing namespaces, the tool adds leading "::" to references that need to
be fully-qualified, which would affect readability.
We avoid adding "::" when the symbol name does not conflict with the new
namespace name. For example, a symbol name "na::nb::X" conflicts with "ns::na"
since it would be resolved to "ns::na::nb::X" in the new namespace.
Reviewers: hokein
Reviewed By: hokein
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D30493
llvm-svn: 298363
Summary:
Add fuzzy SymbolIndex, where identifier needn't match exactly.
The purpose for this is global autocomplete in clangd. The query will be a
partial identifier up to the cursor, and the results will be suggestions.
It's in include-fixer because:
- it handles SymbolInfos, actually SymbolIndex is exactly the right interface
- it's a good harness for lit testing the fuzzy YAML index
- (Laziness: we can't unit test clangd until reorganizing with a tool/ dir)
Other questionable choices:
- FuzzySymbolIndex, which just refines the contract of SymbolIndex. This is
an interface to allow extension to large monorepos (*cough*)
- an always-true safety check that Identifier == Name is removed from
SymbolIndexManager, as it's not true for fuzzy matching
- exposing -db=fuzzyYaml from include-fixer is not a very useful feature, and
a non-orthogonal ui (fuzziness vs data source). -db=fixed is similar though.
Reviewers: bkramer
Subscribers: cfe-commits, mgorny
Differential Revision: https://reviews.llvm.org/D30720
llvm-svn: 297630
Summary:
Remove line number from Symbol identity.
For our purposes (include-fixer and clangd autocomplete), function overloads
within the same header should mostly be treated as a single combined symbol.
We may want to track individual occurrences (line number, full type info)
and aggregate this during mapreduce, but that's not done here.
Reviewers: hokein, bkramer
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D30685
llvm-svn: 297371
Summary:
Previously, the insertion points would conflict with the old namespace
deletion.
Reviewers: hokein
Reviewed By: hokein
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D30490
llvm-svn: 296604
Summary:
Add usage count to find-all-symbols.
FindAllSymbols now finds (most!) main-file usages of the discovered symbols.
The per-TU map output has NumUses=0 or 1 (only one use per file is counted).
The reducer aggregates these to find the number of files that use a symbol.
The NumOccurrences is now set to 1 in the mapper rather than being inferred by
the reducer, for consistency.
The idea here is to use NumUses for ranking: intuitively number of files that
use a symbol is more meaningful than number of files that include the header.
Reviewers: hokein, bkramer
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D30210
llvm-svn: 296446
The check doesn't really know if the code it is warning about came before
or after the header guard, so phrase it more neutral instead of complaining
about code before the header guard. The location for the warning is still
not optimal, but I don't think fixing that is worth the effort, the
preprocessor doesn't give us a better location.
Differential Revision: https://reviews.llvm.org/D30191
llvm-svn: 295715
Summary:
This fixes mismatch between template decls and template specialization decls.
Also added a few more test cases.
Reviewers: bkramer
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D29447
llvm-svn: 293897
Summary:
For example, when we change 'na' to "nb::nc", we need to add leading '::' to
references "::nc::X" in the changed namespace.
Reviewers: bkramer
Reviewed By: bkramer
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D29176
llvm-svn: 293182
Summary:
Prevents misclassifying includes based on the command-line filename (e.g. if a project is in a subdirectory).
This is slightly more robust than the additional duplicate detection, however the current classification scheme is still kind of brittle for a lot of code.
Reviewers: hokein, alexfh
Subscribers: cfe-commits, #clang-tools-extra
Patch by Julian Bangert!
Differential Revision: https://reviews.llvm.org/D26015
llvm-svn: 291767
Summary:
Fix no std::function index.
Previously, we don't index all the template specialization declarations of functions and classes (Because we assume that template functions/classes are indexed by their template declarations), but this is not always true in some cases like `std::function` which only has a partial template specialization declaration.
Reviewers: ioeric, bkramer
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D27920
llvm-svn: 291669
We don't actually need the index until parse time, so fetch it in the
background and start parsing. By the time it is actually needed it's
likely that the loading phase has completed in the background.
llvm-svn: 291446
Summary: Previously, a `\n` might be left in the old namespace and thus not copied to the new namespace, which is bad.
Reviewers: hokein
Subscribers: alexshap, cfe-commits
Differential Revision: https://reviews.llvm.org/D28282
llvm-svn: 290966
Summary:
Instead of moving all the helper declarations blindly, this patch
implements an AST-based call graph solution to make clang-move only move used
helper decls to new.cc and remove unused decls in old.cc.
Depends on D27674.
Reviewers: ioeric
Subscribers: mgorny, cfe-commits
Differential Revision: https://reviews.llvm.org/D27673
llvm-svn: 290873
Summary: Also make sure one function reference is only processed once.
Reviewers: hokein
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D27982
llvm-svn: 290176
This will look through typedefs so include-fixer will look up the target
of the typedef instead of the typedef itself (which is already in
scope).
llvm-svn: 289952
Summary: This fixes templated type aliases and templated type aliases in classes.
Reviewers: hokein
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D27801
llvm-svn: 289799
Marc-Andre Laperle