- 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
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
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
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
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
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
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
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
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
Using XCTAssertEqual on NSString* objects is almost always wrong.
Unfortunately, we have seen a lot of tests doing this and reyling on pointer equality for strings with the same values (which happens to work sometimes - depending on the linker, but this assumption is not guaranteed by the language)
These fixes would make tests less brittle.
Differential Revision: https://reviews.llvm.org/D114975
Checks for various ways that the `const CharT*` constructor of `std::basic_string_view` can be passed a null argument and replaces them with the default constructor in most cases. For the comparison operators, braced initializer list does not compile so instead a call to `.empty()` or the empty string literal are used, where appropriate.
This prevents code from invoking behavior which is unconditionally undefined. The single-argument `const CharT*` constructor does not check for the null case before dereferencing its input. The standard is slated to add an explicitly-deleted overload to catch some of these cases: wg21.link/p2166
https://reviews.llvm.org/D114823 is a companion change to prevent duplicate warnings from the `bugprone-string-constructor` check.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D113148
Fixes PR#47614. Deduction guides, implicit or user-defined, look like
function declarations in the AST. They aren't really functions, though,
and they always have a trailing return type, so it doesn't make sense
to issue this warning for them.
The google-readability-casting check is meant to be on par
with cpplint's readability/casting check, according to the
documentation. However it currently does not diagnose
functional casts, like:
float x = 1.5F;
int y = int(x);
This is detected by cpplint, however, and the guidelines
are clear that such a cast is only allowed when the type
is a class type (constructor call):
> You may use cast formats like `T(x)` only when `T` is a class type.
Therefore, update the clang-tidy check to check this
case.
Differential Revision: https://reviews.llvm.org/D114427
- Jaro–Winkler and Sørensen–Dice should use en-dashes not regular
dashes. In reStructuredText this is typed as `--`.
- Letters at the beginning of a sentence should be capitalized.
Detect when an identifier contains some Right-To-Left characters.
This pass relates to https://trojansource.codes/
Example of misleading source:
short int א = (short int)0;
short int ג = (short int)12345;
int main() {
int א = ג; // a local variable, set to zero?
printf("ג is %d\n", ג);
printf("א is %d\n", א);
}
This is a recommit of 299aa4dfa1 with missing
option registration fixed.
Differential Revision: https://reviews.llvm.org/D112914
This reverts commit 7f92a1a84b.
It triggers an assert, see http://45.33.8.238/linux/60293/step_9.txt
"AST/Decl.h:277: llvm::StringRef clang::NamedDecl::getName() const: Assertion `Name.isIdentifier() && "Name is not a simple identifier"' failed."
Suggests switching the initialization pattern of `absl::Cleanup` instances from the factory function to class template argument deduction (CTAD) in C++17 and higher.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D113195
[NFC] As part of using inclusive language within the llvm project,
this patch replaces master with main when referring to `.chm` files.
Reviewed By: teemperor
Differential Revision: https://reviews.llvm.org/D113299
The CERT rule ERR33-C can be modeled partially by the existing check
'bugprone-unused-return-value'. The existing check is reused with
a fixed set of checked functions.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D112409
To simplify suppressing warnings (for example, for
when multiple check aliases are enabled).
The globbing format reuses the same code as for
globbing when enabling checks, so the semantics
and behavior is identical.
Differential Revision: https://reviews.llvm.org/D111208
The list of checked functions was incomplete in the description.
Reviewed By: aaron.ballman, steakhal
Differential Revision: https://reviews.llvm.org/D111623
This requirement was introduced in the C++ Core guidelines in 2016:
1894380d0a
Then clang-tidy got updated to comply with the rule.
However in 2019 this decision was reverted:
5fdfb20b76
Therefore we need to apply the correct configuration to
clang-tidy again.
This also makes this cppcoreguidelines check consistent
with the other 2 alias checks: hicpp-use-override and
modernize-use-override.
Additionally, add another RUN line to the unit test,
to make sure cppcoreguidelines-explicit-virtual-functions
is tested.
Add support for NOLINTBEGIN ... NOLINTEND comments to suppress
clang-tidy warnings over multiple lines. All lines between the "begin"
and "end" markers are suppressed.
Example:
// NOLINTBEGIN(some-check)
<Code with warnings to be suppressed, line 1>
<Code with warnings to be suppressed, line 2>
<Code with warnings to be suppressed, line 3>
// NOLINTEND(some-check)
Follows similar syntax as the NOLINT and NOLINTNEXTLINE comments
that are already implemented, i.e. allows multiple checks to be provided
in parentheses; suppresses all checks if the parentheses are omitted,
etc.
If the comments are misused, e.g. using a NOLINTBEGIN but not
terminating it with a NOLINTEND, a clang-tidy-nolint diagnostic
message pointing to the misuse is generated.
As part of implementing this feature, the following bugs were fixed in
existing code:
IsNOLINTFound(): IsNOLINTFound("NOLINT", Str) returns true when Str is
"NOLINTNEXTLINE". This is because the textual search finds NOLINT as
the stem of NOLINTNEXTLINE.
LineIsMarkedWithNOLINT(): NOLINTNEXTLINEs on the very first line of a
file are ignored. This is due to rsplit('\n\').second returning a blank
string when there are no more newline chars to split on.
This reverts commit 626586fc25.
Tweak the test for Windows. Windows defaults to delayed template
parsing, which resulted in the main template definition not registering
the test on Windows. Process the file with the additional
`-fno-delayed-template-parsing` flag to change the default beahviour.
Additionally, add an extra check for the fix it and use a more robust
test to ensure that the value is always evaluated.
Differential Revision: https://reviews.llvm.org/D108893
This reverts commit 76dc8ac36d.
Restore the change. The test had an incorrect negative from testing.
The test is expected to trigger a failure as mentioned in the review
comments. This corrects the test and should resolve the failure.
This introduces a new check, readability-containter-data-pointer. This
check is meant to catch the cases where the user may be trying to
materialize the data pointer by taking the address of the 0-th member of
a container. With C++11 or newer, the `data` member should be used for
this. This provides the following benefits:
- `.data()` is easier to read than `&[0]`
- it avoids an unnecessary re-materialization of the pointer
* this doesn't matter in the case of optimized code, but in the case
of unoptimized code, this will be visible
- it avoids a potential invalid memory de-reference caused by the
indexing when the container is empty (in debug mode, clang will
normally optimize away the re-materialization in optimized builds).
The small potential behavioural change raises the question of where the
check should belong. A reasoning of defense in depth applies here, and
this does an unchecked conversion, with the assumption that users can
use the static analyzer to catch cases where we can statically identify
an invalid memory de-reference. For the cases where the static analysis
is unable to prove the size of the container, UBSan can be used to track
the invalid access.
Special thanks to Aaron Ballmann for the discussion on whether this
check would be useful and where to place it.
This also partially resolves PR26817!
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D108893
Finds base classes and structs whose destructor is neither public and
virtual nor protected and non-virtual.
A base class's destructor should be specified in one of these ways to
prevent undefined behaviour.
Fixes are available for user-declared and implicit destructors that are
either public and non-virtual or protected and virtual.
This check implements C.35 [1] from the CppCoreGuidelines.
Reviewed By: aaron.ballman, njames93
Differential Revision: http://reviews.llvm.org/D102325
[1]: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-dtor-virtual
Add a check for enforcing minimum length for variable names. A default
minimum length of three characters is applied to regular variables
(including function parameters). Loop counters and exception variables
have a minimum of two characters. Additionally, the 'i', 'j' and 'k'
are accepted as legacy values.
All three sizes, as well as the list of accepted legacy loop counter
names are configurable.
The patch in http://reviews.llvm.org/D106431 landed in commit
4a097efe77 to the main branch.
However, this patch was also backported to upcoming release 13.0.0 in
commit 8dcdfc0de84f60b5b4af97ac5b357881af55bc6e, which makes this entry
in the release notes **NOT** a new thing for the purposes of 14.0.0.
FixIt, and add support for initialization check of scoped enum
In C++, the enumeration is never Integer, and the enumeration condition judgment is added to avoid compiling errors when it is initialized to an integer.
Add support for initialization check of scope enum.
As the following case show, clang-tidy will give a wrong automatic fix:
enum Color {Red, Green, Blue};
enum class Gender {Male, Female};
void func() {
Color color; // Color color = 0; <--- fix bug
Gender gender; // <--- no warning
}
Reviewd By: aaron.ballman, whisperity
Differential Revision: http://reviews.llvm.org/D106431
Many concepts emulation libraries, such as the one found in Range v3, tend to
use non-type template parameters for the enable_if type expression, due to
their versatility in template functions and constructors containing variadic
template parameter packs.
Unfortunately the bugprone-forwarding-reference-overload check does not
handle non-type template parameters, as was first noted in this bug report:
https://bugs.llvm.org/show_bug.cgi?id=38081
This patch fixes this long standing issue and allows for the check to be suppressed
with the use of a non-type template parameter containing enable_if or enable_if_t in
the type expression, so long as it has a default literal value.
Add string list option of type names analagous to `AllowedTypes` which lets
users specify a list of ExcludedContainerTypes.
Types matching this list will not trigger the check when an expensive variable
is copy initialized from a const accessor method they provide, i.e.:
```
ExcludedContainerTypes = 'ExcludedType'
void foo() {
ExcludedType<ExpensiveToCopy> Container;
const ExpensiveToCopy NecessaryCopy = Container.get();
}
```
Even though an expensive to copy variable is copy initialized the check does not
trigger because the container type is excluded.
This is useful for container types that don't own their data, such as view types
where modification of the returned references in other places cannot be reliably
tracked, or const incorrect types.
Differential Revision: https://reviews.llvm.org/D106173
Reviewed-by: ymandel
Finds function calls where the call arguments might be provided in an
incorrect order, based on the comparison (via string metrics) of the
parameter names and the argument names against each other.
A diagnostic is emitted if an argument name is similar to a *different*
parameter than the one currently passed to, and it is sufficiently
dissimilar to the one it **is** passed to currently.
False-positive warnings from this check are useful to indicate bad
naming convention issues, even if a swap isn't necessary.
This check does not generate FixIts.
Originally implemented by @varjujan as his Master's Thesis work.
The check was subsequently taken over by @barancsuk who added type
conformity checks to silence false positive matches.
The work by @whisperity involved driving the check's review and fixing
some more bugs in the process.
Reviewed By: aaron.ballman, alexfh
Differential Revision: http://reviews.llvm.org/D20689
Co-authored-by: János Varjú <varjujanos2@gmail.com>
Co-authored-by: Lilla Barancsuk <barancsuklilla@gmail.com>
While the original check's purpose is to identify potentially dangerous
functions based on the parameter types (as identifier names do not mean
anything when it comes to the language rules), unfortunately, such a plain
interface check rule can be incredibly noisy. While the previous
"filtering heuristic" is able to find many similar usages, there is an entire
class of parameters that should not be warned about very easily mixed by that
check: parameters that have a name and their name follows a pattern,
e.g. `text1, text2, text3, ...`.`
This patch implements a simple, but powerful rule, that allows us to detect
such cases and ensure that no warnings are emitted for parameter sequences that
follow a pattern, even if their types allow for them to be potentially mixed at a call site.
Given a threshold `k`, warnings about two parameters are filtered from the
result set if the names of the parameters are either prefixes or suffixes of
each other, with at most k letters difference on the non-common end.
(Assuming that the names themselves are at least `k` long.)
- The above `text1, text2` is an example of this. (Live finding from Xerces.)
- `LHS` and `RHS` are also fitting the bill here. (Live finding from... virtually any project.)
- So does `Qmat, Tmat, Rmat`. (Live finding from I think OpenCV.)
Reviewed By: aaron.ballman
Differential Revision: http://reviews.llvm.org/D97297
There are several types of functions and various reasons why some
"swappable parameters" cannot be fixed with changing the parameters' types, etc.
The most common example might be int `min(int a, int b)`... no matter what you
do, the two parameters must remain the same type.
The **filtering heuristic** implemented in this patch deals with trying to find
such functions during the modelling and building of the swappable parameter
range.
If the parameter currently scrutinised matches either of the predicates below,
it will be regarded as **not swappable** even if the type of the parameter
matches.
Reviewed By: aaron.ballman
Differential Revision: http://reviews.llvm.org/D78652
Adds a relaxation option ModelImplicitConversions which will make the check
report for cases where parameters refer to types that are implicitly
convertible to one another.
Example:
struct IntBox { IntBox(int); operator int(); };
void foo(int i, double d, IntBox ib) {}
Implicit conversions are the last to model in the set of things that are
reasons for the possibility of a function being called the wrong way which is
not always immediately apparent when looking at the function (signature or
call).
Reviewed By: aaron.ballman, martong
Differential Revision: http://reviews.llvm.org/D75041
Adds a relaxation option QualifiersMix which will make the check report for
cases where parameters refer to the same type if they only differ in qualifiers.
This makes cases, such as the following, not warned about by default, produce
a warning.
void* memcpy(void* dst, const void* src, unsigned size) {}
However, unless people meticulously const their local variables, unfortunately,
even such a function carry a potential swap:
T* obj = new T; // Not const!!!
void* buf = malloc(sizeof(T));
memcpy(obj, buf, sizeof(T));
// ^~~ ^~~ accidental swap here, even though the interface "specified" a const.
Reviewed By: aaron.ballman
Differential Revision: http://reviews.llvm.org/D96355
The base patch only deals with strict (canonical) type equality, which is
merely a subset of all the dangerous function interfaces that we intend to
find.
In addition, in the base patch, canonical type equivalence is not diagnosed in
a way that is immediately apparent to the user.
This patch extends the check with two features:
* Proper typedef diagnostics and explanations to the user.
* "Reference bind power" matching.
Case 2 is a necessary addition because in every case someone encounters a
function `f(T t, const T& tr)`, any expression that might be passed to either
can be passed to both. Thus, such adjacent parameter sequences should be
matched.
Reviewed By: aaron.ballman
Differential Revision: http://reviews.llvm.org/D95736
Finds function definitions where parameters of convertible types follow
each other directly, making call sites prone to calling the function
with swapped (or badly ordered) arguments.
Such constructs are usually the result of inefficient design and lack of
exploitation of strong type capabilities that are possible in the
language.
This check finds and flags **function definitions** and **not** call
sites!
Reviewed By: aaron.ballman, alexfh
Differential Revision: http://reviews.llvm.org/D69560
Within clang-tidy's NarrowingConversionsCheck.
* Allow opt-out of some common occurring patterns, such as:
- Implicit casts between types of equivalent bit widths.
- Implicit casts occurring from the return of a ::size() method.
- Implicit casts on size_type and difference_type.
* Allow opt-in of errors within template instantiations.
This will help projects adopt these guidelines iteratively.
Developed in conjunction with Yitzhak Mandelbaum (ymandel).
Patch by Stephen Concannon!
Differential Revision: https://reviews.llvm.org/D99543
This lint check is a part of the FLOCL (FPGA Linters for OpenCL) project
out of the Synergy Lab at Virginia Tech.
FLOCL is a set of lint checks aimed at FPGA developers who write code
in OpenCL.
The altera ID dependent backward branch lint check finds ID dependent
variables and fields used within loops, and warns of their usage. Using
these variables in loops can lead to performance degradation.
Overflows are never fun.
In most cases (in most of the code), they are rare,
because usually you e.g. don't have as many elements.
However, it's exceptionally easy to fall into this pitfail
in code that deals with images, because, assuming 4-channel 32-bit FP data,
you need *just* ~269 megapixel image to case an overflow
when computing at least the total byte count.
In [[ https://github.com/darktable-org/darktable | darktable ]], there is a *long*, painful history of dealing with such bugs:
* https://github.com/darktable-org/darktable/pull/7740
* https://github.com/darktable-org/darktable/pull/7419
* eea1989f2c
* 70626dd95b
* https://github.com/darktable-org/darktable/pull/670
* 38c69fb1b2
and yet they clearly keep resurfacing still.
It would be immensely helpful to have a diagnostic for those patterns,
which is what this change proposes.
Currently, i only diagnose the most obvious case, where multiplication
is directly widened with no other expressions inbetween,
(i.e. `long r = (int)a * (int)b` but not even e.g. `long r = ((int)a * (int)b)`)
however that might be worth relaxing later.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D93822
This is the only remaining check that creates `std::move` includes but doesn't add a `<utility>` include.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97683
(this was originally part of https://reviews.llvm.org/D96281 and has been split off into its own patch)
If a macro is used within a function, the code inside the macro
doesn't make the code less readable. Instead, for a reader a macro is
more like a function that is called. Thus the code inside a macro
shouldn't increase the complexity of the function in which it is called.
Thus the flag 'IgnoreMacros' is added. If set to 'true' code inside
macros isn't considered during analysis.
This isn't perfect, as now the code of a macro isn't considered at all,
even if it has a high cognitive complexity itself. It might be better if
a macro is considered in the analysis like a function and gets its own
cognitive complexity. Implementing such an analysis seems to be very
complex (if possible at all with the given AST), so we give the user the
option to either ignore macros completely or to let the expanded code
count to the calling function's complexity.
See the code example from vgeof (originally added as note in https://reviews.llvm.org/D96281)
bool doStuff(myClass* objectPtr){
if(objectPtr == nullptr){
LOG_WARNING("empty object");
return false;
}
if(objectPtr->getAttribute() == nullptr){
LOG_WARNING("empty object");
return false;
}
use(objectPtr->getAttribute());
}
The LOG_WARNING macro itself might have a high complexity, but it do not make the
the function more complex to understand like e.g. a 'printf'.
By default 'IgnoreMacros' is set to 'false', which is the original behavior of the check.
Reviewed By: lebedev.ri, alexfh
Differential Revision: https://reviews.llvm.org/D98070
Fixes bug http://bugs.llvm.org/show_bug.cgi?id=49000.
This patch allows Clang-Tidy checks to do
diag(X->getLocation(), "text") << Y->getSourceRange();
and get the highlight of `Y` as expected:
warning: text [blah-blah]
xxx(something)
^ ~~~~~~~~~
Reviewed-By: aaron.ballman, njames93
Differential Revision: http://reviews.llvm.org/D98635
The default setting for CheckImplicitCasts was changed in
https://reviews.llvm.org/D32164 but the documentation was not updated.
This simple change just syncs the documentation with the behavior of
that checker.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D99991
This allows users to be more precise and exclude a type in a specific namespace
from triggering the check instead of excluding all types with the same
unqualified name.
This change should not interfere with correctly configured clang-tidy setups
since an AllowedType with "::" would never match.
Differential Revision: https://reviews.llvm.org/D98738
Reviewed-by: ymandel, hokein
This lint check is a part of the FLOCL (FPGA Linters for OpenCL)
project out of the Synergy Lab at Virginia Tech.
FLOCL is a set of lint checks aimed at FPGA developers who write code
in OpenCL.
The altera unroll loops check finds inner loops that have not been
unrolled, as well as fully-unrolled loops that should be partially
unrolled due to unknown loop bounds or a large number of loop
iterations.
Based on the Altera SDK for OpenCL: Best Practices Guide.
The deprecation notice was cherrypicked to the release branch in f8b3298924 so its safe to remove this for the 13.X release cycle.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D98612
We have no way to reason about the bool returned by try_emplace, so we
simply ignore any std::move()s that happen in a try_emplace argument.
A lot of the time in this situation, the code will be checking the
bool and doing something else if it turns out the value wasn't moved
into the map, and this has been causing false positives so far.
I don't currently have any intentions of handling "maybe move" functions
more generally.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D98034
Often you are only interested in the overall cognitive complexity of a
function and not every individual increment. Thus the flag
'DescribeBasicIncrements' is added. If it is set to 'true', each increment
is flagged. Otherwise, only the complexity of function with complexity
of at least the threshold are flagged.
By default 'DescribeBasisIncrements' is set to 'true', which is the original behavior of the check.
Added a new test for different flag combinations.
(The option to ignore macros which was original part of this patch will be added in another path)
Reviewed By: lebedev.ri
Differential Revision: https://reviews.llvm.org/D96281
- Create a separate section on silencing erroneous warnings and add more material to it
- Add note that the check is flow-sensitive but not path-sensitive
... For removal in next release cycle.
The clang warning that does the same thing is enabled by default and typically emits better diagnostics making this check surplus to requirements.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97491
Added an option to control whether to apply the fixes found in notes attached to clang tidy errors or not.
Diagnostics may contain multiple notes each offering different ways to fix the issue, for that reason the default behaviour should be to not look at fixes found in notes.
Instead offer up all the available fix-its in the output but don't try to apply the first one unless `-fix-notes` is supplied.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D84924
Adds an option, `PreferResetCall`, currently defaulted to `false`, to the check.
When `true` the check will refactor by calling the `reset` member function.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97630
Following a discussion about the current state of this check on the 12.X branch, it was decided to purge the check as it wasn't in a fit to release state, see https://llvm.org/PR49318.
This check has since had some of those issues addressed and should be good for the next release cycle now, pending any more bug reports about it.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97275
This check registers an IncludeInserter, however the check itself doesn't actually emit any fixes or includes, so the inserter is redundant.
From what I can tell the fixes were removed in D26453(rL290051) but the inserter was left in, probably an oversight.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97243
An option is added to the check to select wich set of functions is
defined as asynchronous-safe functions.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D90851
The run-clang-tidy.py helper script is supposed to be used by the
user, hence it should be placed in the user's PATH. Some
distributions, like Gentoo [1], won't have it in PATH unless it is
installed in bin/.
Furthermore, installed scripts in PATH usually do not carry a filename
extension, since there is no need to know that this is a Python
script. For example Debian and Ubuntu already install this script as
'run-clang-tidy' [2] and hence build systems like Meson also look for
this name first [3]. Hence we install run-clang-tidy.py as
run-clang-tidy, as suggested by Sylvestre Ledru [4].
1: https://bugs.gentoo.org/753380
2: 60aefb1417/debian/clang-tidy-X.Y.links.in (L2)
3: b6dc4d5e5c/mesonbuild/scripts/clangtidy.py (L44)
4: https://reviews.llvm.org/D90972#2380640
Reviewed By: sylvestre.ledru, JonasToth
Differential Revision: https://reviews.llvm.org/D90972
This lint check is a part of the FLOCL (FPGA Linters for OpenCL)
project out of the Synergy Lab at Virginia Tech.
FLOCL is a set of lint checks aimed at FPGA developers who write code
in OpenCL.
The altera single work item barrier check finds OpenCL kernel functions
that call a barrier function but do not call an ID function. These
kernel functions will be treated as single work-item kernels, which
could be inefficient or lead to errors.
Based on the "Altera SDK for OpenCL: Best Practices Guide."
While casting an (integral) pointer to an integer is obvious - you just get
the integral value of the pointer, casting an integer to an (integral) pointer
is deceivingly different. While you will get a pointer with that integral value,
if you got that integral value via a pointer-to-integer cast originally,
the new pointer will lack the provenance information from the original pointer.
So while (integral) pointer to integer casts are effectively no-ops,
and are transparent to the optimizer, integer to (integral) pointer casts
are *NOT* transparent, and may conceal information from optimizer.
While that may be the intention, it is not always so. For example,
let's take a look at a routine to align the pointer up to the multiple of 16:
The obvious, naive implementation for that is:
```
char* src(char* maybe_underbiased_ptr) {
uintptr_t maybe_underbiased_intptr = (uintptr_t)maybe_underbiased_ptr;
uintptr_t aligned_biased_intptr = maybe_underbiased_intptr + 15;
uintptr_t aligned_intptr = aligned_biased_intptr & (~15);
return (char*)aligned_intptr; // warning: avoid integer to pointer casts [misc-no-inttoptr]
}
```
The check will rightfully diagnose that cast.
But when provenance concealment is not the goal of the code, but an accident,
this example can be rewritten as follows, without using integer to pointer cast:
```
char*
tgt(char* maybe_underbiased_ptr) {
uintptr_t maybe_underbiased_intptr = (uintptr_t)maybe_underbiased_ptr;
uintptr_t aligned_biased_intptr = maybe_underbiased_intptr + 15;
uintptr_t aligned_intptr = aligned_biased_intptr & (~15);
uintptr_t bias = aligned_intptr - maybe_underbiased_intptr;
return maybe_underbiased_ptr + bias;
}
```
See also:
* D71499
* [[ https://www.cs.utah.edu/~regehr/oopsla18.pdf | Juneyoung Lee, Chung-Kil Hur, Ralf Jung, Zhengyang Liu, John Regehr, and Nuno P. Lopes. 2018. Reconciling High-Level Optimizations and Low-Level Code in LLVM. Proc. ACM Program. Lang. 2, OOPSLA, Article 125 (November 2018), 28 pages. ]]
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D91055
This extends the check for default initialization in arrays added in
547f89d607 to include scalar types and exclude them from the suggested fix for
make_unique/make_shared.
Rewriting std::unique_ptr<int>(new int) as std::make_unique<int>() (or for
other, similar trivial T) switches from default initialization to value
initialization, a performance regression for trivial T. For these use cases,
std::make_unique_for_overwrite is more suitable alternative.
Reviewed By: hokein
Differential Revision: https://reviews.llvm.org/D90392
Using bools instead of integers better conveys the expected value of the option.
Reviewed By: Eugene.Zelenko, aaron.ballman
Differential Revision: https://reviews.llvm.org/D92652
Checks for some thread-unsafe functions against a black list
of known-to-be-unsafe functions. Usually they access static variables
without synchronization (e.g. gmtime(3)) or utilize signals
in a racy way (e.g. sleep(3)).
The patch adds a check instead of auto-fix as thread-safe alternatives
usually have API with an additional argument
(e.g. gmtime(3) v.s. gmtime_r(3)) or have a different semantics
(e.g. exit(3) v.s. __exit(3)), so it is a rather tricky
or non-expected fix.
An option specifies which functions in libc should be considered
thread-safe, possible values are `posix`, `glibc`,
or `any` (the most strict check). It defaults to 'any' as it is
unknown what target libc type is - clang-tidy may be run
on linux but check sources compiled for other *NIX.
The check is used in Yandex Taxi backend and has caught
many unpleasant bugs. A similar patch for coroutine-unsafe API
is coming next.
Reviewed By: lebedev.ri
Differential Revision: https://reviews.llvm.org/D90944
The module will contain checks related to concurrent programming (including threads, fibers, coroutines, etc.).
Reviewed By: lebedev.ri
Differential Revision: https://reviews.llvm.org/D91656
The idea of suppressing naming checks for variables is to support code bases that allow short variables named e.g 'x' and 'i' without prefix/suffixes or casing styles. This was originally proposed as a 'ShortSizeThreshold' however has been made more generic with a regex to suppress identifier naming checks for those that match.
Reviewed By: njames93, aaron.ballman
Differential Revision: https://reviews.llvm.org/D90282
std::string_view("") produces a string_view instance that compares
equal to std::string_view(), but requires more complex initialization
(storing the address of the string literal, rather than zeroing).
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D91009
This allows for matching the constructors std::string has in common with
std::string_view.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D91015
The altera kernel name restriction check finds kernel files and include
directives whose filename is "kernel.cl", "Verilog.cl", or "VHDL.cl".
Such kernel file names cause the Altera Offline Compiler to generate
intermediate design files that have the same names as certain internal
files, which leads to a compilation error.
As per the "Guidelines for Naming the Kernel" section in the "Intel FPGA
SDK for OpenCL Pro Edition: Programming Guide."
This reverts the reversion from 43a38a6523.
SIG30-C. Call only asynchronous-safe functions within signal handlers
First version of this check, only minimal list of functions is allowed
("strictly conforming" case), for C only.
Differential Revision: https://reviews.llvm.org/D87449
The altera kernel name restriction check finds kernel files and include
directives whose filename is "kernel.cl", "Verilog.cl", or "VHDL.cl".
Such kernel file names cause the Altera Offline Compiler to generate
intermediate design files that have the same names as certain internal
files, which leads to a compilation error.
As per the "Guidelines for Naming the Kernel" section in the "Intel FPGA
SDK for OpenCL Pro Edition: Programming Guide."
Changed two references to developers as "he" or "him" to the more neutral "they".
Reviewed By: JDevlieghere, sylvestre.ledru
Differential Revision: https://reviews.llvm.org/D78807
Added option `ScopedEnumConstant(Prefix|Case|Suffix)` to readability-identitied-naming.
This controls the style for constants in scoped enums, declared as enum (class|struct).
If this option is unspecified the EnumConstant style will be used instead.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D89407
Enables support for transforming loops of the form
```
for (auto I = Cont.rbegin(), E = Cont.rend(); I != E;++I)
```
This is done automatically in C++20 mode using `std::ranges::reverse_view` but there are options to specify a different function to reverse iterator over a container.
This is the first step, down the line I'd like to possibly extend this support for array based loops
```
for (unsigned I = Arr.size() - 1;I >=0;--I) Arr[I]...
```
Currently if you pass a reversing function with no header in the options it will just assume that the function exists, however as we have the ASTContext it may be as wise to check before applying, or at least lower the confidence level if we can't find it.
Reviewed By: alexfh
Differential Revision: https://reviews.llvm.org/D82089
Currently, there is basically just one clang-tidy check to impose
some sanity limits on functions - `clang-tidy-readability-function-size`.
It is nice, allows to limit line count, total number of statements,
number of branches, number of function parameters (not counting
implicit `this`), nesting level.
However, those are simple generic metrics. It is still trivially possible
to write a function, which does not violate any of these metrics,
yet is still rather unreadable.
Thus, some additional, slightly more complicated metric is needed.
There is a well-known [[ https://en.wikipedia.org/wiki/Cyclomatic_complexity | Cyclomatic complexity]], but certainly has its downsides.
And there is a [[ https://www.sonarsource.com/docs/CognitiveComplexity.pdf | COGNITIVE COMPLEXITY by SonarSource ]], which is available for opensource on https://sonarcloud.io/.
This check checks function Cognitive Complexity metric, and flags
the functions with Cognitive Complexity exceeding the configured limit.
The default limit is `25`, same as in 'upstream'.
The metric is implemented as per [[ https://www.sonarsource.com/docs/CognitiveComplexity.pdf | COGNITIVE COMPLEXITY by SonarSource ]] specification version 1.2 (19 April 2017), with two notable exceptions:
* `preprocessor conditionals` (`#ifdef`, `#if`, `#elif`, `#else`,
`#endif`) are not accounted for.
Could be done. Currently, upstream does not account for them either.
* `each method in a recursion cycle` is not accounted for.
It can't be fully implemented, because cross-translational-unit
analysis would be needed, which is not possible in clang-tidy.
Thus, at least right now, i completely avoided implementing it.
There are some further possible improvements:
* Are GNU statement expressions (`BinaryConditionalOperator`) really free?
They should probably cause nesting level increase,
and complexity level increase when they are nested within eachother.
* Microsoft SEH support
* ???
Reviewed By: aaron.ballman, JonasToth, lattner
Differential Revision: https://reviews.llvm.org/D36836
Some projects do not use the TEMP_FAILURE_RETRY macro but define their
own one, as not to depend on glibc / Bionic details. By allowing the
user to override the list of macros, these projects can also benefit
from this check.
Differential Revision: https://reviews.llvm.org/D83144
Finds member initializations in the constructor body which can be placed
into the initialization list instead. This does not only improves the
readability of the code but also affects positively its performance.
Class-member assignments inside a control statement or following the
first control statement are ignored.
Differential Revision: https://reviews.llvm.org/D71199
The integration is already complete; this patch updates information as well as
suggests using Clang-Tidy via Clangd integration that is vastly available
in most editors through LSP client plugins.
Reviewed By: hokein
Differential Revision: https://reviews.llvm.org/D87686
Instead of using CLANG_ENABLE_STATIC_ANALYZER for use of the
static analyzer in both clang and clang-tidy, add a second
toggle CLANG_TIDY_ENABLE_STATIC_ANALYZER.
This allows enabling the static analyzer in clang-tidy while
disabling it in clang.
Differential Revison: https://reviews.llvm.org/D87118
The altera struct pack align lint check finds structs that are inefficiently
packed or aligned and recommends packing/aligning of the structs using the
packed and aligned attributes as needed in a warning.
Checking the same condition again in a nested `if` usually make no sense,
except if the value of the expression could have been changed between
the two checks. Although compilers may optimize this out, such code is
suspicious: the programmer may have meant to check something else.
Therefore it is worth to find such places in the code and notify the
user about the problem.
This patch implements a basic check for this problem. Currently it
only detects redundant conditions where the condition is a variable of
integral type. It also detects the possible bug if the variable is in an
//or// or //and// logical expression in the inner if and/or the variable
is in an //and// logical expression in the outer if statement. Negated
cases are not handled yet.
Differential Revision: https://reviews.llvm.org/D81272
Finds member initializations in the constructor body which can
be placed to the member initializers of the constructor instead.
This does not only improves the readability of the code but also
affects positively its performance. Class-member assignments
inside a control statement or following the first control
statement are ignored.
Differential Revision: https://reviews.llvm.org/D71199
Skeleton checks generated by clang-tidy add_check.py cause assertions to fail when run over anonymous functions(lambda functions). This patch introduces an additional check to verify that the target function is not anonymous before calling getName().
The code snippet from the [[ https://clang.llvm.org/extra/clang-tidy/Contributing.html | clang-tidy tutorial ]]is also updated.
Reviewed By: alexfh, DavidTruby
Differential Revision: https://reviews.llvm.org/D85218
When checking for the style of a decl that isn't in the main file, the check will now search for the configuration that the included files uses to gather the style for its decls.
This can be useful to silence warnings in header files that follow a different naming convention without using header-filter to silence all warnings(even from other checks) in the header file.
Reviewed By: aaron.ballman, gribozavr2
Differential Revision: https://reviews.llvm.org/D84814
This implements the default(firstprivate) clause as defined in OpenMP
Technical Report 8 (2.22.4).
Reviewed By: jdoerfert, ABataev
Differential Revision: https://reviews.llvm.org/D75591
Richard