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
Fabian Wolff