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
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
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
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
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
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
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
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 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
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
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.
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
- 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
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
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;
};
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.
Adrian Vogelsgesang