- Sort new checks by check name
- Sort changes to existing checks by check name
- Add docs for changes to readability-simplify-boolean-expr
- Move check changes from "Improvements to clang-tidy" to
"Changes in existing checks" section
Differential Revision: https://reviews.llvm.org/D118519
Inside a switch the caseStmt() and defaultStmt() have a nested statement
associated with them. Similarly, labelStmt() has a nested statement.
These statements were being missed when looking for a compound-if of the
form "if (x) return true; return false;" when the if is nested under one
of these labelling constructs.
Enhance the matchers to look for these nested statements using some
private matcher hasSubstatement() traversal matcher on case, default
and label statements. Add the private matcher hasSubstatementSequence()
to match the compound "if (x) return true; return false;" pattern.
- Add unit tests for private matchers and corresponding test
infrastructure
- Add corresponding test file readability-simplify-bool-expr-case.cpp.
- Fix variable name copy/paste error in readability-simplify-bool-expr.cpp.
- Drop the asserts, which were used only for debugging matchers.
- Run clang-format on the whole check.
- Move local functions out of anonymous namespace and declare state, per
LLVM style guide
- Declare labels constexpr
- Declare visitor arguments as pointer to const
- Drop braces around simple control statements per LLVM style guide
- Prefer explicit arguments over default arguments to methods
Differential Revision: https://reviews.llvm.org/D56303Fixes#27078
This commit checks if a function is marked with the naked attribute
and, if it is, will silence the emission of any unused-parameter
warning.
Inside a naked function only the usage of basic ASM instructions is
expected. In this context the parameters can actually be used by
fetching them according to the underlying ABI. Since parameters might
be used through ASM instructions, the linter and the compiler will have
a hard time understanding if one of those is unused or not, therefore
no unused-parameter warning should ever be triggered whenever a
function is marked naked.
This results in excessive memory usage and eats a lot of screen estate.
Especially in the cases with lots of nested macro calls.
This patch tries to remedy it before the release cut by suppressing the
initializers. For better UX we should probably update the expression printer to
truncate those (behind some policy).
Fixes https://github.com/clangd/clangd/issues/917
Differential Revision: https://reviews.llvm.org/D118260
The check previously inspected only the immediate parent namespace.
`static` in a named namespace within an unnamed namespace is still
redundant.
We will use `Decl::isInAnonymousNamespace()` method that traverses the
namespaces hierarchy recursively.
Differential Revision: https://reviews.llvm.org/D118010
Underscore-uglified identifiers are used in standard library implementations to
guard against collisions with macros, and they hurt readability considerably.
(Consider `push_back(Tp_ &&__value)` vs `push_back(Tp value)`.
When we're describing an interface, the exact names of parameters are not
critical so we can drop these prefixes.
This patch adds a new PrintingPolicy flag that can applies this stripping
when recursively printing pieces of AST.
We set it in code completion/signature help, and in clangd's hover display.
All three features also do a bit of manual poking at names, so fix up those too.
Fixes https://github.com/clangd/clangd/issues/736
Differential Revision: https://reviews.llvm.org/D116387
Support for NOLINT(BEGIN/END) blocks (implemented in D108560) is
currently costly. This patch aims to improve the performance with the
following changes:
- The use of tokenized NOLINTs instead of a series of repetitive ad-hoc
string operations (`find()`, `split()`, `slice()`, regex matching etc).
- The caching of NOLINT(BEGIN/END) block locations. Determining these
locations each time a new diagnostic is raised is wasteful as it
requires reading and parsing the entire source file.
Move NOLINT-specific code from `ClangTidyDiagnosticConsumer` to new
purpose-built class `NoLintDirectiveHandler`.
Differential Revision: https://reviews.llvm.org/D116085
These make the init lists appear as if designated initialization was used.
Example:
ExpectedHint{"param: ", "arg"}
becomes
ExpectedHint{.Label="param: ", .RangeName="arg"}
Differential Revision: https://reviews.llvm.org/D116786
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
Richard