Commit Graph

7239 Commits

Author SHA1 Message Date
Sam McCall e9274af718 Revert "[clangd] Avoid range-loop init-list lifetime subtleties."
This reverts commit 253b8145de.

This doesn't actually fix anything - I should stop guessing.
See https://github.com/clangd/clangd/issues/800 for update
2021-07-26 11:38:47 +02:00
Kadir Cetinkaya 0a3c7960cb
Revert "Revert D106562 "[clangd] Get rid of arg adjusters in CommandMangler""
This reverts commit 2aa0cf19e7.
Get rid of reference to the temporary.
2021-07-26 11:13:22 +02:00
Fangrui Song 2aa0cf19e7 Revert D106562 "[clangd] Get rid of arg adjusters in CommandMangler"
This reverts commit 1c0d0085bc.

This commit made unittest BuildCompilerInvocation.DropsPlugins crash.
2021-07-23 09:50:43 -07:00
Kadir Cetinkaya e7590d748a
Revert "[clangd] Canonicalize compile flags before applying edits"
This reverts commit 7cc8a8e384.
2021-07-23 18:20:52 +02:00
Kadir Cetinkaya 04e21fbc44
Revert "[clangd] Adjust compile flags to contain only the requested file as input"
This reverts commit ba5dd945ad.
2021-07-23 17:58:11 +02:00
Kadir Cetinkaya ba5dd945ad
[clangd] Adjust compile flags to contain only the requested file as input
Depends on D106527.

Differential Revision: https://reviews.llvm.org/D106639
2021-07-23 17:15:06 +02:00
Kadir Cetinkaya 7cc8a8e384
[clangd] Canonicalize compile flags before applying edits
Pushes input for the compile action to the end while separating with a
`--` before applying other manglings. This ensures edits that effect only the
arguments that come after them works, like changing parse language via -x.

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

Differential Revision: https://reviews.llvm.org/D106527
2021-07-23 17:15:06 +02:00
Kadir Cetinkaya 1c0d0085bc
[clangd] Get rid of arg adjusters in CommandMangler
Differential Revision: https://reviews.llvm.org/D106562
2021-07-23 17:15:06 +02:00
Kadir Cetinkaya d2a6ec8eae
[clangd] Use CommandMangler in TestTU
This makes testing setup look closer to production.

Differential Revision: https://reviews.llvm.org/D106535
2021-07-23 17:15:05 +02:00
Sam McCall 253b8145de [clangd] Avoid range-loop init-list lifetime subtleties.
The original code appears to be OK per the spec, but we've had 3 reports of
crashes with certain unofficial builds of clangd that look a lot like old
compilers (GCC 5.4?) getting lifetime rules wrong.

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

Differential Revision: https://reviews.llvm.org/D106654
2021-07-23 15:33:04 +02:00
Kirill Bobyrev a0987e350c
[clangd] Improve performance of dex by 45-60%
Take full advantage of AND's iterator children size estimation: use early reset
in sync() and prevent large overhead. The idea is that the children at the
beginning of the list are smaller and cheaper to advance. Very large children
negate the effect of this performance optimisation and hence should be
advanced only when absolutely necessary. By reducing the number of large
iterators' updates, we increase the performance by a large margin.

This change was tested on a comprehensive query dataset. The performance
boost increases with the average length of the query, on small queries it is
close to 45% but the longer they go the closer it gets to 60% and beyond.

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D106528
2021-07-23 15:28:35 +02:00
Whisperity 8b0cc4a65d [clang-tidy] Improve "common type" diagnostic output in 'bugprone-easily-swappable-parameters'
Make the check handle cases of the "common type" involved in the mix
being non-trivial, e.g. pointers, references, attributes, these things
coming from typedefs, etc.

This results in clearer diagnostics that have more coverage in their
explanation, such as saying `const int &` as common type instead of
`int`.

Reviewed By: aaron.ballman

Differential Revision: http://reviews.llvm.org/D106442
2021-07-23 10:26:22 +02:00
Felix Berger cb4c12b611 [clang-tidy] performance-unnecessary-copy-initialization: Create option to exclude container types from triggering the check.
Add string list option of type names analagous to `AllowedTypes` which lets
users specify a list of ExcludedContainerTypes.

Types matching this list will not trigger the check when an expensive variable
is copy initialized from a const accessor method they provide, i.e.:

```
ExcludedContainerTypes = 'ExcludedType'

void foo() {
  ExcludedType<ExpensiveToCopy> Container;
  const ExpensiveToCopy NecessaryCopy = Container.get();
}
```

Even though an expensive to copy variable is copy initialized the check does not
trigger because the container type is excluded.

This is useful for container types that don't own their data, such as view types
where modification of the returned references in other places cannot be reliably
tracked, or const incorrect types.

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

