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
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
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
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)
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.
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.
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
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.
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.
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.
Without this patch clangd silently process compiler instance prepare failure and only LSP errors "Invalid AST" could be found in logs.
E.g. the reason of the problem https://github.com/clangd/clangd/issues/734 is impossible to understand without verbose logs or with disabled background index.
This patch adds more information into logs to help understand the reason of such failures.
Logs without this patch:
```
E[...] Could not build a preamble for file test.cpp version 1
```
Logs with this patch:
```
E[...] Could not build a preamble for file test.cpp version 1: CreateTargetInfo() return null
..
E[...] Failed to prepare a compiler instance: unknown target ABI 'lp64'
```
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D104056
That commit removed the include of Features.inc from ClangdLSPServer.h,
but ClangdMain.cpp relied on this include to pull in Features.inc for
the #if at the bottom of Transport.h.
Since the include is needed in Transport.h, just add it to there
directly.
Objective-C lets you use the `self.prop` syntax as sugar for both
`[self prop]` and `[self setProp:]`, but clangd previously did not
provide a semantic token for `prop`.
Now, we provide a semantic token, treating it like a normal property
except it's backed by a `ObjCMethodDecl` instead of a
`ObjCPropertyDecl`.
Differential Revision: https://reviews.llvm.org/D104117
This displays as: `Size: 4 bytes (+4 padding)`
Also stop showing (byte) offset/size for bitfields. They're not
meaningful and using them to calculate padding is dangerous!
Differential Revision: https://reviews.llvm.org/D98377
Some tweaks might edit file types not supported by clang-format. This
patch gives them a way to signal that they do not require formatting.
Differential Revision: https://reviews.llvm.org/D105039
While the original check's purpose is to identify potentially dangerous
functions based on the parameter types (as identifier names do not mean
anything when it comes to the language rules), unfortunately, such a plain
interface check rule can be incredibly noisy. While the previous
"filtering heuristic" is able to find many similar usages, there is an entire
class of parameters that should not be warned about very easily mixed by that
check: parameters that have a name and their name follows a pattern,
e.g. `text1, text2, text3, ...`.`
This patch implements a simple, but powerful rule, that allows us to detect
such cases and ensure that no warnings are emitted for parameter sequences that
follow a pattern, even if their types allow for them to be potentially mixed at a call site.
Given a threshold `k`, warnings about two parameters are filtered from the
result set if the names of the parameters are either prefixes or suffixes of
each other, with at most k letters difference on the non-common end.
(Assuming that the names themselves are at least `k` long.)
- The above `text1, text2` is an example of this. (Live finding from Xerces.)
- `LHS` and `RHS` are also fitting the bill here. (Live finding from... virtually any project.)
- So does `Qmat, Tmat, Rmat`. (Live finding from I think OpenCV.)
Reviewed By: aaron.ballman
Differential Revision: http://reviews.llvm.org/D97297
There are several types of functions and various reasons why some
"swappable parameters" cannot be fixed with changing the parameters' types, etc.
The most common example might be int `min(int a, int b)`... no matter what you
do, the two parameters must remain the same type.
The **filtering heuristic** implemented in this patch deals with trying to find
such functions during the modelling and building of the swappable parameter
range.
If the parameter currently scrutinised matches either of the predicates below,
it will be regarded as **not swappable** even if the type of the parameter
matches.
Reviewed By: aaron.ballman
Differential Revision: http://reviews.llvm.org/D78652
Adds a relaxation option ModelImplicitConversions which will make the check
report for cases where parameters refer to types that are implicitly
convertible to one another.
Example:
struct IntBox { IntBox(int); operator int(); };
void foo(int i, double d, IntBox ib) {}
Implicit conversions are the last to model in the set of things that are
reasons for the possibility of a function being called the wrong way which is
not always immediately apparent when looking at the function (signature or
call).
Reviewed By: aaron.ballman, martong
Differential Revision: http://reviews.llvm.org/D75041
Adds a relaxation option QualifiersMix which will make the check report for
cases where parameters refer to the same type if they only differ in qualifiers.
This makes cases, such as the following, not warned about by default, produce
a warning.
void* memcpy(void* dst, const void* src, unsigned size) {}
However, unless people meticulously const their local variables, unfortunately,
even such a function carry a potential swap:
T* obj = new T; // Not const!!!
void* buf = malloc(sizeof(T));
memcpy(obj, buf, sizeof(T));
// ^~~ ^~~ accidental swap here, even though the interface "specified" a const.
Reviewed By: aaron.ballman
Differential Revision: http://reviews.llvm.org/D96355
The base patch only deals with strict (canonical) type equality, which is
merely a subset of all the dangerous function interfaces that we intend to
find.
In addition, in the base patch, canonical type equivalence is not diagnosed in
a way that is immediately apparent to the user.
This patch extends the check with two features:
* Proper typedef diagnostics and explanations to the user.
* "Reference bind power" matching.
Case 2 is a necessary addition because in every case someone encounters a
function `f(T t, const T& tr)`, any expression that might be passed to either
can be passed to both. Thus, such adjacent parameter sequences should be
matched.
Reviewed By: aaron.ballman
Differential Revision: http://reviews.llvm.org/D95736
Finds function definitions where parameters of convertible types follow
each other directly, making call sites prone to calling the function
with swapped (or badly ordered) arguments.
Such constructs are usually the result of inefficient design and lack of
exploitation of strong type capabilities that are possible in the
language.
This check finds and flags **function definitions** and **not** call
sites!
Reviewed By: aaron.ballman, alexfh
Differential Revision: http://reviews.llvm.org/D69560
They are already provided by Sema, deserializing from preamble if need
be. Moreover category names are meaningless outside interface/implementation
context, hence they were only causing noise.
Differential Revision: https://reviews.llvm.org/D104540
This fixes false positive cases where a reference is initialized outside of a
block statement and then its initializing variable is modified. Another case is
when the looped over container is modified.
Differential Revision: https://reviews.llvm.org/D103021
Reviewed-by: ymandel
Ignored diagnostics were only checked after level adjusters and assumed
it would stay the same for the rest. But it can also be modified by
FeatureModules.
Differential Revision: https://reviews.llvm.org/D103387
Given `int foo, bar;`, TraverseAST reveals this tree:
TranslationUnitDecl
- foo
- bar
Before this patch, with the TraversalScope set to {foo}, TraverseAST yields:
foo
After this patch it yields:
TranslationUnitDecl
- foo
Also, TraverseDecl(TranslationUnitDecl) now respects the traversal scope.
---
The main effect of this today is that clang-tidy checks that match the
translationUnitDecl(), either in order to traverse it or check
parentage, should work.
Differential Revision: https://reviews.llvm.org/D104071
<string> is currently the highest impact header in a clang+llvm build:
https://commondatastorage.googleapis.com/chromium-browser-clang/llvm-include-analysis.html
One of the most common places this is being included is the APInt.h header, which needs it for an old toString() implementation that returns std::string - an inefficient method compared to the SmallString versions that it actually wraps.
This patch replaces these APInt/APSInt methods with a pair of llvm::toString() helpers inside StringExtras.h, adjusts users accordingly and removes the <string> from APInt.h - I was hoping that more of these users could be converted to use the SmallString methods, but it appears that most end up creating a std::string anyhow. I avoided trying to use the raw_ostream << operators as well as I didn't want to lose having the integer radix explicit in the code.
Differential Revision: https://reviews.llvm.org/D103888
mixed integer and floating point types with WarnOnEquivalentBitWidth=0.
Also standardize control flow of handleX conversion functions to make it easier to be consistent.
Patch by Stephen Concannon!
Differential Revision: https://reviews.llvm.org/D103894
There is a followup fix for a unit test introduced at D102906. The test file was placed into a temp folder and test assumed that it would be visible without the full path specification.
This behaviour can be changed in future and it would be good to specify full path to the file at the test.
Test Plan:
```
ninja check-clang-tools
```
Reviewed By: DmitryPolukhin
Differential Revision: https://reviews.llvm.org/D104021
Sam McCall