Commit Graph

7136 Commits

Author SHA1 Message Date
Nathan Ridge 753b247d71 [clangd] Omit parameter hint if parameter name comment is present
Differential Revision: https://reviews.llvm.org/D100715
2021-04-25 19:20:10 -04:00
Nathan Ridge d941863de2 [clangd] Use HeuristicResolver to produce a better semantic token for name referring to UnresolvedUsingValueDecl
Fixes https://github.com/clangd/clangd/issues/686

Differential Revision: https://reviews.llvm.org/D99056
2021-04-25 16:45:04 -04:00
Nathan Ridge ddfe13e757 [clangd] Produce semantic token for name referring to UnresolvedUsingValueDecl
For now, use the token kind Unknown. We may be able to improve on this
using HeuristicResolver.

Differential Revision: https://reviews.llvm.org/D99052
2021-04-25 16:43:58 -04:00
Stephen Kelly 8d018c79ee Add srcloc output to clang-query
Differential Revision: https://reviews.llvm.org/D93325
2021-04-25 12:12:04 +01:00
Christian Kandeler 81dae18dff [clangd] Allow AST request without range
If no range is given, return the translation unit AST.
This is useful for tooling operations that require e.g. the full path to
a node.

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D101057
2021-04-23 21:35:42 +02:00
Chris Hamilton cae3b70ceb [PR49761] Fix variadic arg handling in matcher
Mishandling of variadic arguments in a function call caused a crash
(runtime assert fail) in bugprone-infinite-loop tidy checker.  Fix
is to limit argument matching to the lesser of the number of variadic
params in the prototype or the number of actual args in the call.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D101108
2021-04-23 12:07:14 -05:00
Kadir Cetinkaya a46bbc14f0
[cland] Dont emit missing newline warnings when building preamble
When building preamble, clangd truncates file contents. This yielded
errnous warnings in some cases.

This patch fixes the issue by turning off no-newline-at-eof warnings whenever
the file has more contents than the preamble.

Fixes https://github.com/clangd/clangd/issues/744.

Differential Revision: https://reviews.llvm.org/D100501
2021-04-23 08:56:32 +02:00
Georgy Komarov 9a930aa5bd
[clang-tidy] Avoid bugprone-macro-parentheses warnings after goto argument
clang-tidy should not generate warnings for the goto argument without
parentheses, because it would be a syntax error.

The only valid case where an argument can be enclosed in parentheses is
"Labels as Values" gcc extension: https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html.
This commit adds support for the label-as-values extension as implemented in clang.

Fixes bugzilla issue 49634.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D99924
2021-04-22 10:14:10 +03:00
Nathan James 6b4e8f82a3
[clangd] Use dirty filesystem when performing cross file tweaks
Cross file tweaks can now use the dirty buffer contents easily when performing cross file effects.
This can be noted on the DefineOutline tweak, now working when the target file is unsaved

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D93978
2021-04-20 17:13:44 +01:00
Pan, Tao 8969762fb1 [clangd][test] Fix build error of FeatureModulesTests
clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp:33:58: error:
could not convert ‘(const char*)""’ from ‘const char*’ to
llvm::StringLiteral’
       llvm::StringLiteral kind() const override { return ""; };

Reviewed By: kadircet

Differential Revision: https://reviews.llvm.org/D100612
2021-04-19 08:56:07 +08:00
Sam McCall ecf93a716c [clangd] Only allow remote index to be enabled from user config.
Differential Revision: https://reviews.llvm.org/D100542
2021-04-15 14:51:23 +02:00
Balázs Kéri bda20282cb [clang-tidy] Add exception flag to bugprone-unhandled-exception-at-new test. 2021-04-14 10:01:05 +02:00
Balázs Kéri 530456caf9 [clang-tidy] Add new check 'bugprone-unhandled-exception-at-new'.
Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D97196
2021-04-14 09:33:11 +02:00
Nathan Ridge cbc9c4ea90 [clangd] Add support for inline parameter hints
Differential Revision: https://reviews.llvm.org/D98748
2021-04-14 02:31:20 -04:00
Roman Lebedev 46b8ea2fff
[clang-tidy] Add check for implicit widening of multiplication result
Overflows are never fun.
In most cases (in most of the code), they are rare,
because usually you e.g. don't have as many elements.

However, it's exceptionally easy to fall into this pitfail
in code that deals with images, because, assuming 4-channel 32-bit FP data,
you need *just* ~269 megapixel image to case an overflow
when computing at least the total byte count.

