A semicolon-separated list of the names of functions or methods to be considered as not having side-effects was added for bugprone-assert-side-effect. It can be used to exclude methods like iterator::begin/end from being considered as having side-effects.
Differential Revision: https://reviews.llvm.org/D116478
Using clang::CallGraph to get the called functions.
This makes a better foundation to improve support for
C++ and print the call chain.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D118016
enum FileChangeReason has four possible type EnterFile, ExitFile,
SystemHeaderPragma and RenameFile,
It should pop the back element of Files only if FileChangeReason is ExitFile.
This is a cleanup of all llvm-qualified-auto findings.
This patch was created by automatically applying the fixes from
clang-tidy.
Differential Revision: https://reviews.llvm.org/D113898
* Suppress a lot of `-Wtautological-compare` warning
* Speed up file build a little bit
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D98110
This commit introduces a new check `readability-container-contains` which finds
usages of `container.count()` and `container.find() != container.end()` and
instead recommends the `container.contains()` method introduced in C++20.
For containers which permit multiple entries per key (`multimap`, `multiset`,
...), `contains` is more efficient than `count` because `count` has to do
unnecessary additional work.
While this this performance difference does not exist for containers with only
a single entry per key (`map`, `unordered_map`, ...), `contains` still conveys
the intent better.
Reviewed By: xazax.hun, whisperity
Differential Revision: http://reviews.llvm.org/D112646
This patch adds a forward declaraiton of DynTypedNode.
DumpAST.h is relying on the forward declaration of DynTypedNode in
ASTContext.h, which is undesirable.
Looks for duplicate includes and removes them.
Every time an include directive is processed, check a vector of filenames
to see if the included file has already been included. If so, it issues
a warning and a replacement to remove the entire line containing the
duplicated include directive.
When a macro is defined or undefined, the vector of filenames is cleared.
This enables including the same file multiple times, but getting
different expansions based on the set of active macros at the time of
inclusion. For example:
#undef NDEBUG
#include "assertion.h"
// ...code with assertions enabled
#define NDEBUG
#include "assertion.h"
// ...code with assertions disabled
Since macros are redefined between the inclusion of assertion.h,
they are not flagged as redundant.
Differential Revision: https://reviews.llvm.org/D7982
Currently the fix hint is hardcoded to gsl::at(). This poses
a problem for people who, for a number of reasons, don't want
or cannot use the GSL library (introducing a new third-party
dependency into a project is not a minor task).
In these situations, the fix hint does more harm than good
as it creates confusion as to what the fix should be. People
can even misinterpret the fix "gsl::at" as e.g. "std::array::at",
which can lead to even more trouble (e.g. when having guidelines
that disallow exceptions).
Furthermore, this is not a requirement from the C++ Core Guidelines.
simply that array indexing needs to be safe. Each project should
be able to decide upon a strategy for safe indexing.
The fix-it is kept for people who want to use the GSL library.
Differential Revision: https://reviews.llvm.org/D117857
This is the original patch in my GNUInstallDirs series, now last to merge as the final piece!
It arose as a new draft of D28234. I initially did the unorthodox thing of pushing to that when I wasn't the original author, but since I ended up
- Using `GNUInstallDirs`, rather than mimicking it, as the original author was hesitant to do but others requested.
- Converting all the packages, not just LLVM, effecting many more projects than LLVM itself.
I figured it was time to make a new revision.
I have used this patch series (and many back-ports) as the basis of https://github.com/NixOS/nixpkgs/pull/111487 for my distro (NixOS), which was merged last spring (2021). It looked like people were generally on board in D28234, but I make note of this here in case extra motivation is useful.
---
As pointed out in the original issue, a central tension is that LLVM already has some partial support for these sorts of things. Variables like `COMPILER_RT_INSTALL_PATH` have already been dealt with. Variables like `LLVM_LIBDIR_SUFFIX` however, will require further work, so that we may use `CMAKE_INSTALL_LIBDIR`.
These remaining items will be addressed in further patches. What is here is now rote and so we should get it out of the way before dealing more intricately with the remainder.
Reviewed By: #libunwind, #libc, #libc_abi, compnerd
Differential Revision: https://reviews.llvm.org/D99484
LLVM Programmer’s Manual strongly discourages the use of `std::vector<bool>` and suggests `llvm::BitVector` as a possible replacement.
Currently, some users of `std::vector<bool>` cannot switch to `llvm::BitVector` because it doesn't implement the `pop_back()` and `back()` functions.
To enable easy transition of `std::vector<bool>` users, this patch implements `llvm::BitVector::pop_back()` and `llvm::BitVector::back()`.
Reviewed By: dexonsmith
Differential Revision: https://reviews.llvm.org/D117115
These errors are non-harmful and should be transient. They either
imply:
- compilation database returned stale results for TUs and it'll be fixed once
it's updated to match project state.
- a TUs dependencies has changed and some headers no longer exist. this should
be fixed with the next indexing cycle.
In either case the user will have some stale symbols in their index until clangd
restarts and the underlying issue is resolved. On the downside these logs are
confusing users when there's another issue.
Differential Revision: https://reviews.llvm.org/D117792
Previously, function(nullptr) would have been fixed with function({}). This unfortunately can change overload resolution and even become ambiguous. T(nullptr) was already being fixed with T(""), so this change just brings function calls in line with that.
Differential Revision: https://reviews.llvm.org/D117840
I used a C++ code block in check documentation to show example
output from clang-tidy, but since the example output isn't
kosher C++, sphinx didn't like that when it went to syntax
highlight the block. So switch to a literal block instead
and forego any highlighting.
Fixes build error
<https://lab.llvm.org/buildbot/#/builders/115/builds/21145>
Previously, any macro that didn't look like a varargs macro
or a function style macro was reported with a warning that
it should be replaced with a constexpr const declaration.
This is only reasonable when the macro body contains constants
and not expansions like ",", "[[noreturn]]", "__declspec(xxx)",
etc.
So instead of always issuing a warning about every macro that
doesn't look like a varargs or function style macro, examine the
tokens in the macro and only warn about the macro if it contains
only comment and constant tokens.
Differential Revision: https://reviews.llvm.org/D116386Fixes#39945
It will make the output more versbose, but I found that these are useful
information when debugging selection tree.
Differential Revision: https://reviews.llvm.org/D117475
E.g. `Concept auto Func();`
The nameLoc for the constained auto type loc pointed to the concept name
loc, it should be the auto token loc. This patch fixes it, and remove
a relevant hack in clang-tidy check.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D117009
Targets are not necessarily inserted in the order they appear in source
code. For example we could traverse overload sets, or selectively insert
template patterns after all other decls.
So order the targets before printing to make sure tests are not dependent on
such implementation details. We can also do it in production, but that might be
wasteful as we haven't seen any complaints in the wild around these orderings
yet.
Differential Revision: https://reviews.llvm.org/D117549
Fixes PR#52245. I've also added a few test cases beyond PR#52245 that would also fail with the current implementation, which is quite brittle in many respects (e.g. it uses the `hasDescendant()` matcher to find the container that is being accessed, which is very easy to trick, as in the example in PR#52245).
I have not been able to reproduce the second issue mentioned in PR#52245 (namely that using the `data()` member function is suggested even for containers that don't have it), but I've added a test case for it to be sure.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D113863
LLVM Programmer’s Manual strongly discourages the use of `std::vector<bool>` and suggests `llvm::BitVector` as a possible replacement.
This patch does just that for clangd and clang-tidy.
Reviewed By: dexonsmith
Differential Revision: https://reviews.llvm.org/D117119
The recommendation on Windows is to checkout from git with
core.autolf=false in order to preserve LF line endings on
test files. However, when creating a new check this results
in modified files as having switched all the line endings on
Windows. Write all files with explicit LF line endings to
prevent this.
Fixes#52968
Differential Revision: https://reviews.llvm.org/D117535
The `{HeaderSearch,Preprocessor}::LookupFile()` functions take an out-parameter `const DirectoryLookup *&`. Most callers end up creating a `const DirectoryLookup *` variable that's otherwise unused.
This patch changes the out-parameter from reference to a pointer, making it possible to simply pass `nullptr` to the function without the ceremony.
Reviewed By: ahoppen
Differential Revision: https://reviews.llvm.org/D117312
This is a follow-up on D116643. `isInSystemHeader` check already detects
symbols coming from the Standard Library, so searching for the qualified name
in StdSymbolMap.inc is no longer necessary.
The tests filtering out purely based on the symbol qualified names are removed.
Reviewed By: hokein
Differential Revision: https://reviews.llvm.org/D117491
clang-tidy currently reports false positives even for simple cases such as:
```
struct S {
using X = S;
X &operator=(const X&) { return *this; }
};
```
This is due to the fact that the `misc-unconventional-assign-operator` check fails to look at the //canonical// types. This patch fixes this behavior.
Reviewed By: aaron.ballman, mizvekov
Differential Revision: https://reviews.llvm.org/D114197
Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=48086 | PR#48086 ]]. The problem is that the current matcher uses `hasParent()` to detect friend declarations, but for a template friend declaration, the immediate parent of the `FunctionDecl` is a `FunctionTemplateDecl`, not the `FriendDecl`. Therefore, I have replaced the matcher with `hasAncestor()`.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D114299
Adds a option `use-dirty-preambles` to enable using unsaved in editor contents when building pre-ambles.
This enables a more seamless user experience when switching between header and implementation files and forgetting to save inbetween.
It's also in line with the LSP spec that states open files in the editor should be used instead of on the contents on disk - https://microsoft.github.io/language-server-protocol/overviews/lsp/overview/
For now the option is defaulted to off and hidden, Though I have a feeling it should be moved into the `.clangd` config and possibly defaulted to true.
Addresses https://github.com/clangd/clangd/issues/488
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D95046
The patch was reverted because it caused a crash during PCH build -- we
missed to update the RParenLoc in TreeTransform<Derived>::TransformAutoType.
This relands 55d96ac and 37ec65e with a test and fix.
This is a workaround (adding a newline to the eof) in clangd to avoid the code
completion crash, see https://github.com/clangd/clangd/issues/332.
In principle, this is a clang bug, we should fix it in clang, but it is not
trivial.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D117456
The newline-eof fix was rendered as "insert '...'", this patch
special-case it.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D117294
This is the original patch in my GNUInstallDirs series, now last to merge as the final piece!
It arose as a new draft of D28234. I initially did the unorthodox thing of pushing to that when I wasn't the original author, but since I ended up
- Using `GNUInstallDirs`, rather than mimicking it, as the original author was hesitant to do but others requested.
- Converting all the packages, not just LLVM, effecting many more projects than LLVM itself.
I figured it was time to make a new revision.
I have used this patch series (and many back-ports) as the basis of https://github.com/NixOS/nixpkgs/pull/111487 for my distro (NixOS), which was merged last spring (2021). It looked like people were generally on board in D28234, but I make note of this here in case extra motivation is useful.
---
As pointed out in the original issue, a central tension is that LLVM already has some partial support for these sorts of things. Variables like `COMPILER_RT_INSTALL_PATH` have already been dealt with. Variables like `LLVM_LIBDIR_SUFFIX` however, will require further work, so that we may use `CMAKE_INSTALL_LIBDIR`.
These remaining items will be addressed in further patches. What is here is now rote and so we should get it out of the way before dealing more intricately with the remainder.
Reviewed By: #libunwind, #libc, #libc_abi, compnerd
Differential Revision: https://reviews.llvm.org/D99484
This is the original patch in my GNUInstallDirs series, now last to merge as the final piece!
It arose as a new draft of D28234. I initially did the unorthodox thing of pushing to that when I wasn't the original author, but since I ended up
- Using `GNUInstallDirs`, rather than mimicking it, as the original author was hesitant to do but others requested.
- Converting all the packages, not just LLVM, effecting many more projects than LLVM itself.
I figured it was time to make a new revision.
I have used this patch series (and many back-ports) as the basis of https://github.com/NixOS/nixpkgs/pull/111487 for my distro (NixOS), which was merged last spring (2021). It looked like people were generally on board in D28234, but I make note of this here in case extra motivation is useful.
---
As pointed out in the original issue, a central tension is that LLVM already has some partial support for these sorts of things. Variables like `COMPILER_RT_INSTALL_PATH` have already been dealt with. Variables like `LLVM_LIBDIR_SUFFIX` however, will require further work, so that we may use `CMAKE_INSTALL_LIBDIR`.
These remaining items will be addressed in further patches. What is here is now rote and so we should get it out of the way before dealing more intricately with the remainder.
Reviewed By: #libunwind, #libc, #libc_abi, compnerd
Differential Revision: https://reviews.llvm.org/D99484
During pop() we convert nodes into spans of expanded syntax::Tokens.
If we precompute a range of plausible (expanded) tokens, then we can do an
extremely cheap approximate hit-test against it, because syntax::Tokens are
ordered by pointer.
This would seem not to buy anything (we don't enter nodes unless they overlap
the selection), but in fact the spans we have are for *newly* claimed ranges
(i.e. those unclaimed by any child node).
So if you have:
{ { [[2+2]]; } }
then all of the CompoundStmts pass the hit test and are pushed, but we skip
full hit-testing of the brackets during pop() as they lie outside the range.
This is ~10x average speedup for selectiontree on a bad case I've seen
(large gtest file).
Differential Revision: https://reviews.llvm.org/D117107
Not sure it's OK to suppress this in clang itself - if we're building a PCH
or module, maybe it matters?
Differential Revision: https://reviews.llvm.org/D116925
The AST doesn't track their locations, and the default behavior of attributing
them to the lexically-enclosing node is sloppy and often inaccurate.
Also add a couple of passing test cases for declarators that weren't obvious.
Differential Revision: https://reviews.llvm.org/D117185
When searching for AST nodes that may overlap the selection, mayHit() was only
attempting to prune nodes whose begin/end are both in the main file.
While failing to prune never gives wrong results, it hurts performance.
In GTest unit-tests, `TEST()` macros at the top level declare classes.
These were never pruned and we traversed *every* such class for any selection.
We fix this by reasoning about what tokens such a node might claim.
They must lie within its ultimate macro expansion range, so if this doesn't
overlap with the selection, we can prune the node.
Differential Revision: https://reviews.llvm.org/D116978
New values:
- Split Dynamic into Open/Preamble
- Add Background (previously was just Unknown)
- Soon: stdlib index
This requires extending to 16 bits, which fits within the padding of Symbol.
Unfortunately we're also *serializing* SymbolOrigin as a fixed 8 bits.
Stop serializing SymbolOrigin:
- conceptually, the source is whoever indexes or *deserializes* a symbol
- deserialization takes SymbolOrigin as a parameter and stamps it on each sym
- this is a breaking format change
Differential Revision: https://reviews.llvm.org/D115243
C++ member function bodies (including ctor initializers) are first captured
into a buffer and then parsed after the class is complete. (This allows
members to be referenced even if declared later).
When the boundary of the function body cannot be established, its buffer is
discarded and late-parsing never happens (it would surely fail).
For code completion this is the wrong tradeoff: the point of the parse is to
generate completions as a side-effect.
Today, when the ctor body wasn't typed yet there are no init list completions.
With this patch we parse such an init-list if it contains the completion point.
There's one caveat: the parser has to decide where to resume parsing members
after a broken init list. Often the first clear recovery point is *after* the
next member, so that member is missing from completion/signature help etc. e.g.
struct S {
S() m //<- completion here
int maaa;
int mbbb;
}
Here "int maaa;" is treated as part of the init list, so "maaa" is not available
as a completion. Maybe in future indentation can be used to recognize that
this is a separate member, not part of the init list.
Differential Revision: https://reviews.llvm.org/D116294
Updates the check and tests to not diagnose the null case for string_view (but retains it for string). This prevents the check from giving duplicate warnings that are caught by bugprone-stringview-nullptr ([[ https://reviews.llvm.org/D113148 | D113148 ]]).
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D114823
bugprone-stringview-nullptr was not initially written with tests for return statements. After landing the check, the thought crossed my mind to add such tests. After writing them, I realized they needed additional handling in the matchers.
Differential Revision: https://reviews.llvm.org/D115121
Sometimes a macro invocation will look like an argument list
declaration. Improve the check to detect this situation and not
try to modify the macro invocation.
Thanks to Nathan James for the fix.
- Ignore implicit typedefs (e.g. compiler builtins)
- Improve lexing state machine to locate void argument tokens
- Add additional return_t() macro tests
- clang-format control in the test case file
- remove braces around single statements per LLVM style guide
Fixes#43791
Differential Revision: https://reviews.llvm.org/D116425
std::remove from algorithm is a lot more common than the overload from
the cstdio (which deletes files). This patch introduces a set of symbols
for which we should prefer the overloaded versions.
Differential Revision: https://reviews.llvm.org/D114724
Often we run into situations where we want to ignore
warnings from system headers, but Clang will still
give warnings about the contents of a macro defined
in a system header used in user-code.
Introduce a ShowInSystemMacro option to be able to
specify which warnings we do want to keep raising
warnings for. The current behavior is kept in this patch
(i.e. warnings from system macros are enabled by default).
The decision as to whether this should be an opt-in or opt-out
feature can be made in a separate patch.
To put the feature to test, replace duplicated code for
Wshadow and Wold-style-cast with the SuppressInSystemMacro tag.
Also disable the warning for C++20 designators, fixing #52944.
Differential Revision: https://reviews.llvm.org/D116833
The cppcoreguidelines-pro-bounds-array-to-pointer-decay check currently
accepts:
const char *b = i ? "foo" : "foobar";
but not
const char *a = i ? "foo" : "bar";
This is because the AST is slightly different in the latter case (see
https://godbolt.org/z/MkHVvs).
This eliminates the inconsistency by making it accept the latter form
as well.
Fixes https://github.com/llvm/llvm-project/issues/31155.
"driver <flags> -- <input>" is a particularly convenient form of the
compile command to manipulate, with fewer special cases to handle.
Guaranteeing that the output command is of that form is cheap and makes
it easier to consume the result in some cases.
Differential Revision: https://reviews.llvm.org/D116721
Break up the huge function by extracting a class, storing intermediate
state as class members and breaking up the big function into a group
of class methods all at the same level of abstraction.
Differential Revision: https://reviews.llvm.org/D56343
A function call `unresolved()` in C will generate an implicit declaration
of the missing function and warn `ext_implicit_function_decl` or so.
(Compared to in C++ where we get `err_undeclared_var_use`).
We want to try to resolve these names.
Unfortunately typo correction is disabled in sema for performance
reasons unless this warning is promoted to error.
(We need typo correction for include-fixer.)
It's not clear to me where a switch to force this correction on should
go, include-fixer is kind of a hack. So hack more by telling sema we're
promoting them to error.
Fixes https://github.com/clangd/clangd/issues/937
Differential Revision: https://reviews.llvm.org/D115490
The idea is that the feature will always be advertised at the LSP level, but
depending on config we'll return partial or no responses.
We try to avoid doing the analysis for hints we're not going to return.
Examples of syntax:
```
InlayHints:
Enabled: No
---
InlayHints:
ParameterNames: No
---
InlayHints:
ParameterNames: Yes
DeducedTypes: Yes
```
Differential Revision: https://reviews.llvm.org/D116713
Even if findImplementors does not use
uninitialized parameter it's still UB and
it's going to be detected by msan with:
-Xclang -enable-noundef-analysis -mllvm -msan-eager-checks=1
Differential Revision: https://reviews.llvm.org/D116827
This means it's a "real feature" in clangd 14, albeit one that requires special
client support.
- remove "preview" from the flag description
- expose the `clangdInlayHints` capability by default
- provide `position` as well as `range`
- support `InlayHintsParams.range` to restrict the range retrieved
- inlay hint list is in document order (sorted by position)
Still to come: control feature via config rather than flag.
Fixes https://github.com/clangd/clangd/issues/313
Protocol doc is in https://github.com/llvm/clangd-www/pull/56/files
Differential Revision: https://reviews.llvm.org/D116699
Currently, it's inconsistent that warnings are disabled if they
come from system headers, unless they come from macros.
Typically a user cannot act upon these warnings coming from
system macros, so clang-tidy should ignore them unless the
user specifically requests warnings from system headers
via the corresponding configuration.
This change broke the ProTypeVarargCheck check, because it
was checking for the usage of va_arg indirectly, expanding it
(it's a system macro) to detect the usage of __builtin_va_arg.
The check has been fixed by checking directly what the rule
is about: "do not use va_arg", by adding a PP callback that
checks if any macro with name "va_arg" is expanded. The old
AST matcher is still kept for compatibility with Windows.
Add unit test that ensures warnings from macros are disabled
when not using the -system-headers flag. Document the change
in the Release Notes.
Differential Revision: https://reviews.llvm.org/D116378
Although we moved to Github Issues. The bug report message refers to
Bugzilla still. This patch tries to update these URLs.
Reviewed By: MaskRay, Quuxplusone, jhenderson, libunwind, libc++
Differential Revision: https://reviews.llvm.org/D116351
- Recognize older checks that might not end with Check.cpp
- Update list of checks based on improvements to add_new_check
- Fix spelling error in TransformerClangTidyCheck.h
Fixes#52962
Differential Revision: https://reviews.llvm.org/D116550
This reverts commit 640beb38e7.
That commit caused performance degradtion in Quicksilver test QS:sGPU and a functional test failure in (rocPRIM rocprim.device_segmented_radix_sort).
Reverting until we have a better solution to s_cselect_b64 codegen cleanup
Change-Id: Ibf8e397df94001f248fba609f072088a46abae08
Reviewed By: kzhuravl
Differential Revision: https://reviews.llvm.org/D115960
Change-Id: Id169459ce4dfffa857d5645a0af50b0063ce1105
Main use of these is in the standard library, where they generally clutter up
the index.
Certain macros are also common, we don't touch indexing of macros in this patch.
Differential Revision: https://reviews.llvm.org/D115301
Because declarators nest inside-out, we logically need to claim tokens for
parent declarators logically before child ones.
This is the ultimate reason we had problems with DeclaratorDecl, ArrayType etc.
However actually changing the order of traversal is hard, especially for nodes
that have both declarator and non-declarator children.
Since there's only a few TypeLocs corresponding to declarators, we just
have them claim the exact tokens rather than rely on nesting.
This fixes handling of complex declarators, like
`int (*Fun(OuterT^ype))(InnerType);`.
This avoids the need for the DeclaratorDecl early-claim hack, which is
removed.
Unfortunately the DeclaratorDecl early-claims were covering up an AST
anomaly around CXXConstructExpr, so we need to fix that up too.
Based on D116623 and D116618
Differential Revision: https://reviews.llvm.org/D116630
The check should not trigger on lvalue/rvalue overload pairs:
```
struct S {
S(const A& a) : a(a) {}
S(A&& a) : a(std::move(a)) {}
A a;
}
```
Differential Revision: https://reviews.llvm.org/D116535
It's reasonable to want to use the command from one file to compile another.
In particular, the command from a translation unit to parse a related header:
{"file": "foo.h", "command": "clang foo.cpp"}
This is largely what InterpolatingCompilationDatabase tries to do.
To do this correctly can require nontrivial changes to the argv, because the
file extension affects semantics. e.g. here we must add "-x c++header".
When external tools compile commands for different files, we should apply the
same adjustments. This is better than telling people to "fix their tools":
- simple e.g. python scripts shouldn't have to interpret clang argv
- this is a good way to represent the intent "parse header X in the context of
file Y", which can work even if X is not self-contained. clangd does not
support this today, but some other tools do, and we may one day.
This issue is discussed in https://github.com/clangd/clangd/issues/519
Differential Revision: https://reviews.llvm.org/D116167
The "parameter list" is the list of fields which should be initialized.
We introduce a new OverloadCandidate kind for this.
It starts to become harder for CC consumers to handle all the cases for
params, so I added some extra APIs on OverloadCandidate to abstract them.
Includes some basic support for designated initializers.
The same aggregate signature is shown, the current arg jumps after the
one you just initialized. This follows C99 semantics for mixed
designated/positional initializers (which clang supports in C++ as an extension)
and is also a useful prompt for C++ as C++ designated initializers must be
in order.
Related bugs:
- https://github.com/clangd/clangd/issues/965
- https://github.com/clangd/clangd/issues/306
Differential Revision: https://reviews.llvm.org/D116326
We want to deal with non-default constructors that just happen to
contain constant initializers. There was already a negative test case,
it is now a positive one. We find and refactor this case:
struct PositiveNotDefaultInt {
PositiveNotDefaultInt(int) : i(7) {}
int i;
};
Previously, it was in canSafelySkipNode, which is only used to decide
whether we should descend into it and its children, and we still used
the incomplete Decltypeloc.getSourceRange() to claim tokens, which will
cause some tokens were not claimed correctly.
Separate a change of https://reviews.llvm.org/D116536
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D116586
This involves separating out the concepts of "which tokens should we
descend into this node for" vs "which tokens should this node claim".
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D116218
Implementation is based on the "expected type" as used for
designated-initializers in braced init lists. This means it can deduce the type
in some cases where it's not written:
void foo(Widget);
foo({ /*help here*/ });
Only basic constructor calls are in scope of this patch, excluded are:
- aggregate initialization (no help is offered for aggregates)
- initializer_list initialization (no help is offered for these constructors)
Fixes https://github.com/clangd/clangd/issues/306
Differential Revision: https://reviews.llvm.org/D116317
There are some limitations here, so this is behind a flag for now (in addition
to the config setting for the overall feature).
- symbols without exactly one associated header aren't handled right
- no macro support
- referencing std::size_t usually doesn't leave any trace in the AST that the
alias in std was used, so we associate with stddef.h instead of cstddef.
(An AST issue not specific to stdlib, but much worse there)
Differential Revision: https://reviews.llvm.org/D114077
This mechanism is used almost exclusively to enable extra warnings in clang-tidy
using ExtraArgs=-Wfoo, Checks="clang-diagnostic-foo".
Its presence is a strong signal that these flags are useful.
We choose not to actually emit them as clang-tidy diagnostics, but under their
"main" name - this ensures we show the same diagnostic in a consistent way.
We don't add the ExtraArgs to the compile command in general, but rather just
handle the -W<group> flags, which is the common case and avoids unexpected
side-effects.
And we only do this for the main file parse, when producing diagnostics.
Differential Revision: https://reviews.llvm.org/D116147
Provide signature while typing template arguments: Foo< ^here >
Here the parameters are e.g. "typename x", and the result type is e.g.
"struct" (class template) or "int" (variable template) or "bool (std::string)"
(function template).
Multiple overloads are possible when a template name is used for several
overloaded function templates.
Fixes https://github.com/clangd/clangd/issues/299
Differential Revision: https://reviews.llvm.org/D116352
This reverts commit cc56c66f27.
Fixed a bad assertion, the target of a UsingShadowDecl must not have
*local* qualifiers, but it can be a typedef whose underlying type is qualified.
This preserves all the results we've processed already rather than
throwing them away in the end.
It has some performance implications on the edge cases, in the worst case we
might issue 1 relations and 2 xrefs requests in extra to deduce `HasMore`
correctly.
Fixes https://github.com/clangd/clangd/issues/204.
Differential Revision: https://reviews.llvm.org/D116043
Currently there's no way to find the UsingDecl that a typeloc found its
underlying type through. Compare to DeclRefExpr::getFoundDecl().
Design decisions:
- a sugar type, as there are many contexts this type of use may appear in
- UsingType is a leaf like TypedefType, the underlying type has no TypeLoc
- not unified with UnresolvedUsingType: a single name is appealing,
but being sometimes-sugar is often fiddly.
- not unified with TypedefType: the UsingShadowDecl is not a TypedefNameDecl or
even a TypeDecl, and users think of these differently.
- does not cover other rarer aliases like objc @compatibility_alias,
in order to be have a concrete API that's easy to understand.
- implicitly desugared by the hasDeclaration ASTMatcher, to avoid
breaking existing patterns and following the precedent of ElaboratedType.
Scope:
- This does not cover types associated with template names introduced by
using declarations. A future patch should introduce a sugar TemplateName
variant for this. (CTAD deduced types fall under this)
- There are enough AST matchers to fix the in-tree clang-tidy tests and
probably any other matchers, though more may be useful later.
Caveats:
- This changes a fairly common pattern in the AST people may depend on matching.
Previously, typeLoc(loc(recordType())) matched whether a struct was
referred to by its original scope or introduced via using-decl.
Now, the using-decl case is not matched, and needs a separate matcher.
This is similar to the case of typedefs but nevertheless both adds
complexity and breaks existing code.
Differential Revision: https://reviews.llvm.org/D114251
SymbolAndSignals stores SymbolInfo which stores two std::strings. Then
the values are stored in a llvm::DenseMap<llvm::StringRef, double>. When
the sorting is happening, SymbolAndSignals are swapped and thus because
of small string optimization some strings may become invalid. This
results in incorrect ranking.
This was detected when running new std::sort algorithm against llvm
toolchain. This could have been prevented with running llvm::sort and
EXPENSIVE_CHECKS. Unfortunately, no sanitizer yelled.
I don't have commit rights, kutdanila@yandex.ru Danila Kutenin
Reviewed By: bkramer
Differential Revision: https://reviews.llvm.org/D116037
`Message()` lambda uses `Reason.Details` as an input parameter for `llvm::formatv()`, but `Reason` in `Message()` is a local object.
Return value of `llvm::formatv()` contains references to its input arguments, thus `Message()` returns an object which contains a reference to `Details` field of the local object `Reason`.
This patch fixes this behavior by passing `Reason` as a reference to `Message()` to ensure that return value of `Message()` contains references to alive object and also prevents copying of `InvalidName` structure at passing it to `makeError()`.
Provided test passes on Linux+GCC with or without this patch, but fails on Windows+VisualStudio without this patch.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D115959
This change adds an option to disable warnings from the
cppcoreguidelines-narrowing-conversions check on integer to floating-
point conversions which may be narrowing.
An example of a case where this might be useful:
```
std::vector<double> v = {1, 2, 3, 4};
double mean = std::accumulate(v.cbegin(), v.cend(), 0.0) / v.size();
```
The conversion from std::size_t to double is technically narrowing on
64-bit systems, but v almost certainly does not have enough elements
for this to be a problem.
This option would allow the cppcoreguidelines-narrowing-conversions
check to be enabled on codebases which might otherwise turn it off
because of cases like the above.
The purpose of this checker is to flag a missing throw keyword, and does so by checking for the construction of an exception class that is then unused.
This works great except that placement new expressions are also flagged as those lead to the construction of an object as well, even though they are not temporary (as that is dependent on the storage).
This patch fixes the issue by exempting the match if it is within a placement-new.
Fixes https://github.com/llvm/llvm-project/issues/51939
Differential Revision: https://reviews.llvm.org/D115576
This unifies the behaviour we have in code completion item
documentations and signaturehelp. Providing better line wrapping and detection
of inline code blocks in comments to be renedered appropriately in markdown.
Differential Revision: https://reviews.llvm.org/D115442
This change applies two fixes to the abseil-cleanup-ctad check. It uses hasSingleDecl() to ensure only declStmt()s with one varDecl() are matched (leaving compount declStmt()s unchanged). It also addresses a bug in the handling of comments that surround the absl::MakeCleanup() calls by switching to the callArgs() combinator from Clang Transformer.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D115452
This commit improves the fix-its of modernize-pass-by-value by
no longer proposing partial fixes. In the presence of using/typedef,
we failed to rewrite the function signature but still adjusted the
function body. This led to incorrect, partial fix-its. Instead, the
check now simply doesn't offer any fixes at all in such a situation.
D114072 allows filtering out the warnings for headers behind `// IWYU pragma:
keep`. This is the first step towards more useful IWYU pragmas support and
fine-grained control over the IncludeCleaner warnings.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D115345
Clang doesn't offer these fixes I guess for a couple of reasons:
- where to insert includes is a formatting concern, and clang shouldn't
depend on clang-format
- the way clang prints diagnostics, we'd show a bunch of basically irrelevant
context of "this is where we'd want to insert the include"
Maybe it's possible to hack around 1, but 2 is still a concern.
Meanwhile, bolting this onto include-fixer gets the job done.
Fixes https://github.com/clangd/clangd/issues/355
Fixes https://github.com/clangd/clangd/issues/937
Differential Revision: https://reviews.llvm.org/D114667
This will allow the IncludeCleaner to suppress warnings on the lines with "IWYU
pragma: keep".
Clang APIs are not very convinient, so the code has to navigate around it.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D114072
Add desugared type to hover when the desugared type and the pretty-printed type are different.
```c++
template<typename T>
struct TestHover {
using Type = T;
};
int main() {
TestHover<int>::Type a;
}
```
```
variable a
Type: TestHover<int>::Type (aka int)
```
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D114522
These are the trigrams for queries right now:
- "va" -> {Trigram("va")}
- "va_" -> {} (empty)
This is suboptimal since the resulting query will discard the query information
and return all symbols, some of which will be later be scored expensively
(fuzzy matching score). This is related to
https://github.com/clangd/clangd/issues/39 but does not fix it. Accidentally,
because of that incorrect behavior, when user types "tok::va" there are no
results (the issue is that `tok::kw___builtin_va_arg` does not have "va" token)
but when "tok::va_" is typed, expected result (`tok::kw___builtin_va_arg`)
shows up by accident. This is because the dex query transformer will only
lookup symbols within the `tok::` namespace. There won't be many, so the
returned results will contain symbol we need; this symbol will be filtered out
by the expensive checks and that will be displayed in the editor.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D113995
WG14 adopted the _ExtInt feature from Clang for C23, but renamed the
type to be _BitInt. This patch does the vast majority of the work to
rename _ExtInt to _BitInt, which accounts for most of its size. The new
type is exposed in older C modes and all C++ modes as a conforming
extension. However, there are functional changes worth calling out:
* Deprecates _ExtInt with a fix-it to help users migrate to _BitInt.
* Updates the mangling for the type.
* Updates the documentation and adds a release note to warn users what
is going on.
* Adds new diagnostics for use of _BitInt to call out when it's used as
a Clang extension or as a pre-C23 compatibility concern.
* Adds new tests for the new diagnostic behaviors.
I want to call out the ABI break specifically. We do not believe that
this break will cause a significant imposition for early adopters of
the feature, and so this is being done as a full break. If it turns out
there are critical uses where recompilation is not an option for some
reason, we can consider using ABI tags to ease the transition.
Currently it's hidden inside ClangTidyDiagnosticConsumer,
so it's hard to know it exists.
Given that there are multiple uses of globs in clang-tidy,
it makes sense to have these classes publicly available
for other use cases that might benefit from it.
Also, add unit test by converting the existing tests
for GlobList into typed tests.
Reviewed By: salman-javed-nz
Differential Revision: https://reviews.llvm.org/D113422
This makes IncludeCleaner more useful in the presense of a large number of
forward declarations. If the definition is already in the Translation Unit and
visible to the Main File, forward declarations have no effect.
The original patch D112707 was split in two: D114864 and this one.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D114949
Using XCTAssertEqual on NSString* objects is almost always wrong.
Unfortunately, we have seen a lot of tests doing this and reyling on pointer equality for strings with the same values (which happens to work sometimes - depending on the linker, but this assumption is not guaranteed by the language)
These fixes would make tests less brittle.
Differential Revision: https://reviews.llvm.org/D114975
This patch parameterizes the clang-tidy diagnostic consumer with a boolean that
controls whether to honor NOLINTBEGIN/NOLINTEND blocks. The current support for
scanning these blocks is very costly -- O(n*m) in the size of files (n) and
number of diagnostics found (m), with a large constant factor. So, the patch
allows clients to disable it.
Future patches should make the feature more efficient, but this will mitigate in
the interim.
Differential Revision: https://reviews.llvm.org/D114981
Renaming header guards to match the LLVM convention.
This patch was created by automatically applying the fixes from
clang-tidy.
I've removed the [NFC] tag from the title, as we're adding header guards in some files and thus might trigger behavior changes.
Differential Revision: https://reviews.llvm.org/D113896
Checks for various ways that the `const CharT*` constructor of `std::basic_string_view` can be passed a null argument and replaces them with the default constructor in most cases. For the comparison operators, braced initializer list does not compile so instead a call to `.empty()` or the empty string literal are used, where appropriate.
This prevents code from invoking behavior which is unconditionally undefined. The single-argument `const CharT*` constructor does not check for the null case before dereferencing its input. The standard is slated to add an explicitly-deleted overload to catch some of these cases: wg21.link/p2166
https://reviews.llvm.org/D114823 is a companion change to prevent duplicate warnings from the `bugprone-string-constructor` check.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D113148
This will mark more headers that are unrelated to used symbol but contain its
forawrd declaration. E.g. the following are examples of headers forward
declaring `llvm::StringRef`:
- clang/include/clang/Basic/Cuda.h
- llvm/include/llvm/Support/SHA256.h
- llvm/include/llvm/Support/TrigramIndex.h
- llvm/include/llvm/Support/RandomNumberGenerator.
- ... and more (~50 in total)
This patch is a reduced version of D112707 which was controversial.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D114864
Fixes PR#47614. Deduction guides, implicit or user-defined, look like
function declarations in the AST. They aren't really functions, though,
and they always have a trailing return type, so it doesn't make sense
to issue this warning for them.
Up until now, all references to `errno` were marked with `NOLINT`, since
it was technically calling an external function. This fixes the lint
rules so that `errno`, as well as `malloc`, `calloc`, `realloc`, and
`free` are all allowed to be called as external functions. All of the
relevant `NOLINT` comments have been removed, and the documentation has
been updated.
Reviewed By: sivachandra, lntue, aaron.ballman
Differential Revision: https://reviews.llvm.org/D113946
The google-readability-casting check is meant to be on par
with cpplint's readability/casting check, according to the
documentation. However it currently does not diagnose
functional casts, like:
float x = 1.5F;
int y = int(x);
This is detected by cpplint, however, and the guidelines
are clear that such a cast is only allowed when the type
is a class type (constructor call):
> You may use cast formats like `T(x)` only when `T` is a class type.
Therefore, update the clang-tidy check to check this
case.
Differential Revision: https://reviews.llvm.org/D114427
Fixes https://bugs.llvm.org/show_bug.cgi?id=48613.
llvm-header-guard is suggesting header guards with leading underscores
if the header file path begins with a '/' or similar special character.
Only reserved identifiers should begin with an underscore.
Differential Revision: https://reviews.llvm.org/D114149
Bitfields are special. Due to integral promotion [conv.prom/5] bitfield
member access expressions are frequently wrapped by an implicit cast to
`int` if that type can represent all the values of the bitfield.
Consider these examples:
struct SmallBitfield { unsigned int id : 4; };
x.id & 1; (case-1)
x.id & 1u; (case-2)
x.id << 1u; (case-3)
(unsigned)x.id << 1; (case-4)
Due to the promotion rules, we would get a warning for case-1. It's
debatable how useful this is, but the user at least has a convenient way
of //fixing// it by adding the `u` unsigned-suffix to the literal as
demonstrated by case-2. However, this won't work for shift operators like
the one in case-3. In case of a normal binary operator, both operands
contribute to the result type. However, the type of the shift expression is
the promoted type of the left operand. One could still suppress this
superfluous warning by explicitly casting the bitfield member access as
case-4 demonstrates, but why? The compiler already knew that the value from
the member access should safely fit into an `int`, why do we have this
warning in the first place? So, hereby we suppress this specific scenario,
when a bitfield's value is implicitly cast to int (likely due to integral
promotion).
Note that the bitshift operation might invoke unspecified/undefined
behavior, but that's another topic, this checker is about detecting
conversion-related defects.
Example AST for `x.id << 1`:
BinaryOperator 'int' '<<'
|-ImplicitCastExpr 'int' <IntegralCast>
| `-ImplicitCastExpr 'unsigned int' <LValueToRValue>
| `-MemberExpr 'unsigned int' lvalue bitfield .id
| `-DeclRefExpr 'SmallBitfield' lvalue ParmVar 'x' 'SmallBitfield'
`-IntegerLiteral 'int' 1
Reviewed By: courbet
Differential Revision: https://reviews.llvm.org/D114105
Invalid SourceRanges can occur generally if the code does not compile,
thus we expect clang error diagnostics.
Unlike `clang`, `clang-tidy` did not swallow invalid source ranges, but
tried to highlight them, and blow various assertions.
The following two examples produce invalid source ranges, but this is
not a complete list:
void test(x); // error: unknown type name 'x'
struct Foo {
member; // error: C++ requires a type specifier for all declarations
};
Thanks @whisperity helping me fix this.
Reviewed-By: xazax.hun
Differential Revision: https://reviews.llvm.org/D114254
The `cppcoreguidelines-virtual-base-class-destructor` checker crashed on
this example:
#define DECLARE(CLASS) \
class CLASS { \
protected: \
virtual ~CLASS(); \
}
DECLARE(Foo); // no-crash
The checker will hit the following assertion:
clang-tidy: llvm/include/llvm/ADT/Optional.h:196: T &llvm::optional_detail::OptionalStorage<clang::Token, true>::getValue() & [T = clang::Token]: Assertion `hasVal' failed."
It turns out, `Lexer::findNextToken()` returned `llvm::None` within the
`getVirtualKeywordRange()` function when the `VirtualEndLoc`
SourceLocation represents a macro expansion.
To prevent this from happening, I decided to propagate the `llvm::None`
further up and only create the removal of `virtual` if the
`getVirtualKeywordRange()` succeeds.
I considered an alternative fix for this issue:
I could have checked the `Destructor.getLocation().isMacroID()` before
doing any Fixit calculation inside the `check()` function.
In contrast to this approach my patch will preserve the diagnostics and
drop the fixits only if it would have crashed.
Reviewed By: whisperity
Differential Revision: https://reviews.llvm.org/D113558
When a symbol comes from the non self-contained header, we recursively uplift
the file we consider used to the first includer that has a header guard. We
need to do this while we still have FileIDs because every time a non
self-contained header is included, it gets a new FileID but is later
deduplicated by HeaderID and it's not possible to understand where it was
included from.
Based on D114370.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D114623
[NFC] As part of using inclusive language within the llvm project, this patch
replaces master with main in `SubModule2.h`.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D114100
This fixes "textDocument/prepareCallHierarchy" in clangd for ObjC methods. Details at https://github.com/clangd/vscode-clangd/issues/247.
clangd uses Decl::isFunctionOrFunctionTemplate to check if the decl given in a prepareCallHierarchy request is eligible for prepareCallHierarchy. We change to use isFunctionOrMethod which includes functions and ObjC methods.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D114058
`isConstRefReturningMethodCall` should be considering
`CXXOperatorCallExpr` in addition to `CXXMemberCallExpr`. Clang considers
these to be distinct (`CXXOperatorCallExpr` derives from `CallExpr`, not
`CXXMemberCallExpr`), but we don't care in the context of this
check.
This is important because of
`std::vector<Expensive>::operator[](size_t) const`.
Differential Revision: https://reviews.llvm.org/D114249
The configuration may kick off indexing, which may involve sending LSP
messages.
The crash is fiddly to reproduce in a hermetic test (we need background
indexing on without disk storage, and to handle server->client messages
in LSPClient...)
Fixes https://github.com/clangd/clangd/issues/926
Overriding methods should not get a readability-identifier-naming
warning because the issue can only be fixed in the base class; but the
current check for whether a method is overriding does not take the
override attribute into account.
Differential Revision: https://reviews.llvm.org/D113830
The clang-tidy/infrastructure/pr37091.cpp test inherits the top-level .clang-tidy configuration because it doesn't specify its own checks. It'd be a more stable test if it operates independently of the top-level .clang-tidy settings.
I've made the clang-tidy/infrastructure/pr37091.cpp test independent of the top-level .clang-tidy (picked an arbitrary check that I saw another clang-tidy/infrastructure test was also using: clang-tidy/infrastructure/temporaries.cpp)
Reviewed By: kbobyrev
Differential Revision: https://reviews.llvm.org/D114034
This covers both C-style variadic functions and template variadic w/
parameter packs.
Previously we would return no signatures when working with template
variadic functions once activeParameter reached the position of the
parameter pack (except when it was the only param, then we'd still
show it when no arguments were given). With this commit, we now show
signathure help correctly.
Additionally, this commit fixes the activeParameter value in LSP output
of clangd in the presence of variadic functions (both kinds). LSP does
not allow the activeParamter to be higher than the number of parameters
in the active signature. With "..." or parameter pack being just one
argument, for all but first argument passed to "..." we'd report
incorrect activeParameter value. Clients such as VSCode would then treat
it as 0, as suggested in the spec) and highlight the wrong parameter.
In the future, we should add support for per-signature activeParamter
value, which exists in LSP since 3.16.0. This is not part of this
commit.
Differential Revision: https://reviews.llvm.org/D111318
This is a cleanup of the only llvm-prefer-isa-or-dyn-cast-in-conditionals finding in the clangd code base. This patch was created by automatically applying the fixes from clang-tidy.
Differential Revision: https://reviews.llvm.org/D113899
This will drop file version information from span names, reducing
overall cardinality and also effect logging when skipping actions in scheduler.
Differential Revision: https://reviews.llvm.org/D113390
Cleanup of clang-tidy findings: removing "else" after a return statement
to improve readability of the code.
This patch was created by applying the clang-tidy fixes automatically.
Differential Revision: https://reviews.llvm.org/D113892
The overload shouldSuppressDiagnostic seems unnecessary, and it is only
used in clangd.
This patch removes it and use the real one (suppression diagnostics are
discarded in clangd at the moment).
Fixes https://github.com/clangd/clangd/issues/929
Differential Revision: https://reviews.llvm.org/D113999
Cleaning up unused "using" declarations.
This patch was generated from automatically applyning clang-tidy fixes.
Differential Revision: https://reviews.llvm.org/D113891
Fixes PR#52400. The tests for bugprone-throw-keyword-missing actually
already contain exceptions as class members, but not as members with
initializers, which was probably just an oversight.
This implements the following changes:
* AutoType retains sugared deduced-as-type.
* Template argument deduction machinery analyses the sugared type all the way
down. It would previously lose the sugar on first recursion.
* Undeduced AutoType will be properly canonicalized, including the constraint
template arguments.
* Remove the decltype node created from the decltype(auto) deduction.
As a result, we start seeing sugared types in a lot more test cases,
including some which showed very unfriendly `type-parameter-*-*` types.
Signed-off-by: Matheus Izvekov <mizvekov@gmail.com>
Reviewed By: rsmith, #libc, ldionne
Differential Revision: https://reviews.llvm.org/D110216
modernize-loop-convert checks and fixes when a loop that iterates over the
elements of a container can be rewritten from a for(...; ...; ...) style into
the "new" C++11 for-range format. For that, it needs to parse the elements of
that loop, like its init-statement, such as ItType it = cont.begin().
modernize-loop-convert checks whether the loop variable is initialized by a
begin() member function.
When an iterator is initialized with a conversion operator (e.g. for
(const_iterator it = non_const_container.begin(); ...), attempts to retrieve the
name of the initializer expression resulted in an assert, as conversion
operators don't have a valid IdentifierInfo.
I fixed this by making digThroughConstructors dig through conversion operators
as well.
Differential Revision: https://reviews.llvm.org/D113201
Fixes PR#38187. Constructors are actually already checked,
but only as functions, i.e. the check only looks at the
constructor body and not at the initializers, which misses
the (common) case where constructor parameters are moved
as part of an initializer expression.
One remaining false negative is when both the move //and//
the use-after-move occur in constructor initializers.
This is a lot more difficult to handle, though, because
the `bugprone-use-after-move` check is currently based on
a CFG that only takes the body into account, not the
initializers, so e.g. initialization order would have to
manually be considered. I will file a follow-up issue for
this once PR#38187 is closed.
Reviewed By: carlosgalvezp
Differential Revision: https://reviews.llvm.org/D113708
This implements the following changes:
* AutoType retains sugared deduced-as-type.
* Template argument deduction machinery analyses the sugared type all the way
down. It would previously lose the sugar on first recursion.
* Undeduced AutoType will be properly canonicalized, including the constraint
template arguments.
* Remove the decltype node created from the decltype(auto) deduction.
As a result, we start seeing sugared types in a lot more test cases,
including some which showed very unfriendly `type-parameter-*-*` types.
Signed-off-by: Matheus Izvekov <mizvekov@gmail.com>
Reviewed By: rsmith
Differential Revision: https://reviews.llvm.org/D110216
- Jaro–Winkler and Sørensen–Dice should use en-dashes not regular
dashes. In reStructuredText this is typed as `--`.
- Letters at the beginning of a sentence should be capitalized.
This implements the following changes:
* AutoType retains sugared deduced-as-type.
* Template argument deduction machinery analyses the sugared type all the way
down. It would previously lose the sugar on first recursion.
* Undeduced AutoType will be properly canonicalized, including the constraint
template arguments.
* Remove the decltype node created from the decltype(auto) deduction.
As a result, we start seeing sugared types in a lot more test cases,
including some which showed very unfriendly `type-parameter-*-*` types.
Signed-off-by: Matheus Izvekov <mizvekov@gmail.com>
Reviewed By: rsmith
Differential Revision: https://reviews.llvm.org/D110216
The fixes from the YAML file can refer to relative paths.
Those relative paths are meant to be resolved relative to the
corresponding `build directory`.
However, `clang-apply-replacements` currently interprets all
paths relative to its own working directory. This causes issues,
e.g., when `clang-apply-replacements` is run from outside of
the original build directory.
This commit adjusts `clang-apply-replacements` to take the build
directory into account when resolving relative file paths.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D112647
The call to getTypeSizeInChars() is replaced with
getTypeSizeInCharsIfKnown(), which does not crash on forward declared
structs. This only affects printing.
Differential Revision: https://reviews.llvm.org/D113570
Detect when an identifier contains some Right-To-Left characters.
This pass relates to https://trojansource.codes/
Example of misleading source:
short int א = (short int)0;
short int ג = (short int)12345;
int main() {
int א = ג; // a local variable, set to zero?
printf("ג is %d\n", ג);
printf("א is %d\n", א);
}
This is a recommit of 299aa4dfa1 with missing
option registration fixed.
Differential Revision: https://reviews.llvm.org/D112914
Fixes pr40372 (https://bugs.llvm.org/show_bug.cgi?id=40372).
The llvm-header-guard check does not take into account that the path
separator on Windows is `\`, not `/`.
This means that instead of suggesting a header guard in the form of:
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FOO_H
it incorrectly suggests:
C:\LLVM_PROJECT\CLANG_TOOLS_EXTRA\CLANG_TIDY\FOO_H
Differential Revision: https://reviews.llvm.org/D113450
This reverts commit 7f92a1a84b.
It triggers an assert, see http://45.33.8.238/linux/60293/step_9.txt
"AST/Decl.h:277: llvm::StringRef clang::NamedDecl::getName() const: Assertion `Name.isIdentifier() && "Name is not a simple identifier"' failed."
Calling clang-tidy on ClangTidyDiagnosticConsumer.cpp gives a
"unmatched NOLINTBEGIN without a subsequent NOLINTEND" warning.
The "NOLINTBEGIN" and "NOLINTEND" string literals used in the
implementation of `createNolintError()` get mistaken for actual
NOLINTBEGIN/END comments used to suppress clang-tidy warnings.
Rewrite the string literals so that they can no longer be mistaken for
actual suppression comments.
Differential Revision: https://reviews.llvm.org/D113472
Suggests switching the initialization pattern of `absl::Cleanup` instances from the factory function to class template argument deduction (CTAD) in C++17 and higher.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D113195
[NFC] As part of using inclusive language within the llvm project,
this patch replaces master with main when referring to `.chm` files.
Reviewed By: teemperor
Differential Revision: https://reviews.llvm.org/D113299
The files in compile-commands.json can potentially include duplicates.
Change run-clang-tidy.py so that it does not run on the duplicate entries.
Differential Revision: https://reviews.llvm.org/D112926
[NFC] This patch fixes URLs containing "master". Old URLs were either broken or
redirecting to the new URL.
Reviewed By: #libc, ldionne, mehdi_amini
Differential Revision: https://reviews.llvm.org/D113186
I noticed that, while go-to-def works on cases like:
namespace ns {
template<typename T> struct Foo {};
}
using ::ns::Fo^o;
it only works because of the FileIndex. We can get definition location
directly from AST too.
Differential Revision: https://reviews.llvm.org/D113029
Now in libcxx and clang, all the coroutine components are defined in
std::experimental namespace.
And now the coroutine TS is merged into C++20. So in the working draft
like N4892, we could find the coroutine components is defined in std
namespace instead of std::experimental namespace.
And the coroutine support in clang seems to be relatively stable. So I
think it may be suitable to move the coroutine component into the
experiment namespace now.
This patch would make clang lookup coroutine_traits in std namespace
first. For the compatibility consideration, clang would lookup in
std::experimental namespace if it can't find definitions in std
namespace. So the existing codes wouldn't be break after update
compiler.
And in case the compiler found std::coroutine_traits and
std::experimental::coroutine_traits at the same time, it would emit an
error for it.
The support for looking up std::experimental::coroutine_traits would be
removed in Clang16.
Reviewed By: lxfind, Quuxplusone
Differential Revision: https://reviews.llvm.org/D108696
Back in the mists of time, the CXXRecordDecl for the injected-class-name was
a redecl of the outer class itself.
This got changed in 470c454a61, but only for plain
classes: class template instantation was still detecting the injected-class-name
in the template body and marking its instantiation as a redecl.
This causes some subtle inconsistent behavior between the two, e.g.
hasDefinition() returns true for Foo<int>::Foo but false for Bar::Bar.
This is the root cause of PR51912.
Differential Revision: https://reviews.llvm.org/D112765
The CERT rule ERR33-C can be modeled partially by the existing check
'bugprone-unused-return-value'. The existing check is reused with
a fixed set of checked functions.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D112409
Run clang-tidy on all source files under `clang-tools-extra/clang-tidy`
with `-header-filter=clang-tidy.*` and make suggested corrections.
Differential Revision: https://reviews.llvm.org/D112864
Headers without include guards might have side effects or can be the files we
don't want to consider (e.g. tablegen ".inc" files). Skip them when translating
headers to the HeaderIDs that we will consider as unused.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D112695
We're missing all cases where the return value is a type alias.
Unfortunately, this includes things we care about, such as
`std::vector<T>::operator[]` (return value is `const_reference`,
not `const T&`).
Match the canonical type instead.
Differential Revision: https://reviews.llvm.org/D112722
This is important especially for code that tries to traverse scopes as
written in code, which is the contract SelectionTree tries to satisfy.
Differential Revision: https://reviews.llvm.org/D112712
This changes the handling of special buffers (<command-line> etc) that
SourceManager treats as files but FileManager does not.
We now include them in findReferencedFiles() and drop them as part of
translateToHeaderIDs(). This pairs more naturally with the data representations
we're using, and so avoids a bunch of converting between representations for
filtering.
Differential Revision: https://reviews.llvm.org/D112652
Doing otherwise leads to crashing. Way to reproduce: open "gmock/gmock.h" in
the LLVM source tree.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D112608
This replaces the test removed in 51be7061d0
It is more principled and tests more critical cases: a crash while parsing.
We need two pieces of plumbing:
- a way to re-enable the crashing #pragmas via a flag, to test parse crashes
- a bit of reshuffling around ASTWorker execution so that we set up the
crash handler in both sync/async modes.
Sync mode is useful for debugging, so I tested both.
Differential Revision: https://reviews.llvm.org/D112565
This is a temporary hack to disable diagnostics for system headers. As of right
now, IncludeCleaner does not handle the Standard Library correctly and will
report most system headers as unused because very few symbols are defined in
top-level system headers. This will eventually be fixed, but for now we are
aiming for the most conservative approach with as little false-positive
warnings as possible. After the initial prototype and core functionality is
polished, I will turn back to handling the Standard Library as it requires
custom logic.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D112571
The string table `DefaultIgnoredParameterTypeSuffixes` has a typo:
`ForwardIt` is mistyped as `FowardIt`.
Correct typo and add test coverage.
Differential Revision: https://reviews.llvm.org/D112596
Collect the macro definition locations for all the macros used in the main
file.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D112447
Clangd used first token of filename as filename range rather than the
synthezied filename token. Unfortunately the former only contains `"` or `<` in
the raw lexing mode, resulting in wrong range information and breaking tidy
checks that relied on it.
Fixes https://github.com/clangd/clangd/issues/896.
Differential Revision: https://reviews.llvm.org/D112559
We make assumption that:
getDeclForComment(getDeclForComment(X)) == getDeclForComment(X)
but this is not true if you have a template
instantionation of a template instantiation, which is the case when, for
example, you have a <=> operator in a templated class.
This fix makes getDeclForComment() call itself recursively to ensure
this property is always true.
Fixes https://github.com/clangd/clangd/issues/901
Differential Revision: https://reviews.llvm.org/D112527
I was under the impression that `return false;` in the
RecursiveASTVisitor stops the traversal for the subtree but it appears
that it stops the whole tree traversal, so this change introduces a bug
where `ReferencedLocationCrawler` will not collect any symbols past an
enum.
This is a follow-up on D112209.
clang-tidy can be used to statically analyze CUDA code,
thanks to clang being able to compile CUDA code natively.
This makes clang-tidy the one and only open-source
static analyzer for CUDA.
However it currently warns for native CUDA built-in
variables, like threadIdx, due to the way they
are implemented in clang.
Users don't need to know the details of the clang
implementation, and they should continue to write
idiomatic code. Therefore, suppress the warning
if a CUDA built-in variable is encountered.
Fixes https://bugs.llvm.org/show_bug.cgi?id=48758
Motivation:
At the moment it is hard to attribute a clangd crash to a specific request out of all in-flight requests that might be processed concurrently. So before we can act on production clangd crashes, we have to do quite some digging through the log tables populated by our in-house VSCode extension or sometimes even directly reach out to the affected developer. Having all the details needed to reproduce a crash printed alongside its stack trace has a potential to save us quite some time, that could better be spent on fixing the actual problems.
Implementation approach:
* introduce `ThreadCrashReporter` class that allows to set a temporary signal handler for the current thread
* follow RAII pattern to simplify printing context for crashes occurring within a particular scope
* hold `std::function` as a handler to allow capturing context to print
* set local `ThreadCrashReporter` within `JSONTransport::loop()` to print request JSON for main thread crashes, and in `ASTWorker::run()` to print the file paths, arguments and contents for worker thread crashes
`ThreadCrashReporter` currently allows only one active handler per thread, but the approach can be extended to support stacked handlers printing context incrementally.
Example output for main thread crashes:
```
...
#15 0x00007f7ddc819493 __libc_start_main (/lib64/libc.so.6+0x23493)
#16 0x000000000249775e _start (/home/emmablink/local/llvm-project/build/bin/clangd+0x249775e)
Signalled while processing message:
{"jsonrpc": "2.0", "method": "textDocument/didOpen", "params": {"textDocument": {"uri": "file:///home/emmablink/test.cpp", "languageId": "cpp", "version": 1, "text": "template <typename>\nclass Bar {\n Bar<int> *variables_to_modify;\n foo() {\n for (auto *c : *variables_to_modify)\n delete c;\n }\n};\n"}}}
```
Example output for AST worker crashes:
```
...
#41 0x00007fb18304c14a start_thread pthread_create.c:0:0
#42 0x00007fb181bfcdc3 clone (/lib64/libc.so.6+0xfcdc3)
Signalled during AST action:
Filename: test.cpp
Directory: /home/emmablink
Command Line: /usr/bin/clang -resource-dir=/data/users/emmablink/llvm-project/build/lib/clang/14.0.0 -- /home/emmablink/test.cpp
Version: 1
Contents:
template <typename>
class Bar {
Bar<int> *variables_to_modify;
foo() {
for (auto *c : *variables_to_modify)
delete c;
}
};
```
Testing:
The unit test covers the thread-localitity and nesting aspects of `ThreadCrashReporter`. There might be way to set up a lit-based integration test that would spawn clangd, send a message to it, signal it immediately and check the standard output, but this might be prone to raceconditions.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D109506
Based on post-commit review discussion on
2bd8493847 with Richard Smith.
Other uses of forcing HasEmptyPlaceHolder to false seem OK to me -
they're all around pointer/reference types where the pointer/reference
token will appear at the rightmost side of the left side of the type
name, so they make nested types (eg: the "int" in "int *") behave as
though there is a non-empty placeholder (because the "*" is essentially
the placeholder as far as the "int" is concerned).
This was originally committed in 277623f4d5
Reverted in f9ad1d1c77 due to breakages
outside of clang - lldb seems to have some strange/strong dependence on
"char [N]" versus "char[N]" when printing strings (not due to that name
appearing in DWARF, but probably due to using clang to stringify type
names) that'll need to be addressed, plus a few other odds and ends in
other subprojects (clang-tools-extra, compiler-rt, etc).
Includer cache could get into a bad state when a main file went bad and
added back afterwards. This patch adds a check to invalidate to prevent
that.
Differential Revision: https://reviews.llvm.org/D112130
To simplify suppressing warnings (for example, for
when multiple check aliases are enabled).
The globbing format reuses the same code as for
globbing when enabling checks, so the semantics
and behavior is identical.
Differential Revision: https://reviews.llvm.org/D111208
For example, if you have:
void foo(int bar);
foo(/*^
it should auto-complete to "bar=".
Because Sema callbacks for code completion in comments happen before we
have an AST we need to cheat in clangd by detecting completion on /*
before, moving cursor back by two characters, then running a simplified
verion of SignatureHelp to extract argument name(s) from possible
overloads.
Differential Revision: https://reviews.llvm.org/D110823
Incorrectly triggers for template classes that inherit
from a base class that has virtual destructor.
Any class inheriting from a base that has a virtual destructor
will have their destructor also virtual, as per the Standard:
https://timsong-cpp.github.io/cppwp/n4140/class.dtor#9
> If a class has a base class with a virtual destructor,
> its destructor (whether user- or implicitly-declared) is virtual.
Added unit tests to prevent regression.
Fixes bug https://bugs.llvm.org/show_bug.cgi?id=51912
Differential Revision: https://reviews.llvm.org/D110614
If the Node has an invalid location, it will trigger assert in
isInSystemHeader(...).
void test() {
__builtin_va_list __args;
// __builtin_va_list has no defination in any source file and its
// CXXConstructorDecl has invalid sourcelocation
}
coredump with "Assertion `Loc.isValid() && "Can't get file
characteristic of invalid loc!"' failed." in
getFileCharacteristic(SourceLocation).
Previously we would call getAsTemplate() when kind == TemplateExpansion,
which triggers an assertion. The call is now replaced with
getAsTemplateOrTemplatePattern(), which is exactly the same as
getAsTemplate(), except it allows calls when kind == TemplateExpansion.
No change in behavior for no-assert builds.
Differential Revision: https://reviews.llvm.org/D111648
The list of checked functions was incomplete in the description.
Reviewed By: aaron.ballman, steakhal
Differential Revision: https://reviews.llvm.org/D111623
This requirement was introduced in the C++ Core guidelines in 2016:
1894380d0a
Then clang-tidy got updated to comply with the rule.
However in 2019 this decision was reverted:
5fdfb20b76
Therefore we need to apply the correct configuration to
clang-tidy again.
This also makes this cppcoreguidelines check consistent
with the other 2 alias checks: hicpp-use-override and
modernize-use-override.
Additionally, add another RUN line to the unit test,
to make sure cppcoreguidelines-explicit-virtual-functions
is tested.
As described on D111049, we're trying to remove the <string> dependency from error handling and replace uses of report_fatal_error(const std::string&) with the Twine() variant which can be forward declared.
Previously, the code in add_new_check.py that looks for fixit keywords in check source files when generating list.rst assumed that the script would only be called from its own path. That means it doesn't find any source files for the checks it's attempting to scan for, and it defaults to writing out nothing in the "Offers fixes" column for all checks. Other parts of add_new_check.py work from other paths, just not this part.
After this fix, add_new_check.py's "offers fixes" column generation for list.rst will be consistent regardless of what path it's called from by using the caller path that's deduced elsewhere already from sys.argv[0].
Reviewed By: kbobyrev
Differential Revision: https://reviews.llvm.org/D110600
Addresses https://github.com/clangd/clangd/issues/881
Includes refs of base class method in refs of derived class method.
Previously we reported base class method's refs only for decl of derived
class method. Ideally this should work for all usages of derived class method.
Related patch:
fbeff2ec2b.
Differential Revision: https://reviews.llvm.org/D111039
- Support enums in C and ObjC as their
AST representations differ slightly.
- Add support for typedef'ed enums.
Differential Revision: https://reviews.llvm.org/D110954
Stop using APInt constructors and methods that were soft-deprecated in
D109483. This fixes all the uses I found in clang.
Differential Revision: https://reviews.llvm.org/D110808
References to fields inside anon structs contain an implicit children
for the container, which has the same SourceLocation with the field.
This was resulting in SelectionTree always picking the anon-struct rather than
the field as the selection.
This patch prevents that by claiming the range for the field early.
https://github.com/clangd/clangd/issues/877.
Differential Revision: https://reviews.llvm.org/D110825
Zinovy Nis