Summary:
Looks at conditionals and finds cases of ``cast<>``, which will
assert rather than return a null pointer, and ``dyn_cast<>`` where
the return value is not captured. Additionally, finds cases that
match the pattern ``var.foo() && isa<X>(var.foo())``, where the
method is called twice and could be expensive.
.. code-block:: c++
// Finds cases like these:
if (auto x = cast<X>(y)) <...>
if (cast<X>(y)) <...>
// But not cases like these:
if (auto f = cast<Z>(y)->foo()) <...>
if (cast<Z>(y)->foo()) <...>
Reviewers: alexfh, rjmccall, hokein, aaron.ballman, JonasToth
Reviewed By: aaron.ballman
Subscribers: xbolva00, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D59802
llvm-svn: 359142
Summary:
Include insertion in clangd was inserting absolute paths when the
include directory was an absolute path with a double dot. This patch makes sure
double dots are handled both with absolute and relative paths.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60873
llvm-svn: 359078
It now comes with a follow-up fix for the clients of this API
in clangd and clang-tidy.
Differential Revision: https://reviews.llvm.org/D59977
llvm-svn: 359035
Patch by Nathan Ridge!
Dependent bases are handled heuristically, by replacing them with the
class template that they are a specialization of, where possible. Care
is taken to avoid infinite recursion.
Differential Revision: https://reviews.llvm.org/D59756
llvm-svn: 358866
Summary:
It's never set to true. Its only effect would be to set stdout to binary mode.
Hopefully we have better ways of doing this by now :-)
Reviewers: hokein
Subscribers: jkorous, arphaman, kadircet, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D60871
llvm-svn: 358696
Summary: We already have the structure internally, we just need to expose it.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60267
llvm-svn: 358675
Summary:
Also add a test to verify clang-tidy only apply the first alternative
fix.
Reviewers: alexfh
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60857
llvm-svn: 358666
Summary:
Currently we emit an unfriendly "clang diagnostic" message when rename fails. This
patch makes clangd to emit a detailed diagnostic message.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60821
llvm-svn: 358658
Before the patch calling clang-tidy with -header-filter=.* -system-headers would
result in a few hundred useless warnings:
warning: macro '_GNU_SOURCE' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]
warning: macro '_LP64' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]
warning: macro '__ATOMIC_ACQUIRE' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]
warning: macro '__ATOMIC_ACQ_REL' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]
warning: macro '__ATOMIC_CONSUME' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]
warning: macro '__ATOMIC_RELAXED' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]
warning: macro '__ATOMIC_RELEASE' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]
warning: macro '__ATOMIC_SEQ_CST' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]
warning: macro '__BIGGEST_ALIGNMENT__' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]
... and so on
llvm-svn: 358621
Summary:
This check aims to address a relatively common benign error where
Objective-C subclass initializers call -self on their superclass instead
of invoking a superclass initializer, typically -init. The error is
typically benign because libobjc recognizes that improper initializer
chaining is common¹.
One theory for the frequency of this error might be that -init and -self
have the same return type which could potentially cause inappropriate
autocompletion to -self instead of -init. The equal selector lengths and
triviality of common initializer code probably contribute to errors like
this slipping through code review undetected.
This check aims to flag errors of this form in the interests of
correctness and reduce incidence of initialization failing to chain to
-[NSObject init].
[1] "In practice, it will be hard to rely on this function.
Many classes do not properly chain -init calls."
From _objc_rootInit in https://opensource.apple.com/source/objc4/objc4-750.1/runtime/NSObject.mm.auto.html.
Test Notes:
Verified via `make check-clang-tools`.
Subscribers: mgorny, xazax.hun, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59806
llvm-svn: 358620
Summary:
- for warnings, use the flag the warning is controlled by (-Wfoo)
- for errors, keep using the internal name (there's nothing better) but
drop the err_ prefix
This comes at the cost of uniformity, it's no longer totally obvious
exactly what the code field contains. But the -Wname flags are so much
more useful to end-users than the internal warn_foo that this seems worth it.
Reviewers: kadircet
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60822
llvm-svn: 358611
Summary:
Typically used with umbrella headers, e.g. GTK:
#if !defined (__GTK_H_INSIDE__) && !defined (GTK_COMPILATION)
#error "Only <gtk/gtk.h> can be included directly."
#endif
Heuristic is fairly conservative, a quick code search over github showed
a fair number of hits and few/no false positives. (Not all were umbrella
headers, but I'd be happy avoiding include insertion for all of them).
We may want to relax the heuristic later to catch more cases.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60815
llvm-svn: 358605
Before this patch readability-misleading-indentation could issue diagnostics
with an invalid location, which would lead to an assertion failure in
ClangTidyContext::diag()
llvm-svn: 358589
Summary:
Motivation/Context: in the code review system integrating with clang-tidy,
clang-tidy doesn't provide a human-readable description of the fix. Usually
developers have to preview a code diff (before vs after apply the fix) to
understand what the fix does before applying a fix.
This patch proposes that each clang-tidy check provides a short and
actional fix description that can be shown in the UI, so that users can know
what the fix does without previewing diff.
This patch extends clang-tidy framework to support fix descriptions (will add implementations for
existing checks in the future). Fix descriptions and fixes are emitted via diagnostic::Note (rather than
attaching the main warning diagnostic).
Before this patch:
```
void MyCheck::check(...) {
...
diag(loc, "my check warning") << FixtItHint::CreateReplacement(...);
}
```
After:
```
void MyCheck::check(...) {
...
diag(loc, "my check warning"); // Emit a check warning
diag(loc, "fix description", DiagnosticIDs::Note) << FixtItHint::CreateReplacement(...); // Emit a diagnostic note and a fix
}
```
Reviewers: sammccall, alexfh
Reviewed By: alexfh
Subscribers: MyDeveloperDay, Eugene.Zelenko, aaron.ballman, JonasToth, xazax.hun, jdoerfert, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D59932
llvm-svn: 358576
Summary:
We do have some reports of include insertion behaving badly in some
codebases. Requiring header guards both makes sense in principle, and is
likely to disable this "nice-to-have" feature in codebases where headers don't
follow the expected pattern.
With this we can drop some other heuristics, such as looking at file
extensions to detect known non-headers - implementation files have no guards.
One wrinkle here is #import - objc headers may not have guards because
they're intended to be used via #import. If the header is the main file
or is #included, we won't collect locations - merge should take care of
this if we see the file #imported somewhere. Seems likely to be OK.
Headers which have a canonicalization (stdlib, IWYU) are exempt from this check.
*.inc files continue to be handled by looking up to the including file.
This patch also adds *.def here - tablegen wants this pattern too.
In terms of code structure, the division between SymbolCollector and
CanonicalIncludes has shifted: SymbolCollector is responsible for more.
This is because SymbolCollector has all the SourceManager/HeaderSearch access
needed for checking for guards, and we interleave these checks with the *.def
checks in a loop (potentially).
We could hand all the info into CanonicalIncludes and put the logic there
if that's preferable.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60316
llvm-svn: 358571
Summary:
Previously, we would use include spelling of the declaring header to check
whether the inserted header is the same as the main file. This doesn't help because
we only have file path of the main file.
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60687
llvm-svn: 358496
Summary:
Some implicit/built-in decls lack the source location
information. Fallback to OrigD that we've seen in the source code
instead of the canonical one in those cases.
Reviewers: sammccall
Subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60689
llvm-svn: 358413
Summary:
This makes addDocument non-blocking and would also allow code completion
(in fallback mode) to run when worker waits for the compile command.
Reviewers: sammccall, ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: javed.absar, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60607
llvm-svn: 358400
Summary:
The bugprone-too-small-loop-variable check often catches loop variables which can represent "big enough" values, so we don't actually need to worry about that this variable will overflow in a loop when the code iterates through a container. For example a 32 bit signed integer type's maximum value is 2 147 483 647 and a container's size won't reach this maximum value in most of the cases.
So the idea of this option to allow the user to specify an upper limit (using magnitude bit of the integer type) to filter out those catches which are not interesting for the user, so he/she can focus on the more risky integer incompatibilities.
Next to the option I replaced the term "positive bits" to "magnitude bits" which seems a better naming both in the code and in the name of the new option.
Reviewers: JonasToth, alexfh, aaron.ballman, hokein
Reviewed By: JonasToth
Subscribers: Eugene.Zelenko, xazax.hun, jdoerfert, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D59870
llvm-svn: 358356
Summary:
We have turned on the flag internally for a while, and we don't receive complains.
Should be good to turn it on now.
If the projects doesn't have .clang-tidy files, no clang-tidy check will
be run.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60560
llvm-svn: 358282
Summary:
Last part of re-landing rC356541. Puts TemplateArgumentsList into
responses of the above mentioned two requests.
Reviewers: ioeric, ilya-biryukov
Subscribers: MaskRay, jkorous, arphaman, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59641
llvm-svn: 358274
Summary:
Part of re-landing rC356541 with D59599. Changes the way we store
template arguments, previous patch was storing them inside Name field of Symbol.
Which was violating the assumption:
```Symbol::Scope+Symbol::Name == clang::clangd::printQualifiedName```
which was made in multiple places inside codebase. This patch instead moves
those arguments into their own field. Currently the field is meant to be
human-readable, can be made structured if need be.
Reviewers: ioeric, ilya-biryukov, gribozavr
Subscribers: MaskRay, jkorous, arphaman, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59640
llvm-svn: 358273
Summary:
Prepares ground for printing template arguments as written in the
source code, part of re-landing rC356541 with D59599 applied.
Reviewers: ioeric, ilya-biryukov
Subscribers: mgorny, MaskRay, jkorous, arphaman, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59639
llvm-svn: 358272
Summary:
o Lex the code to get the identifiers and put them into a "symbol" index.
o Adds a new completion mode without compilation/sema into code completion workflow.
o Make IncludeInserter work even when no compile command is present, by avoiding
inserting non-verbatim headers.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60126
llvm-svn: 358159
`Transport.h` does not include `Features.inc`. However, since it is used in a
subdirectory, it cannot directly include the header as it is not available.
Include `Features.inc` in `ClangdLSPServer.h` prior to the inclusion of
`Transport.h` which will provide the interfaces in `ClangdMain.cpp` where the
symbol `newXPCTransport` will not be defined due to it being preprocessed away
since the configuration is not passed along to the initial inclusion.
llvm-svn: 358103
Summary:
There are cases where Sema can't tell that "foo" in foo::Bar is a
namespace qualifier, like in incomplete macro expansions.
After this patch, if sema reports no specifier but it looks like there's one in
the source code, then we take it into account.
Reworked structure and comments in getQueryScopes to try to fight
creeping complexity - unsure if I succeeded.
I made the test harder (the original version also passes).
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60503
llvm-svn: 358091
Summary: One clear use case: use with an editor that reacts poorly to edits above the cursor.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60409
llvm-svn: 358075
Summary:
Intent is to use the heuristically-parsed scope in cases where we get bogus
results from sema, such as in complex macro expansions.
Added a motivating testcase we currently get wrong.
Name changed because we (already) use this for things other than speculation.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60500
llvm-svn: 358074
Summary:
At Mozilla we are using this tool in order to perform review-time static-analysis, since some patches contain a large number of files we've discovered this issue, where `stderr` gets mixed with `stdout` thus obfuscating our possibility to parse the output.
The patch that we are currently use can be found [here](https://searchfox.org/mozilla-central/source/build/build-clang/clang-tidy-8.patch).
This is just an upstream of the original patch.
Reviewers: JonasToth
Reviewed By: JonasToth
Subscribers: cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D60453
llvm-svn: 357994
Summary:
When calling TUScehduler::runWithPreamble (e.g. in code compleiton), allow
entering a fallback mode when compile command or preamble is not ready, instead of
waiting. This allows clangd to perform naive code completion e.g. using identifiers
in the current file or symbols in the index.
This patch simply returns empty result for code completion in fallback mode. Identifier-based
plus more advanced index-based completion will be added in followup patches.
Reviewers: ilya-biryukov, sammccall
Reviewed By: sammccall
Subscribers: sammccall, javed.absar, MaskRay, jkorous, arphaman, kadircet, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59811
llvm-svn: 357916
Requires making the llvm::MemoryBuffer* stored by SourceManager const,
which in turn requires making the accessors for that return const
llvm::MemoryBuffer*s and updating all call sites.
The original motivation for this was to use it and fix the TODO in
CodeGenAction.cpp's ConvertBackendLocation() by using the UnownedTag
version of createFileID, and since llvm::SourceMgr* hands out a const
llvm::MemoryBuffer* this is required. I'm not sure if fixing the TODO
this way actually works, but this seems like a good change on its own
anyways.
No intended behavior change.
Differential Revision: https://reviews.llvm.org/D60247
llvm-svn: 357724
The CMake variable controlling if XPC code is built is called
CLANGD_BUILD_XPC but three places unintentionally checked the
non-existent variable CLANGD_BUILD_XPC_SUPPORT instead, which (due to
being nonexistent, and due to cmake) always silently evaluated to false.
Luckily the test still seems to pass, despite never running after being
added almost 3 months ago in r351280.
Differential Revision: https://reviews.llvm.org/D60120
llvm-svn: 357719
Don Hinton