It cannot match a `pure virtual function`. This patch fixes this behavior.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D116439
As originally reported by @steakhal in
http://github.com/llvm/llvm-project/issues/54074, the name extraction logic of
`readability-suspicious-call-argument` crashes if the argument passed to a
function was a function call to a non-trivially named entity (e.g. an operator).
Fixed this crash case by ignoring such constructs and considering them as having
no name.
Reviewed By: aaron.ballman, steakhal
Differential Revision: http://reviews.llvm.org/D120555
The checker missed a check for a case when the parameter is referenced by an lvalue and this could cause build breakages.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D117090
After D120254 some clang-tidy tests started failing on release builds.
clang-tidy has been using the `-fdeclare-opencl-builtins` functionality
since this became the default in clang, so there is no need to include
`opencl-c.h`.
Differential Revision: https://reviews.llvm.org/D120470
This option allows callers to disable the warning from
https://clang.llvm.org/extra/clang-tidy/checks/performance-move-const-arg.html
that would warn on the following
```
void f(const string &s);
string s;
f(std::move(s)); // ALLOWED if performance-move-const-arg.CheckMoveToConstRef=false
```
The reason people might want to disable this check, is because it allows
callers to use `std::move()` or not based on local reasoning about the
argument, and without having to care about how the function `f` accepts
the argument. Indeed, `f` might accept the argument by const-ref today,
but change to by-value tomorrow, and if the caller had moved the
argument that they were finished with, the code would work as
efficiently as possible regardless of how `f` accepted the parameter.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D119370
Ensure CLANG_PLUGIN_SUPPORT is compatible with llvm_add_library.
Fixes an issue noted in D111100.
Differential Revision: https://reviews.llvm.org/D119199
Display notes for a possible call chain if an unsafe function is found to be
called (maybe indirectly) from a signal handler.
The call chain displayed this way includes probably not the first calls of
the functions, but it is a valid possible (in non path-sensitive way) one.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D118224
This reverts commit 36892727e4 which
breaks the build when LLVM_INSTALL_TOOLCHAIN_ONLY is enabled with:
CMake Error at cmake/modules/AddLLVM.cmake:683 (add_dependencies):
The dependency target "clang-tidy-headers" of target "CTTestTidyModule"
does not exist.
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.
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
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
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
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
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
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
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
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.
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
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
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 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 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.
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.
Jun Zhang