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
Summary:
See tinyurl.com/clangd-automatic-index for design and goals.
Lots of limitations to keep this patch smallish, TODOs everywhere:
- no serialization to disk
- no changes to dynamic index, which now has a much simpler job
- no partitioning of symbols by file to avoid duplication of header symbols
- no reindexing of edited files
- only a single worker thread
- compilation database is slurped synchronously (doesn't scale)
- uses memindex, rebuilds after every file (should be dex, periodically)
It's not hooked up to ClangdServer/ClangdLSPServer yet: the layering
isn't clear (it should really be in ClangdServer, but ClangdLSPServer
has all the CDB interactions).
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D53032
llvm-svn: 344513
Summary:
Previously, SymbolCollector postfilters all references at the end to
find all references of interesting symbols.
It was incorrect when indxing main AST where we don't see locations
of symbol declarations and definitions in the main AST (as those are in
preamble AST).
The fix is to do earily check during collecting references.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53273
llvm-svn: 344507
Checking whether a functions throws indirectly may be very expensive because it
needs to visit its whole call graph. Therefore we should first check whether the
function is forbidden to throw and only check whether it throws afterward. This
also seems to solve bug https://bugs.llvm.org/show_bug.cgi?id=39167 where the
execution time is so long that it seems to hang.
Differential Revision: https://reviews.llvm.org/D53187
llvm-svn: 344444
And also enable it by default to be consistent with e.g. modernize-use-using.
This improves consistency inside the check itself as well: both checks are now
disabled in macros by default.
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: alexfh
Differential Revision: https://reviews.llvm.org/D53217
llvm-svn: 344440
New checker called bugprone-not-null-terminated-result. This check finds function calls where it is possible to cause a not null-terminated result. Usually the proper length of a string is strlen(src) + 1 or equal length of this expression, because the null terminator needs an extra space. Without the null terminator it can result in undefined behaviour when the string is read.
The following function calls are checked:
memcpy, wmemcpy, memcpy_s, wmemcpy_s, memchr, wmemchr, memmove, wmemmove, memmove_s, wmemmove_s, memset, wmemset, strerror_s, strncmp, wcsncmp, strxfrm, wcsxfrm
The following is a real-world example where the programmer forgot to increase the passed third argument, which is size_t length. That is why the length of the allocated memory is problematic too.
static char *StringCpy(const std::string &str) {
char *result = reinterpret_cast<char *>(malloc(str.size()));
memcpy(result, str.data(), str.size());
return result;
}
After running the tool fix-it rewrites all the necessary code according to the given options. If it is necessary, the buffer size will be increased to hold the null terminator.
static char *StringCpy(const std::string &str) {
char *result = reinterpret_cast<char *>(malloc(str.size() + 1));
strcpy(result, str.data());
return result;
}
Patch by Charusso.
Differential ID: https://reviews.llvm.org/D45050
llvm-svn: 344374
New option added to these three checks to be able to silence false positives on
types that are intentionally passed by value or copied. Such types are e.g.
intrusive reference counting pointer types like llvm::IntrusiveRefCntPtr. The
new option is named WhiteListTypes and can contain a semicolon-separated list of
names of these types. Regular expressions are allowed. Default is empty.
Differential Revision: https://reviews.llvm.org/D52727
llvm-svn: 344340
Summary:
The use sites are enclosed by `namespace clang`, so clang:: is not
necessary. Many unqualified names have already been used, e.g. SourceManager SourceLocation LangOptions. This change makes the code terser and more consistent.
Reviewers: hokein
Reviewed By: hokein
Subscribers: ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D53060
llvm-svn: 344256
Summary:
I think this was just copied from somewhere with the belief that it actually
did some crash handling.
Of course the question arises: *should* we set one? I don't think so:
- clangd used to crash a lot, now it's pretty stable, because we found and
fixed the crashes. I think the long-term effects of crashing hard are good.
- the implementation can't do any magic, it just uses longjmp to return without
running any destructors by default. This is unsafe in general (e.g. mutexes
won't unlock) and will certainly end up leaking memory. Whatever UB caused
the crash may still stomp all over global state, etc.
I think there's an argument for isolating the background indexer (autoindex)
because it's not directly under the user's control, the crash surface is larger,
and it doesn't particularly need to interact with the rest of clangd.
But there, fork() and communicate through the FS is safer.
Reviewers: ioeric, ilya-biryukov
Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53034
llvm-svn: 344245
Without this, the code only compiled if the header was included after
something introduced the alias from `clang::` to `llvm::` for this type.
Any modules build would fail here.
llvm-svn: 344225
Heejin Ahn