Now in libcxx and clang, all the coroutine components are defined in
std::experimental namespace.
And now the coroutine TS is merged into C++20. So in the working draft
like N4892, we could find the coroutine components is defined in std
namespace instead of std::experimental namespace.
And the coroutine support in clang seems to be relatively stable. So I
think it may be suitable to move the coroutine component into the
experiment namespace now.
This patch would make clang lookup coroutine_traits in std namespace
first. For the compatibility consideration, clang would lookup in
std::experimental namespace if it can't find definitions in std
namespace. So the existing codes wouldn't be break after update
compiler.
And in case the compiler found std::coroutine_traits and
std::experimental::coroutine_traits at the same time, it would emit an
error for it.
The support for looking up std::experimental::coroutine_traits would be
removed in Clang16.
Reviewed By: lxfind, Quuxplusone
Differential Revision: https://reviews.llvm.org/D108696
Back in the mists of time, the CXXRecordDecl for the injected-class-name was
a redecl of the outer class itself.
This got changed in 470c454a61, but only for plain
classes: class template instantation was still detecting the injected-class-name
in the template body and marking its instantiation as a redecl.
This causes some subtle inconsistent behavior between the two, e.g.
hasDefinition() returns true for Foo<int>::Foo but false for Bar::Bar.
This is the root cause of PR51912.
Differential Revision: https://reviews.llvm.org/D112765
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
We're missing all cases where the return value is a type alias.
Unfortunately, this includes things we care about, such as
`std::vector<T>::operator[]` (return value is `const_reference`,
not `const T&`).
Match the canonical type instead.
Differential Revision: https://reviews.llvm.org/D112722
The string table `DefaultIgnoredParameterTypeSuffixes` has a typo:
`ForwardIt` is mistyped as `FowardIt`.
Correct typo and add test coverage.
Differential Revision: https://reviews.llvm.org/D112596
clang-tidy can be used to statically analyze CUDA code,
thanks to clang being able to compile CUDA code natively.
This makes clang-tidy the one and only open-source
static analyzer for CUDA.
However it currently warns for native CUDA built-in
variables, like threadIdx, due to the way they
are implemented in clang.
Users don't need to know the details of the clang
implementation, and they should continue to write
idiomatic code. Therefore, suppress the warning
if a CUDA built-in variable is encountered.
Fixes https://bugs.llvm.org/show_bug.cgi?id=48758
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
Incorrectly triggers for template classes that inherit
from a base class that has virtual destructor.
Any class inheriting from a base that has a virtual destructor
will have their destructor also virtual, as per the Standard:
https://timsong-cpp.github.io/cppwp/n4140/class.dtor#9
> If a class has a base class with a virtual destructor,
> its destructor (whether user- or implicitly-declared) is virtual.
Added unit tests to prevent regression.
Fixes bug https://bugs.llvm.org/show_bug.cgi?id=51912
Differential Revision: https://reviews.llvm.org/D110614
If the Node has an invalid location, it will trigger assert in
isInSystemHeader(...).
void test() {
__builtin_va_list __args;
// __builtin_va_list has no defination in any source file and its
// CXXConstructorDecl has invalid sourcelocation
}
coredump with "Assertion `Loc.isValid() && "Can't get file
characteristic of invalid loc!"' failed." in
getFileCharacteristic(SourceLocation).
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.
Fixes https://bugs.llvm.org/show_bug.cgi?id=51790. The check triggers
incorrectly with non-type template parameters.
A bisect determined that the bug was introduced here:
ea2225a10b
Unfortunately that patch can no longer be reverted on top of the main
branch, so add a fix instead. Add a unit test to avoid regression in
the future.
At most one variant member of a union may have a default member
initializer. The case of anonymous records with multiple levels of
nesting like the following also needs to meet this rule. The original
logic is to horizontally obtain all the member variables in a record
that need to be initialized and then filter to the variables that need
to be fixed. Obviously, it is impossible to correctly initialize the
desired variables according to the nesting relationship.
See Example 3 in class.union
union U {
U() {}
int x; // int x{};
union {
int k; // int k{}; <== wrong fix
};
union {
int z; // int z{}; <== wrong fix
int y;
};
};
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
Low-level code may occasionally deal with direct access by concrete addresses
such as 0x1234. Values at these addresses act like globals: they can change
at any time. They typically wear volatile qualifiers.
Suppress all warnings on loops with conditions that involve casting anything to
a pointer-to-...-pointer-to-volatile type.
The closely related bugprone-redundant-branch-condition check
doesn't seem to be affected. Add a test just in case.
Differential Revision: https://reviews.llvm.org/D108808
This reverts commit 2fbd254aa4, which broke the libc++ CI. I'm reverting
to get things stable again until we've figured out a way forward.
Differential Revision: https://reviews.llvm.org/D108696
Summary: Now in libcxx and clang, all the coroutine components are
defined in std::experimental namespace.
And now the coroutine TS is merged into C++20. So in the working draft
like N4892, we could find the coroutine components is defined in std
namespace instead of std::experimental namespace.
And the coroutine support in clang seems to be relatively stable. So I
think it may be suitable to move the coroutine component into the
experiment namespace now.
But move the coroutine component into the std namespace may be an break
change. So I planned to split this change into two patch. One in clang
and other in libcxx.
This patch would make clang lookup coroutine_traits in std namespace
first. For the compatibility consideration, clang would lookup in
std::experimental namespace if it can't find definitions in std
namespace and emit a warning in this case. So the existing codes
wouldn't be break after update compiler.
Test Plan: check-clang, check-libcxx
Reviewed By: lxfind
Differential Revision: https://reviews.llvm.org/D108696
https://bugs.llvm.org/show_bug.cgi?id=50069
When clang-tidy sees:
```
if (true) [[unlikely]] {
...
}
```
It thinks the braces are missing and add them again.
```
if (true) { [[unlikely]] {
...
}
}
```
This revision aims to prevent that incorrect code generation
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D105479
The overload of the constructor will repeatedly fix the member variables that need to be initialized.
Removed the duplicate '{}'.
```
struct A {
A() {}
A(int) {}
int _var; // int _var{}{}; <-- wrong fix
};
```
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D107641
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.
std::string, std::string_view, and absl::string_view all have a three-parameter version of find()
which has a "count" (or "n") paremeter limiting the size of the substring to search. We don't want
to propose changing to absl::StrContains in those cases. This change fixes that and adds unit tests
to confirm.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D107837
This patch tries to fix command line too long problem on Windows for
https://reviews.llvm.org/D86671.
The command line is too long with check_clang_tidy.py program on Windows,
because the configuration is long for regression test. Fix this issue by
passing the settings in file instead.
Differential Revision: https://reviews.llvm.org/D107325
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.
An otherwise unexercised code path related to trying to model
"array-to-pointer decay" resulted in a null pointer dereference crash
when parameters of type "reference to array" were encountered.
Fixes crash report http://bugs.llvm.org/show_bug.cgi?id=50995.
Reviewed By: aaron.ballman
Differential Revision: http://reviews.llvm.org/D106946
Make the check handle cases of the "common type" involved in the mix
being non-trivial, e.g. pointers, references, attributes, these things
coming from typedefs, etc.
This results in clearer diagnostics that have more coverage in their
explanation, such as saying `const int &` as common type instead of
`int`.
Reviewed By: aaron.ballman
Differential Revision: http://reviews.llvm.org/D106442
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
This can happen when a template with two parameter types is instantiated with a
single type. The fix would only be valid for this instantiation but fail for
others that rely on an implicit type conversion.
The test cases illustrate when the check should trigger and when not.
Differential Revision: https://reviews.llvm.org/D106011
@vabridgers identified a way to crash the check by running on code that
involve `AttributedType`s. This patch fixes the check to first and
foremost not crash, but also improves the logic handling qualifiers.
If the types contain any additional (not just CVR) qualifiers that are
not the same, they will not be deemed mixable. The logic for CVR-Mixing
and the `QualifiersMix` check option remain unchanged.
Reviewed By: aaron.ballman, vabridgers
Differential Revision: http://reviews.llvm.org/D106361
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>
When deleting the copy assignment statement because copied variable is not used
only remove trailing comments on the same line.
Differential Revision: https://reviews.llvm.org/D105734
Reviewed-by: ymandel
Structured bindings can currently trigger the check and lead to a wrong
fix. Because the DecompositionDecl itself is not used and the check does not
iterate through its the decl's bindings to verify whether the bindings' holding
vars are used this leads to the whole statement to be deleted.
To support structured bindings properly 3 cases would need to be considered.
1. All holding vars are not used -> The statement can be deleted.
2. All holding vars are used as const or not used -> auto can be converted to const auto&.
3. Neither case is true -> leave unchanged.
In the check we'll have to separate the logic that determines this from the code
that produces the diagnostic and fixes and first determine which of the cases
we're dealing with before creating fixes.
Since this is a bigger refactoring we'll disable structured bindings for now to
prevent incorrect fixes.
Differential Revision: https://reviews.llvm.org/D105727
Reviewed-by: ymandel
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
This fixes false positive cases where a reference is initialized outside of a
block statement and then its initializing variable is modified. Another case is
when the looped over container is modified.
Differential Revision: https://reviews.llvm.org/D103021
Reviewed-by: ymandel
mixed integer and floating point types with WarnOnEquivalentBitWidth=0.
Also standardize control flow of handleX conversion functions to make it easier to be consistent.
Patch by Stephen Concannon!
Differential Revision: https://reviews.llvm.org/D103894
There is a followup fix for a unit test introduced at D102906. The test file was placed into a temp folder and test assumed that it would be visible without the full path specification.
This behaviour can be changed in future and it would be good to specify full path to the file at the test.
Test Plan:
```
ninja check-clang-tools
```
Reviewed By: DmitryPolukhin
Differential Revision: https://reviews.llvm.org/D104021
This fixes a false positive case where for instance a pointer is obtained and declared using `auto`.
Differential Revision: https://reviews.llvm.org/D103018
Reviewed-by: ymandel
It is not useful to keep the statement around and can lead to compiler
warnings when -Wall (-Wunused-variable specifically) turned on.
Differential Revision: https://reviews.llvm.org/D102175
Reviewed-by: ymandel
The diff adds Remark to Diagnostic::Level for clang tooling. That makes
Remark diagnostic level ready to use in clang-tidy checks: the
clang-diagnostic-module-import becomes visible as a part of the change.
We can only use ASTContext::getTypeInfo for complete types.
This fixes bugzilla issue 50313.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D102569
If the loop condition is a value of an instance variable, a property value,
or a message result value, it's a good indication that the loop is not infinite
and we have a really hard time proving the opposite so suppress the warning.
Differential Revision: https://reviews.llvm.org/D102294
Take advantage of the new ASTMatcher added in D102213 to fix massive false negatives of the infinite loop checker on Objective-C.
Differential Revision: https://reviews.llvm.org/D102214
This commit fixes the cppcoreguidelines-pro-type-vararg test when it
runs on a Windows host, but the toolchain is targeted a non-Windows
platform.
Reviewed By: njames93
Differential Revision: https://reviews.llvm.org/D102337
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
When a variable is used in an initializer of an aggregate
for its reference-type field this counts as aliasing.
Differential Revision: https://reviews.llvm.org/D101791
D96215 takes care of the situation where the variable is captured into
a nearby lambda. This patch takes care of the situation where
the current function is the lambda and the variable is one of its captures
from an enclosing scope.
The analogous problem for ^{blocks} is already handled automagically
by D96215.
Differential Revision: https://reviews.llvm.org/D101787
The utility function clang::tidy::utils::hasPtrOrReferenceInFunc() scans the
function for pointer/reference aliases to a given variable. It currently scans
for operator & over that variable and for declarations of references to that
variable.
This patch makes it also scan for C++ lambda captures by reference
and for Objective-C block captures.
Differential Revision: https://reviews.llvm.org/D96215
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.
Change instances where options which are boolean are assigned the value 1|0 to use true|false instead.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D101721
This commit fixes cppcoreguidelines-pro-type-vararg false positives on
'char *' variables.
The incorrect warnings generated by clang-tidy can be illustrated with
the following minimal example:
```
goid foo(char* in) {
char *tmp = in;
}
```
The problem is that __builtin_ms_va_list desugared as 'char *', which
leads to false positives.
Fixes bugzilla issue 48042.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D101259
Mishandling of variadic arguments in a function call caused a crash
(runtime assert fail) in bugprone-infinite-loop tidy checker. Fix
is to limit argument matching to the lesser of the number of variadic
params in the prototype or the number of actual args in the call.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D101108
clang-tidy should not generate warnings for the goto argument without
parentheses, because it would be a syntax error.
The only valid case where an argument can be enclosed in parentheses is
"Labels as Values" gcc extension: https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html.
This commit adds support for the label-as-values extension as implemented in clang.
Fixes bugzilla issue 49634.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D99924
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
There was an off-by-one issue with calculating the *exact* end location
of token ranges (as given by SomeDecl->getSourceRange()) which resulted in:
xxx(something)
^~~~~~~~ // Note the missing ~ under the last character.
In addition, a test is added to keep the behaviour in check in the future.
This patch hotfixes commit 3b677b81ce.
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
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 diff patch fixes issue with new line character after check name and before comma. Also ignores all other types of spaces like TAB.
Test Plan: ninja check-clang-tools
Differential Revision: https://reviews.llvm.org/D99180
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.
Don't emit a warning if the `continue` appears in a switch context as changing it to `break` will break out of the switch rather than a do loop containing the switch.
Fixes https://llvm.org/PR49492.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D98338
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
For some reason the initial implementation of the check had an explicit check
for the main file to avoid being applied in headers. This diff removes this
check and add a test for the check on a header.
Similar approach was proposed in D61989 but review there got stuck.
Test Plan: added new test case
Differential Revision: https://reviews.llvm.org/D97563
If a identifier has a correct prefix/suffix but a bad case, the fix won't strip them when computing the correct case, leading to duplication when the are added back.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D98521
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
Enables transforming loops of the form:
```
for (int i = 0; I != container.size(); ++I) { container[I]...; }
for (int i = 0; I != N; ++I) { FixedArrSizeN[I]...; }
```
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97940
This disables the check for false positive cases where implicit type conversion
through either an implicit single argument constructor or a member conversion
operator is triggered when constructing the loop variable.
Fix the test cases that meant to cover these cases.
Differential Revision: https://reviews.llvm.org/D97577
Reviewed-by: hokein
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
If C++17 mode is enabled and the assert doesn't have a string literal, we can emit a static assert with no message in favour of one with an empty message.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97313
Tweak the diagnostics to create small replacements rather than grabbing source text from the lexer.
Also simplified the diagnostic message.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97632
LIBCLANG_INCLUDE_CLANG_TOOLS_EXTRA causes clang-tools-extra tools
to be included in libclang, which caused a dependency cycle. The option
has been off by default for two releases now, and (based on a web search
and mailing list feedback) nobody seems to turn it on. Remove it, like
planned on https://reviews.llvm.org/D79599
Differential Revision: https://reviews.llvm.org/D97693
Fix up cases where diag is called by piecing together a string in favour of placeholders.
Fix up cases where select could be used instead of duplicating the message for sake of 1 word difference.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97488
If a check-suffix is only required for a CHECK-FIXES or CHECK-MESSAGES. check_clang_tidy will pass the prefixes CHECK-FIXES<...> and CHECK-MESSAGES<...> to FileCheck.
This will result in a FileCheck failing because of an unused prefix.
This addresses the problem by not passing unused prefixes. Its also possible to fix this be passing `--allow-unused-prefixes` flag to FileCheck, but seeing as we have already done the legwork in the script to see its unused, this fix seems the better way to go.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97322
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
Prevent warning when the values are initialized using fields that will be initialized later or VarDecls defined in the constructors body.
Both of these cases can't be safely fixed.
Also improve logic of finding where to insert member initializers, previously it could be confused by in class member initializers.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97132
After updating clang driver to include standard
OpenCL headers implicitly, the output being checked
in the test does not match because the implicit
header contains other pragmas. The test does not
aim to use the header and therefore it has to be
updated passing '-cl-no-stdinc' command-line flag.
This fixes failing bots.
Diagnose the problem in templates in the context of the template
declaration instead of in the context of all of the (possibly very many)
template instantiations.
Differential Revision: https://reviews.llvm.org/D96224
Because it no longer relies on finding implicit casts, this check now
works on templates which are not instantiated in the translation unit.
Differential Revision: https://reviews.llvm.org/D96138
In addition to wall time etc. this should allow us to get less noisy
values for time measurements.
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D96049
`clang-tidy -std=c++20` with `modernize-use-nullptr` mistakenly inserts `nullptr` in place of the comparison operator if the comparison internally expands in the AST to a rewritten spaceship operator. This can be reproduced by running the new `modernize-use-nullptr-cxx20.cpp` test without applying the supplied patch to `UseNullptrCheck.cpp`; the current clang-tidy will mistakenly replace:
```result = (a1 < a2);```
with
```result = (a1 nullptr a2);```
Reviewed By: njames93
Differential Revision: https://reviews.llvm.org/D95714
`modernize-loop-convert` handles //array-like// objects like vectors fairly well, but strips slightly too much information from the iteration expression by converting:
```
Vector<Vector<int>> X;
for (int J = 0; J < X[5].size(); ++J)
copyArg(X[5][J]);
```
to
```
Vector<Vector<int>> X;
for (int J : X) // should be for (int J : X[5])
copyArg(J);
```
The `[5]` is a call to `operator[]` and gets stripped by `LoopConvertCheck::getContainerString`. This patch fixes that and adds several test cases.
Reviewed By: njames93
Differential Revision: https://reviews.llvm.org/D95771
Use IgnoreUnlessSpelledInSource to make the matcher code smaller and
more visibly-related to the code.
Differential Revision: https://reviews.llvm.org/D91303
Use mapAnyOf() and matchers based on it.
Use of binaryOperation() means that modernize-loop-convert and
readability-container-size-empty can now be used with rewritten binary
operators.
Differential Revision: https://reviews.llvm.org/D94131
Fail gracefully instead. Prevent further misuse by enforcing the factory builder
instead of the constructor.
Differential Revision: https://reviews.llvm.org/D94420
Attempt to fix the 2 failing tests identifier in 48646.
Appears that python3 doesn't like nested double quotes in single quoted strings, hopefully nested single quotes in double quoted strings is a-ok.
Reviewed By: thakis
Differential Revision: https://reviews.llvm.org/D93979
readability-container-size-empty currently modifies source code based on
AST nodes in template instantiations, which means that it makes
transformations based on substituted types. This can lead to
transforming code to be broken.
Change the matcher implementation to ignore template instantiations
explicitly, and add a matcher to explicitly handle template declarations
instead of instantiations.
Differential Revision: https://reviews.llvm.org/D91302
Modify the cppcoreguidelines-pro-type-member-init checker to ignore warnings from the move and copy-constructors when they are compiler defined with `= default` outside of the type declaration.
Reported as [LLVM bug 36819](https://bugs.llvm.org/show_bug.cgi?id=36819)
Reviewed By: malcolm.parsons
Differential Revision: https://reviews.llvm.org/D93333
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."
Extend the check to not only look at the variable the unnecessarily copied
variable is initialized from, but also ensure that any variable the old variable
references is not modified.
Extend DeclRefExprUtils to also count references and pointers to const assigned
from the DeclRef we check for const usages.
Reviewed-by: aaron.ballman
Differential Revision: https://reviews.llvm.org/D91893
Add methods for emitting diagnostics with no location as well as a special diagnostic for configuration errors.
These show up in the errors as [clang-tidy-config].
The reason to use a custom name rather than the check name is to distinguish the error isn't the same category as the check that reported it.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D91885
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
The check 'cppcoreguidelines-narrowing-conversions' does not detect conversions
involving typedef. This notably includes the standard fixed-width integer types
like int32_t, uint64_t, etc. Now look through the typedefs at the desugared type.
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
Commit fbdff6f3ae0b in the Abseil tree adds an overload for
absl::StrContains to accept a single character needle for optimized
lookups.
Reviewed By: hokein
Differential Revision: https://reviews.llvm.org/D92810
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 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
Consider this code:
```
if (Cond) {
#ifdef X_SUPPORTED
X();
#else
return;
#endif
} else {
Y();
}
Z();```
In this example, if `X_SUPPORTED` is not defined, currently we'll get a warning from the else-after-return check. However If we apply that fix, and then the code is recompiled with `X_SUPPORTED` defined, we have inadvertently changed the behaviour of the if statement due to the else being removed. Code flow when `Cond` is `true` will be:
```
X();
Y();
Z();```
where as before the fix it was:
```
X();
Z();```
This patch adds checks that guard against `#endif` directives appearing between the control flow interrupter and the else and not applying the fix if they are detected.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D91485
Do not warn for "pointer to aggregate" in a `sizeof(A) / sizeof(A[0])`
expression if `A` is an array of pointers. This is the usual way of
calculating the array length even if the array is of pointers.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D91543
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
Chuanqi Xu