Summary:
Instead of receiving compilation commands, auto-index is triggered by just
filenames to reindex, and gets commands from the global comp DB internally.
This has advantages:
- more of the work can be done asynchronously (fetching compilation commands
upfront can be slow for large CDBs)
- we get access to the CDB which can be used to retrieve interpolated commands
for headers (useful in some cases where the original TU goes away)
- fits nicely with the filename-only change observation from r347297
The interface to GlobalCompilationDatabase gets extended: when retrieving a
compile command, the GCDB can optionally report the project the file belongs to.
This naturally fits together with getCompileCommand: it's hard to implement one
without the other. But because most callers don't care, I've ended up with an
awkward optional-out-param-in-virtual method pattern - maybe there's a better
one.
This is the main missing integration point between ClangdServer and
BackgroundIndex, after this we should be able to add an auto-index flag.
Reviewers: ioeric, kadircet
Subscribers: MaskRay, jkorous, arphaman, cfe-commits, ilya-biryukov
Differential Revision: https://reviews.llvm.org/D54865
llvm-svn: 347538
Summary:
The fix for aggregate initialization (`std::make_unique<Foo>(Foo {1, 2})` needs
to see Foo copy constructor, otherwise we will have a compiler error. So we
only emit the check warning.
Reviewers: JonasToth, aaron.ballman
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D54745
llvm-svn: 347537
Removed the uses of the allOf() matcher inside node matchers that are implicit
allOf(). Replaced uses of allOf() with the explicit node matcher where it makes
matchers more readable. Replace anyOf(hasName(), hasName(), ...) with the more
efficient and readable hasAnyName().
llvm-svn: 347520
The test fails with a local modification to
clang-tidy/ClangTidyDiagnosticConsumer.cpp to include fixes into the key when
deduplicating the warnings.
llvm-svn: 347495
The test I'm adding passes without the change due to the deduplication logic in
ClangTidyDiagnosticConsumer::take(). However this bug manifests in our internal
integration with clang-tidy.
I've verified the fix by locally changing LessClangTidyError to consider
replacements.
llvm-svn: 347470
Summary:
Previously, removeDoc followed by an addDoc to TUScheduler resulted in
racy diagnostic responses, i.e. the old dianostics could be delivered
to the client after the new ones by TUScheduler.
To workaround this, we tracked a version number in ClangdServer and
discarded stale diagnostics. After this commit, the TUScheduler will
stop delivering diagnostics for removed files and the workaround in
ClangdServer is not required anymore.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: javed.absar, ioeric, MaskRay, jkorous, arphaman, jfb, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54829
llvm-svn: 347468
Summary:
Instead of passing around a list of supported URI schemes in clangd, we
expose an interface to convert a path to URI using any compatible scheme
that has been registered. It favors customized schemes and falls
back to "file" when no other scheme works.
Changes in this patch are:
- URI::create(AbsPath, URISchemes) -> URI::create(AbsPath). The new API finds a
compatible scheme from the registry.
- Remove URISchemes option everywhere (ClangdServer, SymbolCollecter, FileIndex etc).
- Unit tests will use "unittest" by default.
- Move "test" scheme from ClangdLSPServer to ClangdMain.cpp, and only
register the test scheme when lit-test or enable-lit-scheme is set.
(The new flag is added to make lit protocol.test work; I wonder if there
is alternative here.)
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54800
llvm-svn: 347467
Summary:
The full path of the input header depends on the execution environment
and may result in different behavior (e.g. when different URI schemes are used).
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54833
llvm-svn: 347466
Summary:
- Reads are never executed if canceled before ready-to run.
In practice, we finalize cancelled reads eagerly and out-of-order.
- Cancelled reads don't prevent prior updates from being elided, as they don't
actually depend on the result of the update.
- Updates are downgraded from WantDiagnostics::Yes to WantDiagnostics::Auto when
cancelled, which allows them to be elided when all dependent reads are
cancelled and there are subsequent writes. (e.g. when the queue is backed up
with cancelled requests).
The queue operations aren't optimal (we scan the whole queue for cancelled
tasks every time the scheduler runs, and check cancellation twice in the end).
However I believe these costs are still trivial in practice (compared to any
AST operation) and the logic can be cleanly separated from the rest of the
scheduler.
Reviewers: ilya-biryukov
Subscribers: javed.absar, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54746
llvm-svn: 347450
Summary:
Currently the smart_ptr check (modernize-make-unique) generates the
fixes that cannot compile for cases like below -- because brace list can
not be deduced in `make_unique`.
```
struct Bar { int a, b; };
struct Foo { Foo(Bar); };
auto foo = std::unique_ptr<Foo>(new Foo({1, 2}));
```
Reviewers: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D54704
llvm-svn: 347315
Summary:
This is needed to correctly handle checks that use IncludeInserter,
which is very common.
I couldn't find a totally safe example of a check to enable for testing,
I picked modernize-deprecated-headers which some will probably hate.
We should get configuration working...
This depends on D54691 which ensures our calls to getFile(open=false)
don't break subsequent accesses via the FileManager.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54694
llvm-svn: 347298
Summary:
Currently, changes *within* CDBs are not tracked (CDB has no facility to do so).
However, discovery of new CDBs are tracked (all files are marked as modified).
Also, files whose compilation commands are explicitly set are marked modified.
The intent is to use this for auto-index. Newly discovered files will be indexed
with low priority.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54475
llvm-svn: 347297
Summary:
Puts the digest of the source file that generated the index into
serialized index and stores them back on load, if exists.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D54693
llvm-svn: 347235
Summary:
§1 Description
This check finds function names in function declarations in Objective-C files that do not follow the naming pattern described in the Google Objective-C Style Guide. Function names should be in UpperCamelCase and functions that are not of static storage class should have an appropriate prefix as described in the Google Objective-C Style Guide. The function `main` is a notable exception. Function declarations in expansions in system headers are ignored.
Example conforming function definitions:
```
static bool IsPositive(int i) { return i > 0; }
static bool ABIsPositive(int i) { return i > 0; }
bool ABIsNegative(int i) { return i < 0; }
```
A fixit hint is generated for functions of static storage class but otherwise the check does not generate a fixit hint because an appropriate prefix for the function cannot be determined.
§2 Test Notes
* Verified clang-tidy tests pass successfully.
* Used check_clang_tidy.py to verify expected output of processing google-objc-function-naming.m
Reviewers: benhamilton, hokein, Wizard, aaron.ballman
Reviewed By: benhamilton
Subscribers: Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D51575
llvm-svn: 347132
Summary:
This is our goal. It has a non-zero rick, but so far we haven't see any
collision (externally and internally).
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54622
llvm-svn: 347044
Summary:
This runs checks over a restricted subset of the TU:
- preprocessor callbacks just receive the truncated PP events that
occur when a preamble is used.
- ASTMatchers run only over the top-level decls in the main-file
This patch just turns on one simple check (bugprone-sizeof-expression)
with no configuration. Configuration is complex enough to warrant a separate patch
This depends on a patch allowing traversal to be restricted to a scope.
Reviewers: hokein
Subscribers: srhines, mgorny, ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54204
llvm-svn: 347036
I haven't been involved with the project for years, so it's probably
best for someone else to be the code owner.
Differential Revision: https://reviews.llvm.org/D54453
llvm-svn: 346998
Summary:
For symbols in global namespace (without any scope), we need to
add global scope "" to the fuzzy request.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54519
llvm-svn: 346947
Add support for "polymorphic" types to YAMLIO.
PolymorphicTraits can dynamically switch between other traits (Scalar, Map, or
Sequence). When inputting, the PolymorphicTraits type is told which type to
become, and when outputting the PolymorphicTraits type is asked which type it
currently is.
Also add support for TaggedScalarTraits to allow dynamically differentiating
between multiple scalar types using YAML tags.
Serialize empty maps as "{}" and empty sequences as "[]", so that types
are preserved when round-tripping PolymorphicTraits. This change has
equivalent semantics, but may break e.g. tests which compare output
verbatim.
Differential Revision: https://reviews.llvm.org/D48144
llvm-svn: 346884
Summary:
This would save us 8 bytes per ref, and buy us ~40MB in total
for llvm index (from ~300MB to ~260 MB).
The char pointer must be null-terminated, and llvm::StringSaver
guarantees it.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53427
llvm-svn: 346852
Summary:
Previously code completion did not work well for Objective-C methods
which contained multiple arguments as clangd did not expect to see
multiple typed-text chunks when handling code completion.
Note that even with this change, we do not consider selector fragments
from previous arguments to be part of the signature (although we
could in the future).
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, jfb, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53934
llvm-svn: 346836
Summary:
[[ https://bugs.llvm.org/show_bug.cgi?id=39224 | PR39224 ]]
As discussed, we can't always do the transform automatically due to that array-to-pointer decay of C array.
In order to detect whether we can do said transform, we'd need to be able to see all usages of said array,
which is, i would say, rather impossible if e.g. it is in the header.
Thus right now no fixit exists.
Exceptions: `extern "C"` code.
References:
* [[ https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es27-use-stdarray-or-stack_array-for-arrays-on-the-stack | CPPCG ES.27: Use std::array or stack_array for arrays on the stack ]]
* [[ https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon1-prefer-using-stl-array-or-vector-instead-of-a-c-array | CPPCG SL.con.1: Prefer using STL array or vector instead of a C array ]]
* HICPP `4.1.1 Ensure that a function argument does not undergo an array-to-pointer conversion`
* MISRA `5-2-12 An identifier with array type passed as a function argument shall not decay to a pointer`
Reviewers: aaron.ballman, JonasToth, alexfh, hokein, xazax.hun
Reviewed By: JonasToth
Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53771
llvm-svn: 346835
The new checker searches for those for loops which has a loop variable with a "too small" type which means this type can't represent all values which are part of the iteration range.
For example:
```
int main() {
long size = 300000;
for( short int i = 0; i < size; ++i) {}
}
```
The short type leads to infinite loop here because it can't store all values in the `[0..size]` interval. In a real use case, size means a container's size which depends on the user input. Which means for small amount of objects the algorithm works, but with a larger user input the software will freeze.
The idea of the checker comes from the LibreOffice project, where the same check was implemented as a clang compiler plugin, called `LoopVarTooSmall` (LLVM licensed).
The idea is the same behind this check, but the code is different because of the different framework.
Patch by ztamas.
Reviewers: alexfh, hokein, aaron.ballman, JonasToth, xazax.hun, whisperity
Reviewed By: JonasToth, whisperity
Differential Revision: https://reviews.llvm.org/D53974
llvm-svn: 346665
Summary:
The fix to the issue that `const char* p = ("foo")` is diagnosed as decay
is to ignored the ParenCast.
Resolves PR39583
Reviewers: aaron.ballman, alexfh, hokein
Reviewed By: aaron.ballman
Subscribers: nemanjai, xazax.hun, kbarton, cfe-commits
Differential Revision: https://reviews.llvm.org/D54281
llvm-svn: 346555
Summary:
These get passed to HandleTopLevelDecl() if they happen to have been
deserialized for any reason. We don't want to treat them as part of the
main file.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54303
llvm-svn: 346503
Our testing didn't reflect reality: live clangd almost always uses a
preamble, and sometimes the preamble behaves differently.
This patch fixes a common test helper to be more realistic.
Preamble doesn't preserve information about which tokens come from the
command-line (this gets inlined into a source file). So remove logic
that attempts to treat symbols with such names differently.
A SymbolCollectorTest tries to verify that locals in headers are not
indexed, with preamble enabled this is only meaningful for locals of
auto-typed functions (otherwise the bodies aren't parsed).
Tests were relying on the fact that the findAnyDecl helper actually did expose
symbols from headers. Resolve by making all these functions consistently
able to find symbols in headers/preambles.
llvm-svn: 346488
Summary:
Clang's hierarchy is CompilerInstance -> DiagnosticsEngine -> DiagnosticConsumer.
(Ownership is optional/shared, but this structure is fairly clear).
Currently ClangTidyDiagnosticConsumer *owns* the DiagnosticsEngine:
- this inverts the hierarchy, which is confusing
- this means ClangTidyDiagnosticConsumer() mutates the passed-in context, which
is both surprising and limits flexibility
- it's not possible to use a different DiagnosticsEngine with ClangTidy
This means a little bit more code in the places ClangTidy is used standalone,
but more flexibility in using ClangTidy with other diagnostics configurations.
Reviewers: hokein
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D54033
llvm-svn: 346418
Summary:
Namespace references is less useful compared with other symbols, and
they contribute large part of the index. This patch drops them.
The number of refs is reduced from 5.4 million to 4.7 million.
| | Before | After |
|file size | 78 MB | 71MB |
|memory | 330MB | 300MB|
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54202
llvm-svn: 346319
Summary:
For example, when anonymous namespace is present, duplicated namespaces might be
generated for the enclosing namespace.
Reviewers: ilya-biryukov
Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54105
llvm-svn: 346224
Summary:
This allows us to deduplicate header symbols across TUs. File digests
are collects when collecting symbols/refs. And the index store deduplicates
file symbols based on the file digest.
Reviewers: sammccall, hokein
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53433
llvm-svn: 346221
Summary:
By now the context's SourceManager is now initialized everywhere that
ClangTidyCheck::registerMatcher() is called, so the call from run() seems
entirely redundant, and indeed all the tests pass.
This solves a problem with embedding clang-tidy: if using a DiagnosticsEngine
which already has file state, re-setting its SourceManager (to the same value)
causes an assertion.
(There are other ways to solve this problem, but this is the simplest).
Reviewers: hokein, alexfh
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D54061
llvm-svn: 346219
r345961 introduced a path check in
.\tools\clang\tools\extra\test\clang-tidy\clang-tidy-run-with-database.cpp.
r345961 added a check line for a path that only handled / on unix
machines and not \ on windows machines.
This patch handles both cases.
Patch by TWeaver.
Differential Revision: https://reviews.llvm.org/D54036
llvm-svn: 345995
Summary:
This allows customizing the flags used when no compile database is
available. It addresses some uses of the old extraFlags extension.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53688
llvm-svn: 345973
The size of an os_log buffer is known at any stage of compilation, so making it
a constant expression means that the common idiom of declaring a buffer for it
won't result in a VLA. That allows the compiler to skip saving and restoring
the stack pointer around such buffers.
This also moves the OSLog and other FormatString helpers from
libclangAnalysis to libclangAST to avoid a circular dependency.
llvm-svn: 345971
Summary:
The new implementation is a GlobalCompilationDatabase that overlays a base.
Normally this is the directory-based CDB.
To preserve the behavior of compile_args_from=LSP, the base may be null.
The OverlayCDB is always present, and so the extensions to populate it
are always supported.
It also allows overriding the flags of the fallback command. This is
just unit-tested for now, but the plan is to expose this as an extension
on the initialize message. This addresses use cases like
https://github.com/thomasjo/atom-ide-cpp/issues/16
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53687
llvm-svn: 345970
Summary:
This was added in D34947 to support YCM, but YCM actually provides *all* args,
and this was never actually used.
Meanwhile, we grew another extension that allows specifying all args.
I did find one user of this extension: https://github.com/thomasjo/atom-ide-cpp.
I'll reach out, there are multiple good alternatives:
- compile_commands.txt can serve the same purpose as .clang_complete there
- we can add an extension to support setting the fallback command
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53641
llvm-svn: 345969
Summary:
Currently ClangTidyContext::diag() sends the diagnostics to a
DiagnosticsEngine, which probably delegates to a ClangTidyDiagnosticsConsumer,
which is supposed to go back and populate ClangTidyContext::Errors.
After this patch, the diagnostics are stored in the ClangTidyDiagnosticsConsumer
itself and can be retrieved from there.
Why?
- the round-trip from context -> engine -> consumer -> context is confusing
and makes it harder to establish layering between these things.
- context does too many things, and makes it hard to use clang-tidy as a library
- everyone who actually wants the diagnostics has access to the ClangTidyDiagnosticsConsumer
The most natural implementation (ClangTidyDiagnosticsConsumer::take()
finalizes diagnostics) causes a test failure: clang-tidy-run-with-database.cpp
asserts that clang-tidy exits successfully when trying to process a file
that doesn't exist.
In clang-tidy today, this happens because finish() is never called, so the
diagnostic is never flushed. This looks like a bug to me.
For now, this patch carefully preserves that behavior, but I'll ping the
authors to see whether it's deliberate and worth preserving.
Reviewers: hokein
Subscribers: xazax.hun, cfe-commits, alexfh
Differential Revision: https://reviews.llvm.org/D53953
llvm-svn: 345961
The intention was to fall through to Function case in LLVM_ENABLE_ASSERTIONS=Off builds.
Use #ifndef NDEBUG to fix -Wimplicit-fallthrough
llvm-svn: 345953
This patch should not introduce any behavior changes. It consists of
mostly one of two changes:
1. Replacing fall through comments with the LLVM_FALLTHROUGH macro
2. Inserting 'break' before falling through into a case block consisting
of only 'break'.
We were already using this warning with GCC, but its warning behaves
slightly differently. In this patch, the following differences are
relevant:
1. GCC recognizes comments that say "fall through" as annotations, clang
doesn't
2. GCC doesn't warn on "case N: foo(); default: break;", clang does
3. GCC doesn't warn when the case contains a switch, but falls through
the outer case.
I will enable the warning separately in a follow-up patch so that it can
be cleanly reverted if necessary.
Reviewers: alexfh, rsmith, lattner, rtrieu, EricWF, bollu
Differential Revision: https://reviews.llvm.org/D53950
llvm-svn: 345882
Summary: This will make clang-tidy accept property names like xyz_URL (URL is a common acronym).
Reviewers: benhamilton, hokein
Reviewed By: benhamilton
Subscribers: jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D53955
llvm-svn: 345858
This check flags function top-level const-qualified return types and suggests removing the mostly-superfluous const qualifier where possible.
Patch by Yitzhak Mandelbaum.
llvm-svn: 345764
Summary:
This patch introduces a new clang-tidy check that matches on all `declStmt` that declare more then one variable
and transform them into one statement per declaration if possible.
It currently only focusses on variable declarations but should be extended to cover more kinds of declarations in the future.
It is related to https://reviews.llvm.org/D27621 and does use it's extensive test-suite. Thank you to firolino for his work!
Reviewers: rsmith, aaron.ballman, alexfh, hokein, kbobyrev
Reviewed By: aaron.ballman
Subscribers: ZaMaZaN4iK, mgehre, nemanjai, kbarton, lebedev.ri, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D51949
llvm-svn: 345735
Summary:
Since llvm/Config/config.h is not available on standalone builds,
use __USE_POSIX instead of HAVE_PTHREAD_H and get rid of the include.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: lebedev.ri, krytarowski, ilya-biryukov, ioeric, jkorous, arphaman, jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D53935
llvm-svn: 345729
There is no SCHED_IDLE semantic equivalent in BSD systems.
Reviewers: kadircet, sammccall
Revieweed By: sammccall
Differential Revision: https://reviews.llvm.org/D53922
llvm-svn: 345700
We haven't supported compiling ObjC1 for a long time (and never will again), so
there isn't any reason to keep these separate. This patch replaces
LangOpts::ObjC1 and LangOpts::ObjC2 with LangOpts::ObjC.
Differential revision: https://reviews.llvm.org/D53547
llvm-svn: 345637
Summary:
The macro may not have location (or more generally, the location may not exist),
e.g. if it originates from compiler's command-line.
The check complains on all the macros, even those without the location info.
Which means, it only says it does not like it. What is 'it'? I have no idea.
If we don't print the name, then there is no way to deal with that situation.
And in general, not printing name here forces the user to try to understand,
given, the macro definition location, what is the macro name?
This isn't fun.
Also, ignores-by-default the macros originating from command-line,
with an option to not ignore those.
I suspect some more issues may crop up later.
Reviewers: JonasToth, aaron.ballman, hokein, xazax.hun, alexfh
Reviewed By: JonasToth, aaron.ballman
Subscribers: nemanjai, kbarton, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53817
llvm-svn: 345610
Summary:
Add granular options for AST dumping, text printing and diagnostics.
This makes it possible to
* Have both diag and dump active at once
* Extend the output with other queryable content in the future.
Reviewers: aaron.ballman, pcc, ioeric, ilya-biryukov, klimek, sammccall
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52857
llvm-svn: 345522
Make the following changes to PredefinedExpr:
1. Move PredefinedExpr below StringLiteral so that it can use its definition.
2. Rename IdentType to IdentKind to be more in line with clang's conventions,
and propagate the change to its users.
3. Move the location and the IdentKind into the newly available space of
the bit-fields of Stmt.
4. Only store the function name when needed. When parsing all of Boost,
of the 1357 PredefinedExpr 919 have no function name.
Differential Revision: https://reviews.llvm.org/D53605
Reviewed By: rjmccall
llvm-svn: 345460
Summary:
Detects when the integral literal or floating point (decimal or hexadecimal)
literal has non-uppercase suffix, and suggests to make the suffix uppercase,
with fix-it.
All valid combinations of suffixes are supported.
```
auto x = 1; // OK, no suffix.
auto x = 1u; // warning: integer literal suffix 'u' is not upper-case
auto x = 1U; // OK, suffix is uppercase.
...
```
This is a re-commit, the original was reverted by me in
rL345305 due to discovered bugs. (implicit code, template instantiation)
Tests were added, and the bugs were fixed.
I'm unable to find any further bugs, hopefully there aren't any..
References:
* [[ https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152241 | CERT DCL16-C ]]
* MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a literal suffix
* MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case
Reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52670
llvm-svn: 345381
There are some lurking issues with the handling of the SourceManager.
Somehow sometimes we end up extracting completely wrong
portions of the source buffer.
Reverts r344772, r44760, r344758, r344755.
llvm-svn: 345305
Summary:
To enable this, TUScheduler has to provide a way to run async tasks without
needing a preamble or AST!
Reviewers: ilya-biryukov
Subscribers: javed.absar, ioeric, MaskRay, jkorous, arphaman, jfb, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53644
llvm-svn: 345268
- align struct names/comments with LSP, remove redundant "clangd" prefixes.
- don't map config structs as Optional<> when their presence/absence
doesn't signal anything and all fields must have sensible "absent" values
- be more lax around parsing of 'any'-typed messages
llvm-svn: 345235
Summary:
It doesn't make much sense: setting them is not coupled to opening the file,
it's an asynchronous notification.
I don't think this is a breaking change - this behavior is hard to observe!
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53642
llvm-svn: 345231
Summary: This will make it possible to add non-exclusive mode output.
Reviewers: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D53501
llvm-svn: 345194
Summary: Future development can then dump other content than AST.
Reviewers: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D53500
llvm-svn: 345193
Summary:
This will make it possible to easily
* Add new commands which accept <feature> parameters
* Extend the list of features
Reviewers: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D53498
llvm-svn: 345192
Summary:
This check finds cases where calls to an absl::Duration factory could use the more efficient integer overload.
For example:
// Original - Providing a floating-point literal.
absl::Duration d = absl::Seconds(10.0);
// Suggested - Use an integer instead.
absl::Duration d = absl::Seconds(10);
Patch by hwright.
Reviewers: alexfh, hokein, aaron.ballman, JonasToth
Reviewed By: hokein, JonasToth
Subscribers: zturner, xazax.hun, Eugene.Zelenko, mgorny, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53339
llvm-svn: 345167
Summary:
This information is strictly available in the log (you can find the original
call) but it makes the log easier to follow in practice.
Reviewers: kadircet
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D53647
llvm-svn: 345150
Summary:
In debug builds, getting this wrong will trigger asserts.
In production builds, it will send an error reply if none was sent,
and drop redundant replies. (And log).
No tests because this is always a programming error.
(We did have some cases of this, but I fixed them with the new dispatcher).
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, jfb, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53399
llvm-svn: 345144
Summary:
These are available via qualifiers, but signal to noise level is low.
Keep required quailifier machinery around though, for cross-ns completion.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53571
llvm-svn: 345141
Summary:
CodeAction provides us with a standard way of representing fixes inline, so
use it, replacing our existing ad-hoc extension.
After this, it's easy to serialize diagnostics using the structured
toJSON/Protocol.h mechanism rather than assembling JSON ad-hoc.
Reviewers: hokein, arphaman
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53391
llvm-svn: 345119
Summary:
The goal is 8 bytes, which has a nonzero risk of collisions with huge indexes.
This patch should shake out any issues with truncation at all, we can lower
further later.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53587
llvm-svn: 345113
Summary:
The only way to actually set the directory is at initialize time,
so now CDB is lazy we can pass it to the constructor.
Reviewers: hokein
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53572
llvm-svn: 345031
Summary:
The CDB implementations used in open-source code are fast, and our private
slow CDB will soon do the relevant caching itself.
Simplifying the GlobalCDB layer in clangd is important to get auto-index
implemented at the right layer.
Reviewers: ioeric, ilya-biryukov
Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53439
llvm-svn: 345024
Summary: This is useful when using clangd with CMake based projects in Visual Studio Code since when using CMake the `compile_commands.json` file is usually located in a `build` subdirectory which isn't a parent directory of the source files. Allowing passing relative paths to -compile-commands-dir allows specifying `clangd.arguments = ["-compile-commands-dir=build"]` in VSCode's settings file and having it work for each CMake based project that uses the `build` subdirectory as the build directory (instead of having to specify the absolute path to the compile commands directory for each separate project in VSCode's settings).
Patch by Daan De Meyer!
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53481
llvm-svn: 345022
Summary:
See http://lists.llvm.org/pipermail/clangd-dev/2018-October/000171.html for context.
I kept the error (instead of downgrading to a log message) since the range lengths differing does indicate either a bug in the client or server range calculation or the buffers being out of sync (which both seems serious enough to me to be an error). If any existing clients aside from VSCode break they should only break when accidentally typing a Unicode character which should only be a minor nuisance for a little while until the bug is fixed in the respective client.
Patch by Daan De Meyer!
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53527
llvm-svn: 345020
Summary:
For example:
```
namespace util { class Base; }
namespace new {
namespace util { class Internal; }
}
namespace old {
util::Base b1;
}
```
When changing `old::` to `new::`, `util::` in namespace "new::" will conflict
with "new::util::" unless a leading "::" is added.
Reviewers: hokein
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D53489
llvm-svn: 344897
And also enable it by default to be consistent with e.g. modernize-use-using.
This helps e.g. when running this check on client code where the macro is
provided by the system, so there is no easy way to modify it.
Reviewed By: JonasToth
Differential Revision: https://reviews.llvm.org/D53454
llvm-svn: 344871
Standardize on the most common namespace setup in our *.cpp files:
using namespace llvm;
namespace clang {
namespace clangd {
void foo(StringRef) { ... }
And remove redundant llvm:: qualifiers. (Except for cases like
make_unique where this causes problems with std:: and ADL).
This choice is pretty arbitrary, but some broad consistency is nice.
This is going to conflict with everything. Sorry :-/
Squash the other configurations:
A)
using namespace llvm;
using namespace clang;
using namespace clangd;
void clangd::foo(StringRef);
This is in some of the older files. (It prevents accidentally defining a
new function instead of one in the header file, for what that's worth).
B)
namespace clang {
namespace clangd {
void foo(llvm::StringRef) { ... }
This is fine, but in practice the using directive often gets added over time.
C)
namespace clang {
namespace clangd {
using namespace llvm; // inside the namespace
This was pretty common, but is a bit misleading: name lookup preferrs
clang::clangd::foo > clang::foo > llvm:: foo (no matter where the using
directive is).
llvm-svn: 344850
Summary:
This is useful if using clang-query -f with a file containing multiple
matchers.
Reviewers: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52859
llvm-svn: 344840
Summary:
Rename instance variable to WorkspaceRoot to match what we call it internally.
Add fixme to set it automatically. Don't do it yet, clients have assumptions
that the constructor won't access the FS.
Don't second-guess the provided root.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53404
llvm-svn: 344787
That revision changed integer members to bitfields; the integers were
default initialized before and the bitfields lost that default
initialization. This started causing msan use-of-uninitialized memory in
clangd tests.
llvm-svn: 344773
Summary:
Detects when the integral literal or floating point (decimal or hexadecimal)
literal has non-uppercase suffix, and suggests to make the suffix uppercase,
with fix-it.
All valid combinations of suffixes are supported.
```
auto x = 1; // OK, no suffix.
auto x = 1u; // warning: integer literal suffix 'u' is not upper-case
auto x = 1U; // OK, suffix is uppercase.
...
```
References:
* [[ https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152241 | CERT DCL16-C ]]
* MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a literal suffix
* MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case
Reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52670
llvm-svn: 344755
Summary:
The RefSlab::size can easily cause confusions, it returns the number of
different symbols, rahter than the number of all references.
- add numRefs() method and cache it, since calculating it everytime is nontrivial.
- clear misused places.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53389
llvm-svn: 344745
Summary:
LSP is a slightly awkward map to C++ object lifetimes: the initialize request
is part of the protocol and provides information that doesn't change over the
lifetime of the server.
Until now, we handled this by initializing ClangdServer and ClangdLSPServer
right away, and making anything that can be set in the "initialize" request
mutable.
With this patch, we create ClangdLSPServer immediately, but defer creating
ClangdServer until "initialize". This opens the door to passing the relevant
initialize params in the constructor and storing them immutably.
(That change isn't actually done in this patch).
To make this safe, we have the MessageDispatcher enforce that the "initialize"
method is called before any other (as required by LSP). That way each method
handler can assume Server is initialized, as today.
As usual, while implementing this I found places where our test cases violated
the protocol.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53398
llvm-svn: 344741
Summary:
Most of its functionality is moved into ClangdLSPServer.
The decoupling between JSONRPCDispatcher, ProtocolCallbacks, ClangdLSPServer
was never real, and only served to obfuscate.
Some previous implicit/magic stuff is now explicit:
- the return type of LSP method calls are now in the signature
- no more reply() that gets the ID using global context magic
- arg tracing no longer relies on RequestArgs::stash context magic either
This is mostly refactoring, but some deliberate fixes while here:
- LSP method params are now by const reference
- notifications and calls are now distinct namespaces.
(some tests had protocol errors and needed updating)
- we now reply to calls we failed to decode
- outgoing calls use distinct IDs
A few error codes and message IDs changed in unimportant ways (see tests).
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman, jfb, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53387
llvm-svn: 344737
Summary:
These are often not expected to be used directly e.g.
```
TEST_F(Fixture, X) {
^ // "Fixture_X_Test" expanded in the macro should be down ranked.
}
```
Only doing this for sema for now, as such symbols are mostly coming from sema
e.g. gtest macros expanded in the main file. We could also add a similar field
for the index symbol.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53374
llvm-svn: 344736
Summary:
This would buy us more memory. Using a 32-bits integer is enough for
most human-readable source code (up to 4M lines and 4K columns).
Previsouly, we used 8 bytes for a position, now 4 bytes, it would save
us 8 bytes for each Ref and each Symbol instance.
For LLVM-project binary index file, we save ~13% memory.
| Before | After |
| 412MB | 355MB |
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53363
llvm-svn: 344735
Summary:
Previously, ptr.reset(new char[5]) will be replaced with `p =
make_unique<char[]>(5)`, the fix has side effect -- doing
default initialization, it may cause performace regression (we are
bitten by this rececntly)
The check should be conservative for these cases.
Reviewers: alexfh
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D53377
llvm-svn: 344733
Makes bitcode tests line up with what's actually called in the tool.
Should fix the failing bot.
Also fixes a warning that was being thrown about initialization braces.
Differential Revision: https://reviews.llvm.org/D53381
llvm-svn: 344707
Summary:
This should make all-scope completion more usable. Scope proximity for
indexes will be added in followup patch.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53131
llvm-svn: 344688
Summary:
Add a flag to SymbolCollector to collect refs fdrom headers.
Note that we collect refs from headers in static index, and we don't do it for
dynamic index because of the preamble (we skip function body in preamble,
collecting it will result incomplete results).
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53322
llvm-svn: 344678
Summary:
Instead of parsing into structs that mirror LSP, simply parse into a flat struct
that contains the info we need.
This is an exception to our strategy with Protocol.h, which seems justified:
- the structure here is very large and deeply nested
- we care about almost none of it
- we should never have to serialize client capabilities
Reviewers: kadircet
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D53266
llvm-svn: 344673
Summary:
This paves the way for alternative transports (mac XPC, maybe messagepack?),
and also generally improves layering: testing ClangdLSPServer becomes less of
a pipe dream, we split up the JSONOutput monolith, etc.
This isn't a final state, much of what remains in JSONRPCDispatcher can go away,
handlers can call reply() on the transport directly, JSONOutput can be renamed
to StreamLogger and removed, etc. But this patch is sprawling already.
The main observable change (see tests) is that hitting EOF on input is now an
error: the client should send the 'exit' notification.
This is defensible: the protocol doesn't spell this case out. Reproducing the
current behavior for all combinations of shutdown/exit/EOF clutters interfaces.
We can iterate on this if desired.
Reviewers: jkorous, ioeric, hokein
Subscribers: mgorny, ilya-biryukov, MaskRay, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53286
llvm-svn: 344672
Now that the clang-doc libraries are covered by unit tests, we don't
need to have extensive (and unmaintainable) integration tests. This
replaces the integration test suite with a smaller one that just tests
the tool itself and removes extraneous dumping logic from the tool
itself.
Includes tests that cover the parse->serialize->merge->generate
pipeline, as well as tests for the --public, --format, --doxygen, and
--output flags.
Differential Revision: https://reviews.llvm.org/D53150
llvm-svn: 344655
Add unit tests for Markdown generation.
This is part of a move to convert clang-doc's tests to a more
maintainable unit test framework, with a smaller number of integration
tests to maintain and more granular failure feedback.
Differential Revision: https://reviews.llvm.org/D53085
llvm-svn: 344654
Adds unit tests for the YAML generator library.
This is part of a move to convert clang-doc's tests to a more
maintainable unit test framework, with a smaller number of integration
tests to maintain and more granular failure feedback.
Differential Revision: https://reviews.llvm.org/D53084
llvm-svn: 344653
Adds unit tests for the merging logic in Respresentation.cpp.
This is part of a move to convert clang-doc's tests to a more
maintainable unit test framework, with a smaller number of integration
tests to maintain and more granular failure feedback.
Differential Revision: https://reviews.llvm.org/D53083
llvm-svn: 344652
Adds unit tests for the BitcodeWriter and BitcodeReader libraries.
This is part of a move to convert clang-doc's tests to a more
maintainable unit test framework, with a smaller number of integration
tests to maintain and more granular failure feedback.
Differential Revision: https://reviews.llvm.org/D53082
llvm-svn: 344651
Adds unit tests for the Serialize library.
This is part of a move to convert clang-doc's tests to a more
maintainable unit test framework, with a smaller number of integration
tests to maintain and more granular failure feedback.
Differential Revision: https://reviews.llvm.org/D53081
llvm-svn: 344650
Summary:
This paves the way for alternative transports (mac XPC, maybe messagepack?),
and also generally improves layering: testing ClangdLSPServer becomes less of
a pipe dream, we split up the JSONOutput monolith, etc.
This isn't a final state, much of what remains in JSONRPCDispatcher can go away,
handlers can call reply() on the transport directly, JSONOutput can be renamed
to StreamLogger and removed, etc. But this patch is sprawling already.
The main observable change (see tests) is that hitting EOF on input is now an
error: the client should send the 'exit' notification.
This is defensible: the protocol doesn't spell this case out. Reproducing the
current behavior for all combinations of shutdown/exit/EOF clutters interfaces.
We can iterate on this if desired.
Reviewers: jkorous, ioeric, hokein
Subscribers: mgorny, ilya-biryukov, MaskRay, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53286
llvm-svn: 344620
Summary:
I don't bother mirroring the full capabilities struct, just parse the
bits we care about. I'll send a new patch to use this approach elsewhere too.
Reviewers: kadircet
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D53213
llvm-svn: 344617
Summary:
This patch removes the possibility to change the compilation database
path at runtime using the didChangeConfiguration request. Instead, it
is suggested to use the setting on the initialize request, and clangd
whenever the user wants to use a different build configuration.
Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53220
llvm-svn: 344614
Summary:
This is useful for symbo scope proximity, where down traversals from
the global scope if not desired.
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53317
llvm-svn: 344604
Summary:
One relatively boring bug: forgot to notify the CV after enqueue.
One much more fun bug: the thread member could access instance variables before
they were initialized. Although the thread was last in the init list, QueueCV
etc were listed after Thread in the class, so their default constructors raced
with the thread itself.
We have to get very unlucky to lose this race, I saw it 0.02% of the time.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D53313
llvm-svn: 344595
Summary:
Reuse the old -use-dex-index experiment flag for this.
To avoid breaking the tests, make Dex deduplicate symbols, addressing an old FIXME.
Reviewers: hokein
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53288
llvm-svn: 344594
Haojian Wu