Reviewed-by: ymandel
2021-07-22 16:20:20 -04:00
Felix Berger 00edae9203 [clang-tidy] performance-unnecessary-copy-initialization: Disable check when variable and initializer have different replaced template param types.
This can happen when a template with two parameter types is instantiated with a
single type. The fix would only be valid for this instantiation but fail for
others that rely on an implicit type conversion.

The test cases illustrate when the check should trigger and when not.

Differential Revision: https://reviews.llvm.org/D106011
2021-07-22 15:17:24 -04:00
Whisperity 473eff1c30 [clang-tidy] Fix crash and handle AttributedType in 'bugprone-easily-swappable-parameters'
@vabridgers identified a way to crash the check by running on code that
involve `AttributedType`s. This patch fixes the check to first and
foremost not crash, but also improves the logic handling qualifiers.

If the types contain any additional (not just CVR) qualifiers that are
not the same, they will not be deemed mixable. The logic for CVR-Mixing
and the `QualifiersMix` check option remain unchanged.

Reviewed By: aaron.ballman, vabridgers

Differential Revision: http://reviews.llvm.org/D106361
2021-07-22 10:20:17 +02:00
Nathan Ridge f443793d26 [clangd] Ensure Ref::Container refers to an indexed symbol
Fixes https://github.com/clangd/clangd/issues/806

Differential Revision: https://reviews.llvm.org/D105083
2021-07-22 03:33:40 -04:00
David Blaikie a46c63c878 Fix assigned-but-unused (except in an assert) warning with a void cast 2021-07-21 17:12:22 -07:00
Kirill Bobyrev 907efdf95d [clangd] Cleanup FuzzyFindRequest serialization and dex benchmark
* Due to the LLVM's JSON library changes (?), FuzzyFindRequest serialization is
  no longer valid since arrays are serialized as llvm::json::Array already.
  Hence, current implementation creates a nested array.
* YAML format is no longer the default, mention this for the benchmark.
* FIXME is no longer relevant. I ran benchmarks that showed no improvement with
  priority_queue years ago.

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D106432
2021-07-21 14:51:16 +02:00
Sam McCall 91670f5f20 [clangd] Remove big PreambleData constructor. NFC 2021-07-21 11:31:52 +02:00
Sam McCall fd22785054 [Lex] Consider a PCH header-guarded even with #endif truncated
This seems to be a more useful behavior for tools that use preambles.
I believe it doesn't affect real compiles: the PCH is only included once
when used, and recursive inclusion of the main-file *within* the PCH
isn't supported in any case.

Differential Revision: https://reviews.llvm.org/D106204
2021-07-20 14:25:36 +02:00
Haojian Wu eb03fa1d2c [clang-tidy] Don't suggest "inline" fix for main function in
definitions-in-headers check.
2021-07-20 14:24:38 +02:00
Sam McCall 69c04ef95a [clangd] Propagate header-guarded flag from preamble to main AST
Fixes https://github.com/clangd/clangd/issues/377

Differential Revision: https://reviews.llvm.org/D106203
2021-07-20 14:21:39 +02:00
Sam McCall 4190017245 [clangd] Add tests covering existing header-guard behavior. NFC
A few different mechanisms here that will need some work to untangle:
 - self-include in a preamble being an error even if the file is ifdef-guarded
 - the is-include-guarded flag not being propagated from preamble to main ast
 - preambles containing the first half on an include guard discard that info

For now just record current behavior.

Relevant to:
- https://github.com/clangd/clangd/issues/811
- https://github.com/clangd/clangd/issues/377
- https://github.com/clangd/clangd/issues/262

Differential Revision: https://reviews.llvm.org/D106201
2021-07-20 14:12:23 +02:00
Elton 195786d7c2
Fix duplicate checks in clangd comments
This patch removes a duplicate checks in the top-level comments in `clang-tools-extra/clangd/ParsedAST.h`

Reviewed By: kadircet

Differential Revision: https://reviews.llvm.org/D106227
2021-07-19 15:14:55 +02:00
Ian Campbell f6ba03584b [clang-tidy] ensure run-clang-tidy reports children killed by signals
If a clang-tidy child process exits with a signal then run-clang-tidy will exit
with an error but there is no hint why in the output, since the clang-tidy
doesn't log anything and may not even have had the opportunity to do so
depending on the signal used.

`subprocess.CompletedProcess.returncode` is the negative signal number in this
case.

I hit this in a CI system where the parallelism used exceeded the RAM assigned
to the container causing the OOM killer to SIGKILL clang-tidy processes.

Reviewed By: sylvestre.ledru