In [[ https://github.com/darktable-org/darktable | darktable ]], there is a *long*, painful history of dealing with such bugs:
* https://github.com/darktable-org/darktable/pull/7740
* https://github.com/darktable-org/darktable/pull/7419
* eea1989f2c
* 70626dd95b
* https://github.com/darktable-org/darktable/pull/670
* 38c69fb1b2

and yet they clearly keep resurfacing still.

It would be immensely helpful to have a diagnostic for those patterns,
which is what this change proposes.

Currently, i only diagnose the most obvious case, where multiplication
is directly widened with no other expressions inbetween,
(i.e. `long r = (int)a * (int)b` but not even e.g. `long r = ((int)a * (int)b)`)
however that might be worth relaxing later.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D93822
2021-04-13 21:41:22 +03:00
Kadir Cetinkaya b5b2c81055
[clangd] Propagate data in diagnostics
Differential Revision: https://reviews.llvm.org/D98505
2021-04-13 17:45:09 +02:00
Kadir Cetinkaya bce3ac4f22
[clangd] Introduce ASTHooks to FeatureModules
These can be invoked at different stages while building an AST to let
FeatureModules implement features on top of it. The patch also
introduces a sawDiagnostic hook, which can mutate the final clangd::Diag
while reading a clang::Diagnostic.

Differential Revision: https://reviews.llvm.org/D98499
2021-04-13 17:45:09 +02:00
Kadir Cetinkaya bb6d96ced8
[clangd] Enable modules to contribute tweaks.
First patch to enable diagnostic fix generation through modules. The
workflow will look like:
- ASTWorker letting modules know about diagnostics while building AST,
modules can read clang::Diagnostic and mutate clangd::Diagnostic through
that hook.
- Modules can implement and expose tweaks to fix diagnostics or act as
general refactorings.
- Tweak::Selection will contain information about the diagnostic
associated with the codeAction request to enable modules to fail their
diagnostic fixing tweakson prepare if need be.

Differential Revision: https://reviews.llvm.org/D98498
2021-04-13 17:45:08 +02:00
Kadir Cetinkaya ecc6965b23
Revert "Revert "[clangd] Provide a way to disable external index""
This reverts commit c2ad7c2370 while
adding the handling for the new enum value into the switch statement.
2021-04-13 11:24:32 +02:00
Nathan James 27dfcd978e
[clang-tidy] Add <utility> include to misc-uniqueptr-reset-release
This is the only remaining check that creates `std::move` includes but doesn't add a `<utility>` include.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D97683
2021-04-12 23:32:15 +01:00
Sterling Augustine c2ad7c2370 Revert "[clangd] Provide a way to disable external index"
This reverts commit 63bc9e4435.

This breaks llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp:570:11:

with error: enumeration value 'None' not handled in switch [-Werror,-Wswitch]
2021-04-12 14:39:13 -07:00
Richard Smith fc1e146e44 Fix documentation typo. 2021-04-12 11:39:08 -07:00
Alexander Kornienko 8883cb3e40 Fix nits. 2021-04-12 18:46:13 +02:00
Jens Massberg 8a944d82cd [clang-tidy] Add option to ignore macros in readability-function-cognitive-complexity check.
(this was originally part of https://reviews.llvm.org/D96281 and has been split off into its own patch)

If a macro is used within a function, the code inside the macro
doesn't make the code less readable. Instead, for a reader a macro is
more like a function that is called. Thus the code inside a macro
shouldn't increase the complexity of the function in which it is called.
Thus the flag 'IgnoreMacros' is added. If set to 'true' code inside
macros isn't considered during analysis.

This isn't perfect, as now the code of a macro isn't considered at all,
even if it has a high cognitive complexity itself. It might be better if
a macro is considered in the analysis like a function and gets its own
cognitive complexity. Implementing such an analysis seems to be very
complex (if possible at all with the given AST), so we give the user the
option to either ignore macros completely or to let the expanded code
count to the calling function's complexity.

See the code example from vgeof (originally added as note in https://reviews.llvm.org/D96281)

   bool doStuff(myClass* objectPtr){
         if(objectPtr == nullptr){
             LOG_WARNING("empty object");
             return false;
         }
         if(objectPtr->getAttribute() == nullptr){
             LOG_WARNING("empty object");
             return false;
         }
         use(objectPtr->getAttribute());
     }

The LOG_WARNING macro itself might have a high complexity, but it do not make the
the function more complex to understand like e.g. a 'printf'.

By default 'IgnoreMacros' is set to 'false', which is the original behavior of the check.

Reviewed By: lebedev.ri, alexfh

Differential Revision: https://reviews.llvm.org/D98070
2021-04-12 18:46:12 +02:00
Kadir Cetinkaya 63bc9e4435
[clangd] Provide a way to disable external index
Users can reset any external index set by previous fragments by
putting a `None` for the external block, e.g:

```
Index:
  External: None
```

Differential Revision: https://reviews.llvm.org/D100106
2021-04-12 16:43:23 +02:00
Mikael Holmen 2dd22da965 [libtooling][clang-tidy] Fix compiler warnings in testcase [NFC]
Without the fix we get:

06:31:09 In file included from ../../clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp:3:
06:31:09 ../utils/unittest/googletest/include/gtest/gtest.h:1392:11: error: comparison of integers of different signs: 'const int' and 'const unsigned int' [-Werror,-Wsign-compare]
06:31:09   if (lhs == rhs) {
06:31:09       ~~~ ^  ~~~
06:31:09 ../utils/unittest/googletest/include/gtest/gtest.h:1421:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<int, unsigned int>' requested here
06:31:09     return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
06:31:09            ^
06:31:09 ../../clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp:60:3: note: in instantiation of function template specialization 'testing::internal::EqHelper<false>::Compare<int, unsigned int>' requested here
06:31:09   EXPECT_EQ(4, Errors[0].Message.FileOffset);
06:31:09   ^
06:31:09 ../utils/unittest/googletest/include/gtest/gtest.h:1924:63: note: expanded from macro 'EXPECT_EQ'
06:31:09                       EqHelper<GTEST_IS_NULL_LITERAL_(val1)>::Compare, \
06:31:09                                                               ^
06:31:09 ../utils/unittest/googletest/include/gtest/gtest.h:1392:11: error: comparison of integers of different signs: 'const int' and 'const unsigned long' [-Werror,-Wsign-compare]
06:31:09   if (lhs == rhs) {
06:31:09       ~~~ ^  ~~~
06:31:09 ../utils/unittest/googletest/include/gtest/gtest.h:1421:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<int, unsigned long>' requested here
06:31:09     return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
06:31:09            ^
06:31:09 ../../clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp:64:3: note: in instantiation of function template specialization 'testing::internal::EqHelper<false>::Compare<int, unsigned long>' requested here
06:31:09   EXPECT_EQ(1, Errors[0].Message.Ranges.size());
06:31:09   ^
06:31:09 ../utils/unittest/googletest/include/gtest/gtest.h:1924:63: note: expanded from macro 'EXPECT_EQ'
06:31:09                       EqHelper<GTEST_IS_NULL_LITERAL_(val1)>::Compare, \
06:31:09                                                               ^
06:31:09 2 errors generated.
2021-04-12 08:26:46 +02:00
Whisperity 8fa3975247 [libtooling][clang-tidy] Fix off-by-one rendering issue with SourceRanges
There was an off-by-one issue with calculating the *exact* end location
of token ranges (as given by SomeDecl->getSourceRange()) which resulted in:

  xxx(something)
      ^~~~~~~~   // Note the missing ~ under the last character.

In addition, a test is added to keep the behaviour in check in the future.

This patch hotfixes commit 3b677b81ce.
2021-04-10 18:52:55 +02:00
Whisperity 3b677b81ce [libtooling][clang-tidy] Fix diagnostics not highlighting fed SourceRanges
Fixes bug http://bugs.llvm.org/show_bug.cgi?id=49000.

This patch allows Clang-Tidy checks to do

    diag(X->getLocation(), "text") << Y->getSourceRange();

and get the highlight of `Y` as expected:

    warning: text [blah-blah]
        xxx(something)
        ^   ~~~~~~~~~

Reviewed-By: aaron.ballman, njames93

Differential Revision: http://reviews.llvm.org/D98635
2021-04-10 16:43:44 +02:00
Kadir Cetinkaya b9b708eef8
[clangd] Log a message when gRPC support is off, but remote-index is configured
Before this change clangd would emit a diagnostic whenever remote-index
was configured but binary didn't have grpc support.

This can be annoying when projects are configuring remote-index through their
configs but developers have a clangd binary without the support.

Differential Revision: https://reviews.llvm.org/D100103
2021-04-09 15:52:51 +02:00
Adam Czachorowski 3b4936ba29 [clangd] Add --check-lines to restrict --check to specific lines
This will allow us to add code completion, which is too expensive at
every token, to --check too.

Differential Revision: https://reviews.llvm.org/D98970
2021-04-09 13:47:20 +02:00
crr0004 43637c0dfe
Fix crash when an invalid URI is parsed and error handling is attempted
When you pass in a payload with an invalid URI in a build with assertions enabled, it will crash.
Consuming the error from the failed URI parse prevents the error.

The crash is caused by the [llvm::expected](https://llvm.org/doxygen/classllvm_1_1Expected.html) having protection around trying to deconstruct without consuming the error first.

Reviewed By: kadircet

Differential Revision: https://reviews.llvm.org/D99872
2021-04-07 12:32:33 +02:00
Vince Bridgers c060945b23 [docs] Update documentation for bugprone-misplaced-widening-cast
The default setting for CheckImplicitCasts was changed in
https://reviews.llvm.org/D32164 but the documentation was not updated.
This simple change just syncs the documentation with the behavior of
that checker.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D99991
2021-04-06 16:18:50 -05:00
Felix Berger ddebed8e97 [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".
This allows users to be more precise and exclude a type in a specific namespace
from triggering the check instead of excluding all types with the same
unqualified name.

This change should not interfere with correctly configured clang-tidy setups
since an AllowedType with "::" would never match.

Differential Revision: https://reviews.llvm.org/D98738

Reviewed-by: ymandel, hokein
2021-04-06 15:41:35 -04:00
oToToT 14a7296c01
[clang][clangd] Avoid inconsistent target creation
As proposed in D97109, I tried to make target creation consistent in `clang` and `clangd` by replacing the original procedure with a single function introduced in D97493.

This also helps `clangd` works with CUDA, OpenMP, etc.

Reviewed By: kadircet

Differential Revision: https://reviews.llvm.org/D98128
2021-04-06 23:23:34 +08:00
Abhina Sreeskantharajan 82b3e28e83 [SystemZ][z/OS][Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text
Problem:
On SystemZ we need to open text files in text mode. On Windows, files opened in text mode adds a CRLF '\r\n' which may not be desirable.

Solution:
This patch adds two new flags

  - OF_CRLF which indicates that CRLF translation is used.
  - OF_TextWithCRLF = OF_Text | OF_CRLF indicates that the file is text and uses CRLF translation.

Developers should now use either the OF_Text or OF_TextWithCRLF for text files and OF_None for binary files. If the developer doesn't want carriage returns on Windows, they should use OF_Text, if they do want carriage returns on Windows, they should use OF_TextWithCRLF.

So this is the behaviour per platform with my patch:

z/OS:
OF_None: open in binary mode
OF_Text : open in text mode
OF_TextWithCRLF: open in text mode

Windows:
OF_None: open file with no carriage return
OF_Text: open file with no carriage return
OF_TextWithCRLF: open file with carriage return

The Major change is in llvm/lib/Support/Windows/Path.inc to only set text mode if the OF_CRLF is set.
```
  if (Flags & OF_CRLF)
    CrtOpenFlags |= _O_TEXT;
```

These following files are the ones that still use OF_Text which I left unchanged. I modified all these except raw_ostream.cpp in recent patches so I know these were previously in Binary mode on Windows.
./llvm/lib/Support/raw_ostream.cpp
./llvm/lib/TableGen/Main.cpp
./llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
./llvm/unittests/Support/Path.cpp
./clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
./clang/lib/Frontend/CompilerInstance.cpp
./clang/lib/Driver/Driver.cpp
./clang/lib/Driver/ToolChains/Clang.cpp

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D99426
2021-04-06 07:23:31 -04:00
Kadir Cetinkaya 6d2fb3cefb
[clangd] Perform merging for stale symbols in MergeIndex
Clangd drops symbols from static index whenever the dynamic index is
authoritative for the file. This results in regressions when static and
dynamic index contains different set of information, e.g.
IncludeHeaders.

After this patch, we'll choose to merge symbols from static index with
dynamic one rather than just dropping. This implies correctness problems
when the definition/documentation of the symbol is deleted. But seems
like it is worth having in more cases.

We still drop symbols if dynamic index owns the file and didn't report
the symbol, which means symbol is deleted.

Differential Revision: https://reviews.llvm.org/D98538
2021-03-30 11:09:51 +02:00
Stephen Kelly ea2225a10b [clang-tidy] Simplify readability checks to not need ignoring* matchers
Differential Revision: https://reviews.llvm.org/D98296
2021-03-28 11:25:41 +01:00
Utkarsh Saxena aa979084df [clang][Syntax] Optimize expandedTokens for token ranges.
`expandedTokens(SourceRange)` used to do a binary search to get the
expanded tokens belonging to a source range. Each binary search uses
`isBeforeInTranslationUnit` to order two source locations. This is
inherently very slow.
By profiling clangd we found out that users like clangd::SelectionTree
spend 95% of time in `isBeforeInTranslationUnit`. Also it is worth
noting that users of `expandedTokens(SourceRange)` majorly use ranges
provided by AST to query this funciton. The ranges provided by AST are
token ranges (starting at the beginning of a token and ending at the
beginning of another token).

Therefore we can avoid the binary search in majority of the cases by
maintaining an index of ExpandedToken by their SourceLocations. We still
do binary search for ranges which are not token ranges but such
instances are quite low.

Performance:
`~/build/bin/clangd --check=clang/lib/Serialization/ASTReader.cpp`
Before: Took 2:10s to complete.
Now: Took 1:13s to complete.

Differential Revision: https://reviews.llvm.org/D99086
2021-03-25 18:54:15 +01:00
Kadir Cetinkaya 7f5abb6373
[clangd] Fix a use-after-free
Clangd was storing reference to a possibly-dead string in compiled
config. This patch fixes the issue by copying suppression strings from
fragments into compiled Config.

Fixes https://github.com/clangd/clangd/issues/724.

Differential Revision: https://reviews.llvm.org/D99326
2021-03-25 18:26:17 +01:00
Nathan James 02d7ef3181
[clang-tidy] Fix mpi checks when running multiple TUs per clang-tidy process
Both the mpi-type-mismatch and mpi-buffer-deref check make use of a static MPIFunctionClassifier object.
This causes issue as the classifier is initialized with the first ASTContext that produces a match.
If the check is enabled on multiple translation units in a single clang-tidy process, this classifier won't be reinitialized for each TU. I'm not an expert in the MPIFunctionClassifier but I'd imagine this is a source of UB.
It is suspected that this bug may result in the crash caused here: https://bugs.llvm.org/show_bug.cgi?id=48985. However even if not the case, this should still be addressed.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D98275
2021-03-25 14:38:37 +00:00
Alexander Lanin 0becc4d721 fix readability-braces-around-statements Stmt type dependency
Replaces Token based approach to identify EndLoc of Stmt with AST traversal.
This also improves handling of macros.

Fixes Bugs 22785, 25970 and 35754.
2021-03-25 09:44:41 -04:00
Dmitry Polukhin 338d162755 [clang-tidy] Ignore all spaces in the list of checks
This diff patch fixes issue with new line character after check name and before comma. Also ignores all other types of spaces like TAB.

Test Plan: ninja check-clang-tools

Differential Revision: https://reviews.llvm.org/D99180
2021-03-24 06:43:13 -07:00
Frank Derry Wanye 5a87f81fe9 new altera unroll loops check
This lint check is a part of the FLOCL (FPGA Linters for OpenCL)
project out of the Synergy Lab at Virginia Tech.

FLOCL is a set of lint checks aimed at FPGA developers who write code
in OpenCL.

The altera unroll loops check finds inner loops that have not been
unrolled, as well as fully-unrolled loops that should be partially
unrolled due to unknown loop bounds or a large number of loop
iterations.

Based on the Altera SDK for OpenCL: Best Practices Guide.
2021-03-22 13:09:53 -04:00
Kadir Cetinkaya f71404c37c
[clangd] Replace usages of dummy with more descriptive words
Dummy is a word with inappropriate associations. This patch updates the
references to it in clangd code base with more precise ones.

The only user-visible change is the default variable name used when extracting a
variable. It will be named as `placeholder` from now on.

Differential Revision: https://reviews.llvm.org/D99065
2021-03-22 12:49:24 +01:00
serge-sans-paille f51ab18716 Make clangd CompletionModel usable even with non-standard (but supported) layout
llvm supports specifying a non-standard layout where each project lies in its
own place. Do not assume a fixed layout and use the appropriate cmake variable
instead.

Differential Revision: https://reviews.llvm.org/D96787
2021-03-22 10:05:25 +01:00
Nathan Ridge 2e58226d8d [clangd] Fix linker error when linking clang-index-server with shared libraries
Fixes https://github.com/clangd/clangd/issues/723

Differential Revision: https://reviews.llvm.org/D99049
2021-03-22 02:38:58 -04:00
Nathan James 4dd92d61db
[clang-tidy] Fix bugprone-terminating-continue when continue appears inside a switch
Don't emit a warning if the `continue` appears in a switch context as changing it to `break` will break out of the switch rather than a do loop containing the switch.
Fixes https://llvm.org/PR49492.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D98338
2021-03-20 10:59:37 +00:00
Vassil Vassilev 0cb7e7ca0c Make iteration over the DeclContext::lookup_result safe.
The idiom:
```
DeclContext::lookup_result R = DeclContext::lookup(Name);
for (auto *D : R) {...}
```

is not safe when in the loop body we trigger deserialization from an AST file.
The deserialization can insert new declarations in the StoredDeclsList whose
underlying type is a vector. When the vector decides to reallocate its storage
the pointer we hold becomes invalid.

This patch replaces a SmallVector with an singly-linked list. The current
approach stores a SmallVector<NamedDecl*, 4> which is around 8 pointers.
The linked list is 3, 5, or 7. We do better in terms of memory usage for small
cases (and worse in terms of locality -- the linked list entries won't be near
each other, but will be near their corresponding declarations, and we were going
to fetch those memory pages anyway). For larger cases: the vector uses a
doubling strategy for reallocation, so will generally be between half-full and
full. Let's say it's 75% full on average, so there's N * 4/3 + 4 pointers' worth
of space allocated currently and will be 2N pointers with the linked list. So we
break even when there are N=6 entries and slightly lose in terms of memory usage
after that. We suspect that's still a win on average.

Thanks to @rsmith!

Differential revision: https://reviews.llvm.org/D91524
2021-03-17 08:59:04 +00:00
Nathan James 9a5af541ee
[clang-tidy] Remove readability-deleted-default
The deprecation notice was cherrypicked to the release branch in f8b3298924 so its safe to remove this for the 13.X release cycle.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D98612
2021-03-16 14:03:33 +00:00
Kirill Bobyrev 524fe51509
[clangd] Add basic monitoring info request for remote index server
This allows requesting information about the server uptime and start time. This is the first patch in a series of monitoring changes, hence it's not immediately useful. Next step is propagating the index freshness information and then probably loading metadata into the index server.

The way to test new behaviour through command line:

```
$ grpc_cli call localhost:50051 Monitor/MonitoringInfo ''
connecting to localhost:50051
uptime_seconds: 42
index_age_seconds: 609568
Rpc succeeded with OK status
```

Reviewed By: kadircet

Differential Revision: https://reviews.llvm.org/D98246
2021-03-16 13:37:58 +01:00
Kadir Cetinkaya 2772c3a975
[clangd] Introduce pullDiags endpoint
Implement initial support for pull-based diagnostics in ClangdServer.
This is planned for LSP 3.17, and initial proposal is in
d15eb0671e/protocol/src/common/proposed.diagnostic.ts (L111).

We chose to serve the requests only when clangd has a fresh preamble
available. In case of a stale preamble we just drop the request on the
floor.

This patch doesn't plumb this to LSP layer yet, as pullDiags is still a
proposal with only an implementation in vscode.

Differential Revision: https://reviews.llvm.org/D98623
2021-03-16 12:52:15 +01:00
Sam McCall 128ce70eef [CodeCompletion] Avoid spurious signature help for init-list args
Somewhat surprisingly, signature help is emitted as a side-effect of
computing the expected type of a function argument.
The reason is that both actions require enumerating the possible
function signatures and running partial overload resolution, and doing
this twice would be wasteful and complicated.

Change #1: document this, it's subtle :-)

However, sometimes we need to compute the expected type without having
reached the code completion cursor yet - in particular to allow
completion of designators.
eb4ab3358c did this but introduced a
regression - it emits signature help in the wrong location as a side-effect.

Change #2: only emit signature help if the code completion cursor was reached.

Currently there is PP.isCodeCompletionReached(), but we can't use it
because it's set *after* running code completion.
It'd be nice to set this implicitly when the completion token is lexed,
but ConsumeCodeCompletionToken() makes this complicated.

Change #3: call cutOffParsing() *first* when seeing a completion token.

After this, the fact that the Sema::Produce*SignatureHelp() functions
are even more confusing, as they only sometimes do that.
I don't want to rename them in this patch as it's another large
mechanical change, but we should soon.

Change #4: prepare to rename ProduceSignatureHelp() to GuessArgumentType() etc.

Differential Revision: https://reviews.llvm.org/D98488
2021-03-16 12:46:40 +01:00
Sam McCall ca13f5595a [clangd] Add `limit` extension on completion and workspace-symbols
This overrides the --limit-results command-line flag, and is not constrained
by it.
See https://github.com/clangd/clangd/issues/707

Differential Revision: https://reviews.llvm.org/D97801
2021-03-16 12:28:01 +01:00
Sam McCall 3b99731c4e [clangd] Turn off implicit cancellation based on client capabilities
Capability is in upcoming 3.17: https://microsoft.github.io/language-server-protocol/specifications/specification-3-17/

(This is also useful for C++ embedders)

Differential Revision: https://reviews.llvm.org/D98414
2021-03-16 12:27:40 +01:00
Sam McCall 43d0b1c9c1 [clangd] Reject renames to non-identifier characters
Differential Revision: https://reviews.llvm.org/D98424
2021-03-16 12:18:29 +01:00
Kirill Bobyrev 9bcf0eff99
[clangd] Optionally add reflection for clangd-index-server
This was originally landed without the optional part and reverted later:

8080ea4c4b

Reviewed By: kadircet

Differential Revision: https://reviews.llvm.org/D98404
2021-03-15 21:07:25 +01:00
Dmitry Polukhin da55af7f1d [clang-tidy] Enable modernize-concat-nested-namespaces also on headers
For some reason the initial implementation of the check had an explicit check
for the main file to avoid being applied in headers. This diff removes this
check and add a test for the check on a header.

Similar approach was proposed in D61989 but review there got stuck.

Test Plan: added new test case

Differential Revision: https://reviews.llvm.org/D97563
2021-03-15 07:32:45 -07:00
Nathan James 0333dde923
[clang-tidy] Fix readability-identifer-naming duplicating prefix or suffix for replacements.
If a identifier has a correct prefix/suffix but a bad case, the fix won't strip them when computing the correct case, leading to duplication when the are added back.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D98521
2021-03-15 14:20:48 +00:00
Kadir Cetinkaya dc9c09632f
[clangd] Make ProjectAwareIndex optionally sync
Depends on D98029.

Differential Revision: https://reviews.llvm.org/D98165
2021-03-11 20:30:35 +01:00
Kadir Cetinkaya ac292dafa7
[clangd] Add config block for Completion and option for AllScopes
Depends on D98029

Differential Revision: https://reviews.llvm.org/D98037
2021-03-11 20:30:35 +01:00
Kadir Cetinkaya cec62ae28a
[clangd] Fix buildbots without grpc enabled 2021-03-11 13:46:52 +01:00
Kadir Cetinkaya 4f1bbc0b84
[clangd] Introduce a CommandLineConfigProvider
This enables unifying command line flags with config options in clangd
internals. This patch changes behaviour in 2 places:
- BackgroundIndex was previously disabled when -remote-index was
provided. After this patch, it will be enabled but all files will have
bkgindex policy set to Skip.
- -index-file was loaded at startup (at least load was initiated), now
the load will happen through ProjectAwareIndex with first index query.

Unfortunately this doesn't simplify any options initially, as
- CompileCommandsDir is also used by clangd --check workflow, which
doesn't use configs.
- EnableBackgroundIndex option controls whether the component will be
created at all, which implies creation of extra threads registering a
listener for compilation database discoveries.

Differential Revision: https://reviews.llvm.org/D98029
2021-03-11 13:35:05 +01:00
Kadir Cetinkaya b1a5df174e
[clangd] Drop explicit specifier on define out-of-line
Explicit specifier can only be mentioned on the in-line declaration of a
constructor, so don't carry it over to the definition.

Differential Revision: https://reviews.llvm.org/D98164
2021-03-11 13:27:24 +01:00
Sam McCall b8c58374f6 [clangd] Group filename calculations in SymbolCollector, and cache mroe.
Also give CanonicalIncludes a less powerful interface (canonicalizes
symbols vs headers separately) so we can cache its results better.

Prior to this:
 - path->uri conversions were not consistently cached, this is
   particularly cheap when we start from a FileEntry* (which we often can)
 - only a small fraction of header-to-include calculation was cached

This is a significant speedup at least for dynamic indexing of preambles.
On my machine, opening XRefs.cpp:

```
PreambleCallback 1.208 -> 1.019 (-15.7%)
BuildPreamble    5.538 -> 5.214 (-5.8%)
```

Differential Revision: https://reviews.llvm.org/D98371
2021-03-11 12:59:26 +01:00
Nathan James 7044f1d875
[clangd] Use Dirty Filesystem for cross file rename.
Refactor cross file rename to use a Filesystem instead of a function for getting buffer contents of open files.

Depends on D94554

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D95043
2021-03-10 13:41:29 +00:00
Kadir Cetinkaya 99b01cf28d
Revert "[clangd] Enable reflection for clangd-index-server"
This reverts commit 8080ea4c4b.

As discussed offline we should only do that for debug builds.
2021-03-10 14:12:37 +01:00
Jinzheng Tu 481079e284 [NFC] Unify FIME with FIXME in comments
There are 5 occurrences FIME and 15333 FIXME. All of them should be FIXME.

Reviewed By: alexfh

Differential Revision: https://reviews.llvm.org/D98321
2021-03-10 14:00:51 +01:00
Kirill Bobyrev 8080ea4c4b [clangd] Enable reflection for clangd-index-server
This allows sending requests through CLI and more debugging
opportunities. Example:

```bash
$ grpc_cli ls localhost:50051
clang.clangd.remote.v1.SymbolIndex
grpc.reflection.v1alpha.ServerReflection
grpc.health.v1.Health
```
2021-03-10 09:07:39 +01:00
Fangrui Song cd6d1799ad [clangd] Treat __GCC_HAVE_DWARF2_CFI_ASM the same as isWrittenInBuiltinFile macros 2021-03-09 23:11:54 -08:00
Fangrui Song 46bf25a7c5 [test] Update tests 2021-03-09 22:32:28 -08:00
Nathan James c92d2ea59e
[clangd][NFC] Use std::string::replace in SourceCode:applyChange.
Just looks nicer and easier to read.

Reviewed By: kadircet

Differential Revision: https://reviews.llvm.org/D98274
2021-03-09 21:39:23 +00:00
Nathan James 574663f9d5
[clangd][NFC] Silence some buildbot warnings after 0250b053
https://reviews.llvm.org/D94554 introduced code which wont compile with some build flags due to a field having the same identifier as a type.

clang-tools-extra/clangd/DraftStore.h:55:11: error: declaration of ‘clang::clangd::DraftStore::Draft clang::clangd::DraftStore::DraftAndTime::Draft’ changes meaning of ‘Draft’ [-fpermissive]
   55 |     Draft Draft;
      |           ^~~~~
clang-tools-extra/clangd/DraftStore.h:30:10: note: ‘Draft’ declared here as ‘struct clang::clangd::DraftStore::Draft’
   30 |   struct Draft {
         |          ^~~~~
2021-03-09 14:55:55 +00:00
Nathan James 0250b053b5
[clangd] Add a Filesystem that overlays Dirty files.
Create a `ThreadsafeFS` in the `DraftStore` that overlays the dirty file contents over another `ThreadsafeFS`.
This provides a nice thread-safe interface for using dirty file contents throughout the codebase, for example cross file refactoring.
Creating a Filesystem view will overlay a snapshot of the current contents, so if the draft store is updated while the view is being used, it will contain stale contents.

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D94554
2021-03-09 14:35:21 +00:00
Kadir Cetinkaya d1531b08c3
[clangd] Move logging out of LSPTest base class into a separate one.
This was causing TSan failures due to a race on vptr during destruction,
see
https://lab.llvm.org/buildbot/#/builders/131/builds/6579/steps/6/logs/FAIL__Clangd_Unit_Tests__LSPTest_FeatureModulesThr.

The story is, during the execution of a destructor all the virtual
dispatches should resolve to implementations in the class being
destroyed, not the derived ones. And LSPTests will log some stuff during
destruction (we send shutdown/exit requests, which are logged), which is
a virtual dispatch when LSPTest is derived from clang::clangd::Logger.

It is a benign race in our case, as gtests that derive from LSPTest
doesn't override the `log` method. But still during destruction, we
might try to update vtable ptr (due to being done with destruction of
test and starting destruction of LSPTest) and read from it to dispatch a
log message at the same time.

This patch fixes that race by moving `log` out of the vtable of
`LSPTest`.

Differential Revision: https://reviews.llvm.org/D98241
2021-03-09 11:57:05 +01:00
Aleksandr Platonov c4efd04f18 [clangd] Use URIs instead of paths in the index file list
Without this patch the file list of the preamble index contains URIs, but other indexes file lists contain file paths.
This makes `indexedFiles()` always returns `IndexContents::None` for the preamble index, because current implementation expects file paths inside the file list of the index.

This patch fixes this problem and also helps to avoid a lot of URI to path conversions during indexes merge.

Reviewed By: kadircet

Differential Revision: https://reviews.llvm.org/D97535
2021-03-06 10:47:05 +03:00
Nathan James 3bca86170d [clang-tidy][NFC] Remove unsupported language version checks from vector 2021-03-05 15:35:34 +00:00
Martin Boehme e67d91faec [clang-tidy] Use-after-move: Ignore moves inside a try_emplace.
We have no way to reason about the bool returned by try_emplace, so we
simply ignore any std::move()s that happen in a try_emplace argument.
A lot of the time in this situation, the code will be checking the
bool and doing something else if it turns out the value wasn't moved
into the map, and this has been causing false positives so far.

I don't currently have any intentions of handling "maybe move" functions
more generally.

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D98034
2021-03-05 15:05:09 +01:00
Sam McCall a60d06d8b7 [clangd] Rename Module -> FeatureModule to avoid confusion. NFC
As pointed out in D96244, "Module" is already pretty overloaded to refer
to clang and llvm modules. (And clangd deals directly with the former).

FeatureModule is a bit of a mouthful but it's pretty self-descriptive.
I think it might be better than "Component" which doesn't really capture
the "common interface" aspect - it's IMO confusing to refer to
"components" but exclude CDB for example.

Differential Revision: https://reviews.llvm.org/D97950
2021-03-05 10:04:00 +01:00
Yang Fan 889da99523
[clang][AST] Fix Wreturn-type gcc warning (NFC)
GCC warning:
```
/llvm-project/clang-tools-extra/clangd/SemanticHighlighting.cpp: In function ‘bool clang::clangd::{anonymous}::canHighlightName(clang::DeclarationName)’:
/llvm-project/clang-tools-extra/clangd/SemanticHighlighting.cpp:64:1: warning: control reaches end of non-void function [-Wreturn-type]
   64 | }
      | ^
```
2021-03-05 11:24:55 +08:00
Jens Massberg bff7faea20 [clang-tidy] Add options to describe individual core increments to readability-function-cognitive-complexity check.
Often you are only interested in the overall cognitive complexity of a
function and not every individual increment. Thus the flag
'DescribeBasicIncrements' is added. If it is set to 'true', each increment
is flagged. Otherwise, only the complexity of function with complexity
of at least the threshold are flagged.

By default 'DescribeBasisIncrements' is set to 'true', which is the original behavior of the check.

Added a new test for different flag combinations.

(The option to ignore macros which was original part of this patch will be added in another path)

Reviewed By: lebedev.ri

Differential Revision: https://reviews.llvm.org/D96281
2021-03-04 21:02:27 +01:00
Nathan James a85eb11129
[clang-tidy] Extend LoopConvert on array with `!=` comparison
Enables transforming loops of the form:
```
for (int i = 0; I != container.size(); ++I) { container[I]...; }
for (int i = 0; I != N; ++I) { FixedArrSizeN[I]...; }
```

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D97940
2021-03-04 18:58:59 +00:00
martinboehme 3ea0e119b9 [clang-tidy][NFC] Update docs for bugprone-use-after-move
- Create a separate section on silencing erroneous warnings and add more material to it
- Add note that the check is flow-sensitive but not path-sensitive
2021-03-04 13:22:19 +01:00
Kadir Cetinkaya 1d7b328198
[clangd] Introduce client state invalidation
Clangd can invalidate client state of features like semantic higlighting
without client explicitly triggering, for example after a preamble build
caused by an onSave notification on a different file.

This patch introduces a mechanism to let client know of such actions,
and also calls the workspace/semanticTokens/refresh request to
demonstrate the situation after each preamble build.

Fixes https://github.com/clangd/clangd/issues/699.

Differential Revision: https://reviews.llvm.org/D97548
2021-03-04 11:15:10 +01:00
Sam McCall 7d2fba8ddb [clangd] ObjC fixes for semantic highlighting and xref highlights
- highlight references to protocols in class/protocol/extension decls
- support multi-token selector highlights in semantic + xref highlights
  (method calls and declarations only)
- In `@interface I(C)`, I now references the interface and C the category
- highlight uses of interfaces as types
- added semantic highlightings of protocol names (as "interface") and
  category names (as "namespace").
  These are both standard kinds, maybe "extension" will be standardized...
- highlight `auto` as "class" when it resolves to an ObjC pointer
- don't highlight `self` as a variable even though the AST models it as one

Not fixed: uses of protocols in type names (needs some refactoring of
unrelated code first)

Differential Revision: https://reviews.llvm.org/D97617
2021-03-03 20:16:08 +01:00
Nathan James 19aefd2d5d
[clang-tidy] Deprecate readability-deleted-default check
... For removal in next release cycle.
The clang warning that does the same thing is enabled by default and typically emits better diagnostics making this check surplus to requirements.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D97491
2021-03-03 16:01:13 +00:00
Nathan James e7080aa225
[clang-query] Fix help text after D91918
After rG5e1801813d93210acae84ff3c68a01512c2df9bc The help command still lists IgnoreImplicitCastsAndParentheses as a valid option.

Reviewed By: aaron.ballman, rsmith

Differential Revision: https://reviews.llvm.org/D97806
2021-03-03 15:57:49 +00:00
Kadir Cetinkaya 188373fb46
[clangd] Make WorkspaceSymbols request work with empty queries
Clangd uses codecompletion limit as the limit for workspacesymbols, so
in theory this should only be an order of magnitude slower than a
codecompletion request with empty identifier (as code completion limits
the available symbols).

This is also what LSP suggests "Clients may send an empty string here to request all symbols.".
Clangd doesn't really fulfill the "all" part of that statement, but we
never do unless user set the index query limit to zero explicitly.

Differential Revision: https://reviews.llvm.org/D97773
2021-03-03 15:41:39 +01:00
Sam McCall 1a4990a4f7 [clangd] Fix uninit member 2021-03-03 11:45:16 +01:00
Nathan James 1a91b8232a
[clang-tidy][NFC] Use equalsBoundNode matchers to simplify LoopConvertCheck
Make use of the `equalsBoundNode` matcher to ensure Init, Conditon and Increment variables all refer to the same variable during matching.

Reviewed By: steveire

Differential Revision: https://reviews.llvm.org/D97639
2021-03-03 02:51:34 +00:00
Felix Berger a189b3b9e8 [clang-tidy] performance-for-range-copy: Don't trigger on implicit type conversions.
This disables the check for false positive cases where implicit type conversion
through either an implicit single argument constructor or a member conversion
operator is triggered when constructing the loop variable.

Fix the test cases that meant to cover these cases.

Differential Revision: https://reviews.llvm.org/D97577

Reviewed-by: hokein
2021-03-02 20:02:48 -05:00
Sam McCall bca3e24139 [clangd] Move DraftStore from ClangdLSPServer into ClangdServer.
ClangdServer already gets notified of every change, so it makes sense for it to
be the source of truth.
This is a step towards having ClangdServer expose a FS that includes dirty
buffers: D94554

Related changes:
 - version is now optional for ClangdServer, to preserve our existing fuzziness
   in this area (missing version ==> autoincrement)
 - ClangdServer::format{File,Range} are now more regular ClangdServer functions
   that don't need the code passed in. While here, combine into one function.
 - incremental content update logic is moved from DraftStore to
   ClangdLSPServer, with most of the implementation in SourceCode.cpp.
   DraftStore is now fairly trivial, and will probably ultimately be
   *replaced* by the dirty FS stuff.

Differential Revision: https://reviews.llvm.org/D97738
2021-03-02 22:58:50 +01:00
Nathan James 00c7d6699a
[cte][NFC] Remove all references to stdlib stream headers.
Inclusion of iostream is frobidden and using other stream classes from standard library is discouraged as per https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D97771
2021-03-02 21:57:16 +00:00
Stephen Kelly 7b6fc9a105 [clang-tidy] Simplify unused RAII check
Fix handling of default construction where the constructor has a default arg.

Differential Revision: https://reviews.llvm.org/D97142
2021-03-02 21:33:34 +00:00
Utkarsh Saxena 890190a61d Revert "Revert "[clangd] Use ML Code completion ranking as default.""
The ASAN failure was fixed by
bf935a034b.

This reverts commit 7f086d74c3.
2021-03-02 18:03:52 +01:00
Sam McCall 91679c95bb [clangd] Include macro expansions in documentSymbol hierarchy
Browsing macro-generated symbols is confusing.
On the one hand, it seems very *useful* to be able to see the summary of
symbols that were generated.
On the other hand, some macros spew a lot of confusing symbols into the
namespace and when used repeatedly (ABSL_FLAG) can create a lot of spam
that's hard to navigate.

Design constraints:
 - the macro expansion tree need not align with the AST, though it often
   does in practice.
   We address this by defining the nesting based on the *primary*
   location of decls, rather than their ranges.
 - DocumentSymbol.children[*].range should nest within DocumentSymbol.range
   (This constraint is not in LSP "breadcrumbs" breaks without it)
   We adjust macro ranges so they cover their "children", rather than
   just the macro expansion
 - LSP does not have a "macro expansion" symbolkind, nor does it allow a
   symbol to have no kind. I've arbitrarily picked "null" as this is
   unlikely to conflict with anything useful.

This patch makes all macros and children visible for simplicity+consistency,
though in some cases it may be better to elide the macro node.
We may consider adding heuristics for this in future (e.g. when it expands
to one decl only?) but it doesn't seem clear-cut to me.

Differential Revision: https://reviews.llvm.org/D97615
2021-03-02 17:52:24 +01:00
Sam McCall 289fee4ab7 [clangd] Show hex value of numeric constants
Don't show negative numbers
Don't show numbers <10 (hex is the same as decimal)
Show numeric enum values in hex too

Differential Revision: https://reviews.llvm.org/D97226
2021-03-02 16:33:02 +01:00
Utkarsh Saxena bf935a034b [clangd] Make categorical features 64 bit in DecisionForest Model.
CodeCompletionContext::Kind has 36 Kinds. The completion model used to
support categorical features of 32 cardinality.
Due to this clangd tests were failing asan tests due to overflow.

This patch makes the completion model support 64 cardinality of
categorical features by storing ENUM Features as uint64_t instead of
uint32_t.

Verified that this fixes the asan failures.

Latency: 6.7ms (old) VS 6.8ms (new) per 1000 predictions.

Differential Revision: https://reviews.llvm.org/D97770
2021-03-02 16:22:30 +01:00
Sam McCall 7556abf821 [clangd] findExplicitReferences impl filters nulls centrally. NFC 2021-03-02 15:55:03 +01:00
Utkarsh Saxena 7f086d74c3 Revert "[clangd] Use ML Code completion ranking as default."
CodeCompletionContext::Kind has 36 Kinds. The completion model currently
only handles categorical features of 32 cardinality.
Changing the datatype to uint64_t will solve the problem.

This reverts commit 438b5bb05a.
2021-03-02 15:04:23 +01:00
Utkarsh Saxena bad8e577f9
Fix DecisionForestBenchmark.cpp compile errors
clang-tools-extra/clangd/benchmarks/CompletionModel/DecisionForestBenchmark.cpp fails to compile since `"CompletionModel.h"` is auto-generated from clang-tools-extra/clangd/quality/model/features.json, which was changed in https://reviews.llvm.org/D94697 to remove `setFilterLength` and `setIsForbidden`, rename `setFileProximityDistance` and `setSymbolScopeDistance`, and add `setNumNameInContext` and `setFractionNameInContext`.  This patch removes calls to the two removed functions, updates calls to the two renamed functions, and adds calls to the two new functions. (`20` is an arbitrary choice for the `setNumNameInContext` argument.) It also changes the `FlipCoin` argument from float to double to silence lossy conversion warnings.

Note: I don't use this tool but encountered the build errors and took a shot at fixing them. Please holler if there's another recommended solution. Thanks!

Reviewed By: usaxena95

Differential Revision: https://reviews.llvm.org/D97620
2021-03-02 10:27:46 +01:00