Summary:
Hello, i would like to suggest a fix for one of the checks in clang-tidy.
The bug was reported in https://bugs.llvm.org/show_bug.cgi?id=38039 where you can find more information.
```
struct UOB{
UOB(const UOB &Other):j{Other.j}{}
int j;
};
```
In this case the check modernize-use-equals-default does not detect copy constructors that can be defaulted; that should be:
```
struct UOB{
UOB(const UOB &Other) = default;
int j;
};
```
Reviewers: aaron.ballman, hokein, alexfh
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D49356
llvm-svn: 337286
Finds functions which may throw an exception directly or indirectly, but they
should not: Destructors, move constructors, move assignment operators, the
main() function, swap() functions, functions marked with throw() or noexcept
and functions given as option to the checker.
Differential Revision: https://reviews.llvm.org/D33537
llvm-svn: 336997
Summary:
The goal is to reduce false positives when the difference is intentional, like:
foo(StringRef name);
foo(StringRef name_ref) {
string name = cleanup(name_ref);
...
}
Or semantically unimportant, like:
foo(StringRef full_name);
foo(StringRef name) { ... }
There are other matching names we won't recognise (e.g. syns vs synonyms) but
this catches many that we see in practice, and gives people a systematic
workaround.
The old behavior is available as a 'Strict' option.
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D49285
llvm-svn: 336992
Having `using qualified::name;` for some symbol is an important signal
for clangd code completion as the user is more likely to use such
symbol. This patch helps to uprank the relevant symbols by saving
UsingShadowDecl in the new field of CodeCompletionResult and checking
whether the corresponding UsingShadowDecl is located in the main file
later in ClangD code completion routine. While the relative importance
of such signal is a subject to change in the future, this patch simply
bumps DeclProximity score to the value of 1.0 which should be enough for
now.
The patch was tested using
`$ ninja check-clang check-clang-tools`
No unexpected failures were noticed after running the relevant testsets.
Reviewers: sammccall, ioeric
Subscribers: MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D49012
llvm-svn: 336810
Summary:
Sema code complete in the recovery mode is generally useless. For many
cases, sema first completes in recovery context and then recovers to more useful
context, in which it's favorable to ignore results from recovery (as results are
often bad e.g. all builtin symbols and top-level symbols). There is also case
where only sema would fail to recover e.g. completions in excluded #if block.
Sema would try to give results, but the results are often useless (see the updated
excluded #if block test).
Reviewers: sammccall, ilya-biryukov
Subscribers: MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D49175
llvm-svn: 336801
Summary:
log() is split into four functions:
- elog()/log()/vlog() have different severity levels, allowing filtering
- dlog() is a lazy macro which uses LLVM_DEBUG - it logs to the logger, but
conditionally based on -debug-only flag and is omitted in release builds
All logging functions use formatv-style format strings now, e.g:
log("Could not resolve URI {0}: {1}", URI, Result.takeError());
Existing log sites have been split between elog/log/vlog by best guess.
This includes a workaround for passing Error to formatv that can be
simplified when D49170 or similar lands.
Subscribers: ilya-biryukov, javed.absar, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D49008
llvm-svn: 336785
Summary:
This gives better coverage to the check as ExprMutationAnalyzer is more
accurate comparing to isOnlyUsedAsConst.
Majority of wins come from const usage of member field, e.g.:
for (auto widget : container) { // copy of loop variable
if (widget.type == BUTTON) { // const usage only recognized by ExprMutationAnalyzer
// ...
}
}
Reviewers: george.karpenkov
Subscribers: a.sidorin, cfe-commits
Differential Revision: https://reviews.llvm.org/D48854
llvm-svn: 336737
Summary: This is not enabled in the global-symbol-builder or dynamic index yet.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D49028
llvm-svn: 336553
Summary:
The library has graduated from clangd to llvm/Support.
This is a mechanical change to move to the new API and remove the old one.
Main API changes:
- namespace clang::clangd::json --> llvm::json
- json::Expr --> json::Value
- Expr::asString() etc --> Value::getAsString() etc
- unsigned longs need a cast (due to r336541 adding lossless integer support)
Reviewers: ilya-biryukov
Subscribers: mgorny, ioeric, MaskRay, jkorous, omtcyfz, cfe-commits
Differential Revision: https://reviews.llvm.org/D49077
llvm-svn: 336549
Summary:
To avoid wasting time deserializing them on code completion and
further reparses.
We do not use the comments anyway, because we cannot rely on the file
contents staying the same for reparses that reuse the prebuilt
preamble PCH.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48943
llvm-svn: 336540
Summary:
To avoid doing extra work of processing headers in the preamble
mutilple times in parallel.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: javed.absar, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48940
llvm-svn: 336538
Summary:
An AST-based approach is used to retrieve the document symbols rather than an
in-memory index query. The index is not an ideal fit to achieve this because of
the file-centric query being done here whereas the index is suited for
project-wide queries. Document symbols also includes more symbols and need to
keep the order as seen in the file.
Signed-off-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Subscribers: tomgr, ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47846
llvm-svn: 336386
Summary:
Following D48903 ([VirtualFileSystem] InMemoryFileSystem::status: Return
a Status with the requested name), the paths output by clang-move in the
FileToReplacements map may contain leading "./". For example, where we
would get "foo.h", we'll now get "./foo.h". This breaks the tests,
because we are doing exact string lookups in the FileToFileID and
Results maps (they contain "foo.h", but we search for "./foo.h").
To mitigate this, try to normalize a little bit the paths output by
clang-move to remove that leading "./".
This patch should be safe to merge before D48903, remove_dots will just
be a no-op.
Reviewers: ilya-biryukov, hokein
Reviewed By: hokein
Subscribers: ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D48951
llvm-svn: 336358
This patch is a preparation for another one containing meaningful
changes. This patch simply removes trailing whitespaces in few files
affected by the upcoming patch and reformats
llvm-svn: 336330
The bad behavior seems to have been fixed by r336242 after all.
I thought it was persisting, but that was a different bug fixed by D48940
llvm-svn: 336320
Summary: Surface it in the completion items C++ API, and when a flag is set.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48938
llvm-svn: 336309
These checks flag use of random number generators with poor seeds that would possibly lead to degraded random number generation.
Patch by Borsik Gábor
llvm-svn: 336301
The following issues are resolved:
* Doxygen didn't generate documentation for a bunch of existing tools
due to the absence of their directories in the doxygen configuration
file. This patch adds all relevant directories to the appropriate list.
* clang-tools-extra/docs/Doxyfile seems to be unused and irrelevant,
doxygen.cfg.in is passed to the CMake's Doxygen invocation, hence
Doxyfile is removed.
The validity of proposed changes was manually checked by building
doxygen-clang-tools and making sure that clangd and other tools are
present in Doxygen-generated docs of clang-tools-extra.
Reviewers: ioeric
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D47537
llvm-svn: 336257
Summary:
For example, template parameter might not be resolved in a broken TU,
which can result in wrong USR/SymbolID.
Reviewers: ilya-biryukov
Subscribers: MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48881
llvm-svn: 336252
Summary:
We now compute a distance from the main file to the symbol header, which
is a weighted count of:
- some number of #include traversals from source file --> included file
- some number of FS traversals from file --> parent directory
- some number of FS traversals from parent directory --> child file/dir
This calculation is performed in the appropriate URI scheme.
This means we'll get some proximity boost from header files in main-file
contexts, even when these are in different directory trees.
This extended file proximity model is not yet incorporated in the index
interface/implementation.
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48441
llvm-svn: 336177
Summary:
This allows hovering on keywords that refer to deduced types.
This should cover most useful cases. Not covered:
- auto template parameters: Since this can be instantiated with many types,
it would not be practical to show the types.
- Structured binding: This could be done later to show multiple deduced types
in the hover.
- auto:: (part of concepts): Outside the scope of this patch.
Signed-off-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48159
llvm-svn: 336119
Summary:
This provides more structured information that embedders can use for rendering.
ClangdLSPServer continues to call render(), so NFC.
The patch is:
- trivial changes to ClangdServer/ClangdLSPServer
- mostly-mechanical updates to CodeCompleteTests etc for the new API
- new direct tests of render() in CodeCompleteTests
- tiny cleanups to CodeCompletionItem (operator<< and missing initializers)
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48821
llvm-svn: 336094
Summary:
LSP has some presentational fields with limited semantics (e.g. 'detail') and
doesn't provide a good place to return information like namespace.
Some places where more detailed information is useful:
- tools like quality analysis
- alternate frontends that aren't strictly LSP
- code completion unit tests
In this patch, ClangdServer::codeComplete still return LSP CompletionList, but
I plan to switch that soon (should be a no-op for ClangdLSPServer).
Deferring this makes it clear that we don't change behavior (tests stay the
same) and also keeps the API break to a small patch which can be reverted.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, cfe-commits, jkorous
Differential Revision: https://reviews.llvm.org/D48762
llvm-svn: 335980
By recomputing CompilerInvocation instead of copying it.
The problem was caused by the fact that copies of CompilerInvocation
store references to the shared state (DiagnosticOptions) when copied,
causing data races when two different copies are destroyed from
different threads.
llvm-svn: 335836
Summary:
This PR adds a few acronyms related to hashing algorithms to the standard
list in `objc-property-declaration`.
Reviewers: Wizard
Reviewed By: Wizard
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D48652
llvm-svn: 335770
Summary:
(Originally started as a clang-tidy check but there's already D45444 so shifted to just adding ExprMutationAnalyzer)
`ExprMutationAnalyzer` is a generally useful helper that can be used in different clang-tidy checks for checking whether a given expression is (potentially) mutated within a statement (typically the enclosing compound statement.)
This is a more general and more powerful/accurate version of isOnlyUsedAsConst, which is used in ForRangeCopyCheck, UnnecessaryCopyInitialization.
It should also be possible to construct checks like D45444 (suggest adding const to variable declaration) or https://bugs.llvm.org/show_bug.cgi?id=21981 (suggest adding const to member function) using this helper function.
This function is tested by itself and is intended to stay generally useful instead of tied to any particular check.
Reviewers: hokein, alexfh, aaron.ballman, ilya-biryukov, george.karpenkov
Reviewed By: aaron.ballman
Subscribers: lebedev.ri, shuaiwang, rnkovacs, hokein, alexfh, aaron.ballman, a.sidorin, Eugene.Zelenko, xazax.hun, JonasToth, klimek, mgorny, cfe-commits
Tags: #clang-tools-extra
Patch by Shuai Wang.
Differential Revision: https://reviews.llvm.org/D45679
llvm-svn: 335736
Summary:
Injected names being ranked too high was just a bug.
The high boost for keywords was intended, but was too much given how useless
keywords are. We should probably boost them on a case-by-case basis eventually.
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48368
llvm-svn: 335723
Summary:
Comments from namespaces that clangd produces are too noisy and often
not useful.
Namespaces have too many redecls and we don't have a good way of
determining which of the comments are relevant and which should be
ignored (e.g. because they come from code generators like the protobuf
compiler).
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48211
llvm-svn: 335718
Summary:
Instead of checking symbol name and container (scope) separately, check the
qualified name instead. This is much shorter and similar to how it is done
in the SymbolCollector tests.
Signed-off-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Reviewers: simark
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47847
llvm-svn: 335624
Summary:
Previously, the strings matched LSP completion pretty closely.
The completion label was a single string, for instance. This made
implementing completion itself easy but makes it hard to use the names
in other way, e.g. pretty-printed name in synthesized
documentation/hover.
It also limits our introspection into completion items, which can only
be as precise as the indexed symbols. This change is a prerequisite to
improvements to overload bundling which need to inspect e.g. signature
structure.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48475
llvm-svn: 335360
Summary:
The qualified name can be used to match a completion item to its corresponding
symbol. This can be useful for tools that measure code completion quality.
Qualified names are not precise for identifying symbols; we need to figure out a
better way to identify completion items.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48425
llvm-svn: 335334
Summary:
It's almost always identical to Name, and in fact we never used it (we used name
instead).
The only case where they differ is objc method selectors (foo: vs foo:bar:).
We can live with the latter for both name and filterText, so I've made that
change too.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48375
llvm-svn: 335321
I don't remember why I added it, but it's definitely not needed, since the check
doesn't have any options and the check doesn't have any special relation to the
Google C++ style.
llvm-svn: 335252
Summary: This allows tools to examine symbols that would be collected in a symbol index. For example, a tool that measures index-based completion quality would be interested in references to symbols that are collected in the index.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48418
llvm-svn: 335218
Summary:
Some URI schemes require a hint path to be provided, and workspace root
path seems to be a good fit.
Reviewers: sammccall, malaperle
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48290
llvm-svn: 335035
Summary:
For completion items that would trigger include insertions (i.e. index symbols
that are not #included yet), add a visual indicator "+" before the completion
label. The inserted headers will appear in the completion detail.
Open to suggestions for better visual indicators; "+" was picked because it
seems cleaner than a few other candidates I've tried (*, #, @ ...).
The displayed header would be like a/b/c.h (without quote) or <vector> for system
headers. I didn't add quotation or "#include" because they can take up limited
space and do not provide additional information after users know what the
headers are. I think a header alone should be obvious for users to infer that
this is an include header..
To align indentation, also prepend ' ' to labels of candidates that would not
trigger include insertions (only for completions where index results are
possible).
Vim:
{F6357587}
vscode:
{F6357589}
{F6357591}
Reviewers: sammccall, ilya-biryukov, hokein
Reviewed By: sammccall
Subscribers: MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48163
llvm-svn: 334828
Summary:
Adds a CodeCompleteOption to folds together compatible function/method overloads
into a single item. This feels pretty good (for editors with signatureHelp
support), but has limitations.
This happens in the code completion merge step, so there may be inconsistencies
(e.g. if only one overload made it into the index result list, no folding).
We don't want to bundle together completions that have different side-effects
(include insertion), because we can't constructo a coherent CompletionItem.
This may be confusing for users, as the reason for non-bundling may not
be immediately obvious. (Also, the implementation seems a little fragile)
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47957
llvm-svn: 334822
Summary:
Also move unittest: URI scheme to TestFS so that it can be shared by
different tests.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47935
llvm-svn: 334810
Summary:
This allows dynamic index to have consistent URI schemes with the
static index which can have customized URI schemes, which would make file
proximity scoring based on URIs easier.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47931
llvm-svn: 334809
Summary:
Like the following:
// -------
// =======
// *******
It does not cover all the cases, but those are definitely not very
useful.
Reviewers: sammccall, ioeric, hokein
Reviewed By: sammccall
Subscribers: MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48171
llvm-svn: 334807
Summary:
This is a small code change but vastly reduces noise in code completion results.
The intent of allowing this was to let [sc] ~ "strncpy" and [strcpy] ~ "strncpy"
however the benefits for unsegmented names aren't IMO worth the costs.
Test cases should be representative of the changes here.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47950
llvm-svn: 334712
Summary: These have few signals other than being keywords, so the boost is high.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48083
llvm-svn: 334711
ExprMutationAnalyzer is a generally useful helper that can be used in different clang-tidy checks for checking whether a given expression is (potentially) mutated within a statement (typically the enclosing compound statement.) This is a more general and more powerful/accurate version of isOnlyUsedAsConst, which is used in ForRangeCopyCheck, UnnecessaryCopyInitialization.
Patch by Shuai Wang
llvm-svn: 334604
Summary:
Caching is now handled by ClangdLSPServer and hidden behind the
GlobalCompilationDatabase interface. This simplifies ClangdServer.
This change also removes the SkipCache flag from addDocument,
which is now obsolete.
No behavioral changes are intended, the clangd binary still caches the
compile commands on the first read.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: mgorny, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48068
llvm-svn: 334585
The Semaphore is currently used to limit the number of concurrently
running tasks. Tracing the wait times will allow to find out how much
time is wasted waiting on other operations to complete.
llvm-svn: 334495
Summary: Now we can support property names like "hasADog" correctly.
Reviewers: benhamilton, hokein
Reviewed By: benhamilton
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D48039
llvm-svn: 334448
Summary:
Add support for arrays (and structure that use naked pointers for their iterator, like std::array) in performance-implicit-conversion-in-loop
Reviewers: alexfh
Reviewed By: alexfh
Subscribers: cfe-commits
Patch by Alex Pilkiewicz.
Differential Revision: https://reviews.llvm.org/D47945
llvm-svn: 334400
Summary: Macros are terribly spammy at the moment and this offers some relief.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47936
llvm-svn: 334287
Summary:
This patch improves the check to match the desugared "string" type (so that it
can handle custom-implemented string classes), see the newly-added test.
Reviewers: alexfh
Subscribers: klimek, xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D47704
llvm-svn: 334270
Summary: The comment incorrectly claims that the listed acronyms are all extracted from the linked Apple documentation.
Reviewers: Wizard, benhamilton
Reviewed By: Wizard, benhamilton
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D46922
Contributed by @stephanemoore.
llvm-svn: 334238
This breaks the OpenFlags enumeration into two separate
enumerations: OpenFlags and CreationDisposition. The first
controls the behavior of the API depending on whether or not
the target file already exists, and is not a flags-based
enum. The second controls more flags-like values.
This yields a more easy to understand API, while also allowing
flags to be passed to the openForRead api, where most of the
values didn't make sense before. This also makes the apis more
testable as it becomes easy to enumerate all the configurations
which make sense, so I've added many new tests to exercise all
the different values.
llvm-svn: 334221
Idriss Riouak