Differential Revision: https://reviews.llvm.org/D99081
2021-07-19 14:18:26 +02:00
Whisperity 73e4b5cfa8 [clang-tidy] Add 'readability-suspicious-call-argument' check
Finds function calls where the call arguments might be provided in an
incorrect order, based on the comparison (via string metrics) of the
parameter names and the argument names against each other.

A diagnostic is emitted if an argument name is similar to a *different*
parameter than the one currently passed to, and it is sufficiently
dissimilar to the one it **is** passed to currently.

False-positive warnings from this check are useful to indicate bad
naming convention issues, even if a swap isn't necessary.
This check does not generate FixIts.

Originally implemented by @varjujan as his Master's Thesis work.
The check was subsequently taken over by @barancsuk who added type
conformity checks to silence false positive matches.
The work by @whisperity involved driving the check's review and fixing
some more bugs in the process.

Reviewed By: aaron.ballman, alexfh

Differential Revision: http://reviews.llvm.org/D20689

Co-authored-by: János Varjú <varjujanos2@gmail.com>
Co-authored-by: Lilla Barancsuk <barancsuklilla@gmail.com>
2021-07-19 10:18:09 +02:00
Mehdi Amini 76374573ce Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer
We can build it with -Werror=global-constructors now. This helps
in situation where libSupport is embedded as a shared library,
potential with dlopen/dlclose scenario, and when command-line
parsing or other facilities may not be involved. Avoiding the
implicit construction of these cl::opt can avoid double-registration
issues and other kind of behavior.

Reviewed By: lattner, jpienaar

Differential Revision: https://reviews.llvm.org/D105959
2021-07-16 07:38:16 +00:00
Mehdi Amini 8d051d8546 Revert "Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer"
This reverts commit af9321739b.
Still some specific config broken in some way that requires more
investigation.
2021-07-16 07:35:13 +00:00
Mehdi Amini af9321739b Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer
We can build it with -Werror=global-constructors now. This helps
in situation where libSupport is embedded as a shared library,
potential with dlopen/dlclose scenario, and when command-line
parsing or other facilities may not be involved. Avoiding the
implicit construction of these cl::opt can avoid double-registration
issues and other kind of behavior.

Reviewed By: lattner, jpienaar

Differential Revision: https://reviews.llvm.org/D105959
2021-07-16 06:54:26 +00:00
Mehdi Amini 16b5e9d6a2 Revert "Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer"
This reverts commit 42f588f39c.
Broke some buildbots
2021-07-16 03:46:53 +00:00
Mehdi Amini 42f588f39c Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer
We can build it with -Werror=global-constructors now. This helps
in situation where libSupport is embedded as a shared library,
potential with dlopen/dlclose scenario, and when command-line
parsing or other facilities may not be involved. Avoiding the
implicit construction of these cl::opt can avoid double-registration
issues and other kind of behavior.

Reviewed By: lattner, jpienaar

Differential Revision: https://reviews.llvm.org/D105959
2021-07-16 03:33:20 +00:00
Sam McCall 462d4de35b [clangd] Add CMake option to (not) link in clang-tidy checks
This reduces the size of the dependency graph and makes incremental
development a little more pleasant (less rebuilding).

This introduces a bit of complexity/fragility as some tests verify
clang-tidy behavior. I attempted to isolate these and build/run as much
of the tests as possible in both configs to prevent rot.

Expectation is that (some) developers will use this locally, but
buildbots etc will keep testing clang-tidy.

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

Differential Revision: https://reviews.llvm.org/D105679
2021-07-14 10:04:21 +02:00
Felix Berger 0ec812023b [clang-tidy] performance-unnecessary-copy-initialization: Do not remove comments on new lines.
When deleting the copy assignment statement because copied variable is not used
only remove trailing comments on the same line.

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

Reviewed-by: ymandel
2021-07-12 16:23:04 -04:00
Felix Berger 187e050b33 [clang-tidy] performance-unnecessary-copy-initialization: Disable structured bindings.
Structured bindings can currently trigger the check and lead to a wrong
fix. Because the DecompositionDecl itself is not used and the check does not
iterate through its the decl's bindings to verify whether the bindings' holding
vars are used this leads to the whole statement to be deleted.

To support structured bindings properly 3 cases would need to be considered.

  1. All holding vars are not used -> The statement can be deleted.
  2. All holding vars are used as const or not used -> auto can be converted to const auto&.
  3. Neither case is true -> leave unchanged.

In the check we'll have to separate the logic that determines this from the code
that produces the diagnostic and fixes and first determine which of the cases
we're dealing with before creating fixes.

Since this is a bigger refactoring we'll disable structured bindings for now to
prevent incorrect fixes.

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

Reviewed-by: ymandel
2021-07-12 09:55:27 -04:00
David Blaikie 1def2579e1 PR51018: Remove explicit conversions from SmallString to StringRef to future-proof against C++23
C++23 will make these conversions ambiguous - so fix them to make the
codebase forward-compatible with C++23 (& a follow-up change I've made
will make this ambiguous/invalid even in <C++23 so we don't regress
this & it generally improves the code anyway)
2021-07-08 13:37:57 -07:00
Tim Northover 48c68a630e Recommit: Support: add llvm::thread class that supports specifying stack size.
This adds a new llvm::thread class with the same interface as std::thread
except there is an extra constructor that allows us to set the new thread's
stack size. On Darwin even the default size is boosted to 8MB to match the main
thread.

It also switches all users of the older C-style `llvm_execute_on_thread` API
family over to `llvm::thread` followed by either a `detach` or `join` call and
removes the old API.

Moved definition of DefaultStackSize into the .cpp file to hopefully
fix the build on some (GCC-6?) machines.
2021-07-08 16:22:26 +01:00
Tim Northover 2bf5e8d953 Revert "Support: add llvm::thread class that supports specifying stack size."
It's causing build failures because DefaultStackSize isn't defined everywhere
it should be and I need time to investigate.
2021-07-08 14:59:47 +01:00
Tim Northover 727e1c9be3 Support: add llvm::thread class that supports specifying stack size.
This adds a new llvm::thread class with the same interface as std::thread
except there is an extra constructor that allows us to set the new thread's
stack size. On Darwin even the default size is boosted to 8MB to match the main
thread.

It also switches all users of the older C-style `llvm_execute_on_thread` API
family over to `llvm::thread` followed by either a `detach` or `join` call and
removes the old API.
2021-07-08 14:51:53 +01:00
Kirill Bobyrev de8274a1b9
[clangd] NFC: Remove outdated comment 2021-07-05 13:58:54 +02:00
Nathan Ridge a15adbcddd [clangd] Type hints for structured bindings
Hints are shown for the individual bindings, not the aggregate.

Differential Revision: https://reviews.llvm.org/D104617
2021-07-04 21:53:36 -04:00
Christopher Di Bella 478092d331 [clangd][iwyu] explicitly includes `<atomic>`
Compiling clangd with Clang modules and libc++ revealed that
`support/Threading.h` uses `std::atomic` but wasn't including the
correct header.

Differential Revision: https://reviews.llvm.org/D105400
2021-07-04 06:00:39 +00:00
Sam McCall e42bb5e35a Reapply [clangd] Fix possible assertion fail in TUScheduler
This reverts commit fff966b685.

Seems I managed to delete a critical ! after running the tests :-\
2021-07-02 16:32:13 +02:00
Sam McCall 33ff8078ff Revert "[clangd] Unbreak mac build differently 0c96a92d8666b8"
This reverts commit 2f79acb7b7.

Should no longer be needed after 26e1553a10
2021-07-02 16:29:48 +02:00
Sam McCall fff966b685 Revert "[clangd] Fix possible assertion fail in TUScheduler"
This reverts commit 50566947e9.
2021-07-02 16:07:11 +02:00
Sam McCall 50566947e9 [clangd] Fix possible assertion fail in TUScheduler
BlockUntilIdle is supposed to return false if it fails.
If an intermediate step fails to clear the queue, we shouldn't
charge ahead and assert on the state of the queue.
2021-07-02 15:57:39 +02:00
Sam McCall 26e1553a10 [clangd] CMake: express -Iclangd/ at top level and inherit
For files directly under clangd/, -Iclang-tools-extra/clangd (and the
equivalent for generated files) are not required, as CMake/the compiler puts
these directories on the include path by default.

However this means each subdirectory needs to
include_directories(.. ${CMAKE_CURRENT_BINARY_DIR}/..) etc, and this
proved annoying and error-prone to maintain and debug.

Since include_directories is inherited by subdirectories, we just
configure this explicitly at the top level instead.
2021-07-02 09:52:36 +02:00
Sam McCall 0c53f602d5 [clangd] Add some more missing include dirs for completeness 2021-07-02 09:04:53 +02:00
Sam McCall 86c5afa6e6 [clangd] Fix XPC build due to missing include path
(Tentative, untested as I don't have a mac)
2021-07-02 08:47:46 +02:00
Nico Weber 2f79acb7b7 [clangd] Unbreak mac build differently 0c96a92d86
This reverts b56e5f8a10 (and follow-up f6db88535c) and instead
restores the state we had before 0c96a92d8666b8: ClangdMain.cpp
includes Features.inc before including Transport.h.

This is a bit ugly, but it matches the former state and making Transport.h
include Features.h means that xpc/ needs to be able to find the generated
Features.inc, wich is also a bit ugly.
2021-07-01 10:51:27 -04:00
Sam McCall 0e2d4bd4bf [clangd] Fix gRPC build due to missing include path 2021-07-01 09:33:29 +02:00