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
The block arguments in dispatch_async() and dispatch_after() are
guaranteed to escape. If those blocks capture any pointers with the
noescape attribute then it is an error.
Added an alias llvm-else-after-return from readability-else-after-return to help enforce one of the llvm coding guidelines.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D82825
Added a 'RefactorConditionVariables' option to control how the check handles condition variables
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D82824
Extend the default string like classes to include `std::basic_string_view`.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D82720
- Added `FixItHint` comments to Check files for the script to mark those checks as offering fix-its when the fix-its are generated in another file.
- Case insensitive file searching when looking for the file a checker code resides in.
Also regenerated the list, sphinx had no issue generating the docs after this.
Reviewed By: sylvestre.ledru
Differential Revision: https://reviews.llvm.org/D81932
Summary:
This check finds macro expansions of `DISALLOW_COPY_AND_ASSIGN(Type)` and
replaces them with a deleted copy constructor and a deleted assignment operator.
Before the `delete` keyword was introduced in C++11 it was common practice to
declare a copy constructor and an assignment operator as a private members. This
effectively makes them unusable to the public API of a class.
With the advent of the `delete` keyword in C++11 we can abandon the
`private` access of the copy constructor and the assignment operator and
delete the methods entirely.
Migration example:
```
lang=dif
class Foo {
private:
- DISALLOW_COPY_AND_ASSIGN(Foo);
+ Foo(const Foo &) = delete;
+ const Foo &operator=(const Foo &) = delete;
};
```
Reviewers: alexfh, hokein, aaron.ballman, njames93
Reviewed By: njames93
Subscribers: Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D80531
Summary:
Finds range-based for loops that can be replaced by a call to ``std::any_of`` or
``std::all_of``. In C++ 20 mode, suggests ``std::ranges::any_of`` or
``std::ranges::all_of``.
For now, no fixits are produced.
Reviewers: aaron.ballman, alexfh, hokein
Subscribers: mgorny, xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77572
Updates the docs to include `MacroDefinition` documentation. The docs are still missing `ObjCIVar` however I don't have a clue about how that looks in code. If someone wants to show the code block needed for the example I'll add that in too.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D80877
Revert "clang-tidy doc: add a note for checkers with an autofix"
This reverts commit dc0f79ea5b.
Revert "add_new_check.py: Update of the template to add an autofix section"
This reverts commit f97f92e5b0.
Summary:
Currently, when looking at a checker documentation, we have to go back
to the whole list or look at the sources to figure out if an autofix
is available or not.
Reviewers: alexfh, aaron.ballman
Subscribers: wuzish, Eugene.Zelenko, nemanjai, kbarton, arphaman, Charusso, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77983
Summary:
Sometimes in templated code Member references are reported as `DependentScopeMemberExpr` because that's what the standard dictates, however in many trivial cases it is easy to resolve the reference to its actual Member.
Take this code:
```
template<typename T>
class A{
int value;
A& operator=(const A& Other){
value = Other.value;
this->value = Other.value;
return *this;
}
};
```
When ran with `clang-tidy file.cpp -checks=readability-identifier-naming --config="{CheckOptions: [{key: readability-identifier-naming.MemberPrefix, value: m_}]}" -fix`
Current behaviour:
```
template<typename T>
class A{
int m_value;
A& operator=(const A& Other){
m_value = Other.value;
this->value = Other.value;
return *this;
}
};
```
As `this->value` and `Other.value` are Dependent they are ignored when creating the fix-its, however this can easily be resolved.
Proposed behaviour:
```
template<typename T>
class A{
int m_value;
A& operator=(const A& Other){
m_value = Other.m_value;
this->m_value = Other.m_value;
return *this;
}
};
```
Reviewers: aaron.ballman, JonasToth, alexfh, hokein, gribozavr2
Reviewed By: aaron.ballman
Subscribers: merge_guards_bot, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D73052
Summary:
Added `DiagnoseSignedUnsignedCharComparisons` option to
filter out unrelated use cases. The SEI cert catches explicit
integer casts (two use cases), while in the case of
`signed char` \ `unsigned char` comparison, we have implicit
conversions.
Reviewers: aaron.ballman
Reviewed By: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D79334
Summary:
To cover STR34-C rule's second use case, where ``signed char`` is
used for array subscript after an integer conversion. In the case
of non-ASCII character this conversion will result in a value
in excess of UCHAR_MAX.
There is another clang-tidy check which catches these cases.
cppcoreguidelines-pro-bounds-constant-array-index catches any
indexing which is not integer constant. I think this check is
very strict about the index (e.g. constant), so it's still useful
to cover the ``signed char`` use case in this check, so we
can provide a way to catch the SEI cert rule's use cases on a
codebase, where this CPP guideline is not used.
Reviewers: aaron.ballman, njames93
Reviewed By: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D78904
Summary:
This check will ensure that all calls to functions resolve to one inside the `__llvm_libc` namespace.
This is done to ensure that if we include a public header then we don't accidentally call into the a function within the global namespace.
Reviewers: aaron.ballman, njames93
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, mgorny, xazax.hun, cfe-commits, sivachandra
Tags: #clang-tools-extra, #libc-project, #clang
Differential Revision: https://reviews.llvm.org/D78890
Summary:
Without this patch clang-tidy stops finding file configs on the nearest
.clang-tidy file. In some cases it is not very convenient because it
results in common parts duplication into every child .clang-tidy file.
This diff adds optional config inheritance from the parent directories
config files.
Test Plan:
Added test cases in existing config test.
Reviewers: alexfh, gribozavr2, klimek, hokein
Subscribers: njames93, arphaman, xazax.hun, aheejin, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D75184
Summary: This check is similar to an ARC Migration check that warned about this incorrect usage under ARC, but most projects are no longer undergoing migration from pre-ARC code. The documentation for NSInvocation is not explicit about these requirements and incorrect usage has been found in many of our projects.
Reviewers: stephanemoore, benhamilton, dmaclach, alexfh, aaron.ballman, hokein, njames93
Reviewed By: stephanemoore, benhamilton, aaron.ballman
Subscribers: xazax.hun, Eugene.Zelenko, mgorny, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D77571
Summary:
Made llvmlibc::RestrictSystemLibcHeadersCheck a subclass of protability::RestrictSystemIncludesCheck to re-use common code between the two.
This also adds the ability to white list linux development headers.
Reviewers: aaron.ballman
Reviewed By: aaron.ballman
Subscribers: mgorny, xazax.hun, MaskRay, cfe-commits, sivachandra
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D76395
Summary:
Cover a new use case when using a 'signed char' as an integer
might lead to issue with non-ASCII characters. Comparing
a 'signed char' with an 'unsigned char' using equality / unequality
operator produces an unexpected result for non-ASCII characters.
Reviewers: aaron.ballman, alexfh, hokein, njames93
Reviewed By: njames93
Subscribers: xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D75749
Summary: This adds a new module to enforce standards specific to the llvm-libc project. This change also adds the first check which restricts user from including system libc headers accidentally which can lead to subtle bugs that would be a challenge to detect.
Reviewers: alexfh, hokein, aaron.ballman
Reviewed By: aaron.ballman
Subscribers: juliehockett, arphaman, jfb, abrachet, sivachandra, Eugene.Zelenko, njames93, mgorny, xazax.hun, MaskRay, cfe-commits
Tags: #clang-tools-extra, #libc-project, #clang
Differential Revision: https://reviews.llvm.org/D75332
Summary:
Created a general check for restrict-system-includes under portability as recommend in the comments under D75332. I also fleshed out the user facing documentation to show examples for common use-cases such as allow-list, block-list, and wild carding.
Removed fuchsia's check as per phosek sugguestion.
Reviewers: aaron.ballman, phosek, alexfh, hokein, njames93
Reviewed By: phosek
Subscribers: Eugene.Zelenko, mgorny, xazax.hun, phosek, cfe-commits, MaskRay
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D75786
Summary:
Also use //check// in add_new_check.py for terminology consistency.
PS
My GitHub ID is [[ https://github.com/EugeneZelenko | EugeneZelenko ]], if it's necessary for attribution.
Reviewers: alexfh, hokein, aaron.ballman, njames93, MyDeveloperDay
Reviewed By: njames93
Subscribers: Andi, xazax.hun, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D73580
Added FixItHint comments to ReservedIdentifierCheck and IdentifierNamingCheck to trick the python scripts into detecting a fix it is provided as it can't see the FixItHints in RenamerClangTidyCheck.cpp
Summary:
Recursion is a powerful tool, but like any tool
without care it can be dangerous. For example,
if the recursion is unbounded, you will
eventually run out of stack and crash.
You can of course track the recursion depth
but if it is hardcoded, there can always be some
other environment when that depth is too large,
so said magic number would need to be env-dependent.
But then your program's behavior is suddenly more env-dependent.
Also, recursion, while it does not outright stop optimization,
recursive calls are less great than normal calls,
for example they hinder inlining.
Recursion is banned in some coding guidelines:
* SEI CERT DCL56-CPP. Avoid cycles during initialization of static objects
* JPL 2.4 Do not use direct or indirect recursion.
* I'd say it is frowned upon in LLVM, although not banned
And is plain unsupported in some cases:
* OpenCL 1.2, 6.9 Restrictions: i. Recursion is not supported.
So there's clearly a lot of reasons why one might want to
avoid recursion, and replace it with worklist handling.
It would be great to have a enforcement for it though.
This implements such a check.
Here we detect both direct and indirect recursive calls,
although since clang-tidy (unlike clang static analyzer)
is CTU-unaware, if the recursion transcends a single standalone TU,
we will naturally not find it :/
The algorithm is pretty straight-forward:
1. Build call-graph for the entire TU.
For that, the existing `clang::CallGraph` is re-used,
although it had to be modified to also track the location of the call.
2. Then, the hard problem: how do we detect recursion?
Since we have a graph, let's just do the sane thing,
and look for Strongly Connected Function Declarations - widely known as `SCC`.
For that LLVM provides `llvm::scc_iterator`,
which is internally an Tarjan's DFS algorithm, and is used throught LLVM,
so this should be as performant as possible.
3. Now that we've got SCC's, we discard those that don't contain loops.
Note that there may be more than one loop in SCC!
4. For each loopy SCC, we call out each function, and print a single example
call graph that shows recursion -- it didn't seem worthwhile enumerating
every possible loop in SCC, although i suppose it could be implemented.
* To come up with that call graph cycle example, we start at first SCC node,
see which callee of the node is within SCC (and is thus known to be in cycle),
and recurse into it until we hit the callee that is already in call stack.
Reviewers: JonasToth, aaron.ballman, ffrankies, Eugene.Zelenko, erichkeane, NoQ
Reviewed By: aaron.ballman
Subscribers: Charusso, Naghasan, bader, riccibruno, mgorny, Anastasia, xazax.hun, cfe-commits
Tags: #llvm, #clang
Differential Revision: https://reviews.llvm.org/D72362
Summary: Such implementations may override the class's own implementation, and even be a danger in case someone later comes and adds one to the class itself. Most times this has been encountered have been a mistake.
Reviewers: stephanemoore, benhamilton, dmaclach
Reviewed By: stephanemoore, benhamilton, dmaclach
Subscribers: dmaclach, mgorny, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D72876
Summary: Adds an option called `AddConstToQualified` to readability-qualified-auto to toggle adding const to the auto typed pointers and references. By default its enabled but in the LLVM module its disabled.
Reviewers: aaron.ballman, alexfh, JonasToth, hokein, sammccall
Reviewed By: aaron.ballman
Subscribers: Quuxplusone, merge_guards_bot, lebedev.ri, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D73548
Summary:
I added subsubsections for typical Clang-tidy entries in Release Notes, so now scripts are aware of this changes.
I don't have GitHub commit access, so please commit changes.
Reviewers: aaron.ballman, alexfh, hokein
Reviewed By: alexfh
Subscribers: njames93, xazax.hun, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D72527
Summary:
The original behaviour of this check only looked at VarDecls with strings that had an empty string initializer. This has been improved to check for FieldDecls with an in class initializer as well as constructor initializers.
Addresses [[ https://bugs.llvm.org/show_bug.cgi?id=44474 | clang-tidy "modernize-use-default-member-init"/"readability-redundant-string-init" and redundant initializer of std::string ]]
Reviewers: aaron.ballman, alexfh, hokein
Reviewed By: aaron.ballman
Subscribers: merge_guards_bot, mgorny, Eugene.Zelenko, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D72448
Summary:
Typically most main functions have the signature:
```
int main(int argc, char *argv[])
```
To stick with convention when renaming parameters we should ignore the `argc` and `argv` names even if the parameter style says they should be renamed. This patch addresses this by checking all ParmVarDecls if they form part of a function with a signature that matches main `int name(int argc, char * argv[], (optional char *env[]))`
Reviewers: aaron.ballman, JonasToth, alexfh, hokein
Reviewed By: aaron.ballman
Subscribers: Mordante, merge_guards_bot, xazax.hun, kristof.beyls, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D73098
Summary:
Typically most main functions have the signature:
```
int main(int argc, char *argv[])
```
To stick with convention when renaming parameters we should ignore the `argc` and `argv` names even if the parameter style says they should be renamed. This patch addresses this by checking all ParmVarDecls if they form part of a function with a signature that matches main `int name(int argc, char * argv[], (optional char *env[]))`
Reviewers: aaron.ballman, JonasToth, alexfh, hokein
Reviewed By: aaron.ballman
Subscribers: Mordante, merge_guards_bot, xazax.hun, kristof.beyls, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D73098
Finds cases where an integer expression is added to the result
of a memory allocation function instead of its argument.
Differential Revision: https://reviews.llvm.org/D71001
This patch adds bugprone-reserved-identifier, which flags uses of __names _Like
::_this, which are reserved for the implementation. The check can optionally be
inverted, i.e. configured to flag any names that are _not_ reserved, which may
be useful for e.g. standard library implementors.
Adds a check that detects any auto variables that are deduced to a pointer or
a const pointer then adds in the const and asterisk according. Will also
check auto L value references that could be written as const. This relates
to the coding standard
https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto
Major changes are introduction of subsubsections to prevent people
putting new entries in wrong places. I also polished line length and
highlighting.
Patch by Eugene Zelenko!
Summary:
It now handles `typedef`s that include comma-separated multiple types, and handles embedded struct definitions, which previously could not be automatically converted.
For example, with this patch `modernize-use-using` now can convert:
typedef struct { int a; } R_t, *R_p;
to:
using R_t = struct { int a; };
using R_p = R_t*;
`-ast-dump` showed that the `CXXRecordDecl` definitions and multiple `TypedefDecl`s come consecutively in the tree, so `check()` stores information between calls to determine when it is receiving a second or additional `TypedefDecl` within a single `typedef`, or when the current `TypedefDecl` refers to an embedded `CXXRecordDecl` like a `struct`.
Reviewers: alexfh, aaron.ballman
Patch by: poelmanc
Subscribers: riccibruno, sammccall, cfe-commits, aaron.ballman
Tags: clang-tools-extra, clang
Differential Revision: https://reviews.llvm.org/D70270
Summary:
This check searches for signed char -> integer conversions which might
indicate programming error, because of the misinterpretation of char
values. A signed char might store the non-ASCII characters as negative
values. The human programmer probably expects that after an integer
conversion the converted value matches with the character code
(a value from [0..255]), however, the actual value is in
[-128..127] interval.
See also:
STR34-C. Cast characters to unsigned char before converting to larger integer sizes
<https://wiki.sei.cmu.edu/confluence/display/c/STR34-C.+Cast+characters+to+unsigned+char+before+converting+to+larger+integer+sizes>
By now this check is limited to assignment / variable declarations.
If we would catch all signed char -> integer conversion, then it would
produce a lot of findings and also false positives. So I added only
this use case now, but this check can be extended with additional
use cases later.
The CERT documentation mentions another use case when the char is
used for array subscript. Next to that a third use case can be
the signed char - unsigned char comparison, which also a use case
where things happen unexpectedly because of conversion to integer.
Reviewers: alexfh, hokein, aaron.ballman
Reviewed By: aaron.ballman
Subscribers: sylvestre.ledru, whisperity, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D71174
Summary:
> tools/clang/tools/extra
has become
>clang-tools-extra
which was not updated in all docs.
Reviewers: alexfh, aaron.ballman, ilya-biryukov, juliehockett
Reviewed By: aaron.ballman
Subscribers: Jim, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D71982
Summary:
Currently, the list isn't very useful.
This change adds two tables.
* The checkers
* The aliases
For each checkers, we provide extract info:
* the severity. Taken from codechecker - https://github.com/Ericsson/codechecker/blob/master/config/checker_severity_map.json
* if the checker has an autofix or not
I used the cvs format for the table because:
* it is easy
* the data could be reused by other tools (we could move
that into a separated / generated file at some point)
Reviewers: alexfh, jdoerfert, jfb, lebedev.ri, Eugene.Zelenko
Subscribers: dexonsmith, wuzish, nemanjai, kbarton, arphaman, lebedev.ri, whisperity, Eugene.Zelenko, JonasToth, JDevlieghere, xazax.hun, cfe-commits, #clang-tools-extra
Tags: #clang
Differential Revision: https://reviews.llvm.org/D36051
This represents largely a full re-write of modernize-avoid-bind, adding
significant new functionality in the process. In particular:
* Both boost::bind and std::bind are now supported
* Function objects are supported in addition to functions
* Member functions are supported
* Nested calls are supported using capture-init syntax
* std::ref() and boost::ref() are now recognized, and will capture by reference.
* Rather than capturing with a global =, we now build up an individual
capture list that is both necessary and sufficient for the call.
* Fixits are supported in a much larger variety of scenarios than before.
All previous tests pass under the re-write, but a large number of new
tests have been added as well.
Differential Revision: https://reviews.llvm.org/D70368
Summary:
`modernize-use-equals-default` replaces default constructors/destructors with `= default;`. When the optional semicolon after a member function is present, this results in two consecutive semicolons.
This patch checks to see if the next non-comment token after the code to be replaced is a semicolon, and if so offers a replacement of `= default` rather than `= default;`.
This patch adds trailing comments and semicolons to about 5 existing tests.
Reviewers: malcolm.parsons, angelgarcia, aaron.ballman, alexfh
Patch by: poelmanc
Subscribers: MyDeveloperDay, JonasToth, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D70144
Summary:
readability-redundant-member-init removes redundant / unnecessary member and base class initialization. Unfortunately for the specific case of a copy constructor's initialization of a base class, gcc at strict warning levels warns if "base class is not initialized in the copy constructor of a derived class".
This patch adds an option `IgnoreBaseInCopyConstructors` defaulting to 0 (thus maintaining current behavior by default) to skip the specific case of removal of redundant base class initialization in the copy constructor. Enabling this option enables the resulting code to continue to compile successfully under `gcc -Werror=extra`. New test cases `WithCopyConstructor1` and `WithCopyConstructor2` in clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp show that it removes redundant members even from copy constructors.
Reviewers: malcolm.parsons, alexfh, hokein, aaron.ballman, lebedev.ri
Patch by: poelmanc
Subscribers: mgehre, lebedev.ri, cfe-commits
Tags: #clang, #clang-tools-extra
Differential revision: https://reviews.llvm.org/D69145
Summary:
In addition to adding `override` wherever possible, clang-tidy's `modernize-use-override` nicely removes `virtual` when `override` or `final` is specified, and further removes override when final is specified. While this is great default behavior, when code needs to be compiled with gcc at high warning levels that include `gcc -Wsuggest-override` or `gcc -Werror=suggest-override`, clang-tidy's removal of the redundant `override` keyword causes gcc to emit a warning or error. This discrepancy / conflict has been noted by others including a comment on Stack Overflow and by Mozilla's Firefox developers.
This patch adds an AllowOverrideAndFinal option defaulting to 0 - thus preserving current behavior - that when enabled allows both `override` and `final` to co-exist, while still fixing all other issues.
The patch includes a test file verifying all combinations of virtual/override/final, and mentions the new option in the release notes.
Reviewers: alexfh, djasper, JonasToth
Patch by: poelmanc
Subscribers: JonasToth, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D70165
Summary:
This patch adds a feature requested in https://reviews.llvm.org/D69238 to enable `readability-redundant-string-init` to take a list of strings to apply the fix to rather than hard-coding `basic_string`. It adds a `StringNames` option of semicolon-delimited names of string classes to which to apply this fix. Tests ensure this works with test class out::TestString as well as std::string and std::wstring as before. It should be applicable to llvm::StringRef, QString, etc.
Note: This commit was previously reverted due to a failing unit test. That test has been fixed in this version.
Reviewers: MyDeveloperDay, aaron.ballman, hokein, alexfh, JonasToth, gribozavr2
Patch by: poelmanc
Subscribers: gribozavr2, xazax.hun, Eugene.Zelenko, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D69548
Summary:
`readability-redundant-string-init` was one of several clang-tidy checks documented as failing for C++17. (The failure mode in C++17 is that it changes `std::string Name = ""`; to `std::string Name = Name;`, which actually compiles but crashes at run-time.)
Analyzing the AST with `clang -Xclang -ast-dump` showed that the outer `CXXConstructExprs` that previously held the correct SourceRange were being elided in C++17/2a, but the containing `VarDecl` expressions still had all the relevant information. So this patch changes the fix to get its source ranges from `VarDecl`.
It adds one test `std::string g = "u", h = "", i = "uuu", j = "", k;` to confirm proper warnings and fixit replacements in a single `DeclStmt` where some strings require replacement and others don't. The readability-redundant-string-init.cpp and readability-redundant-string-init-msvc.cpp tests now pass for C++11/14/17/2a.
Reviewers: gribozavr, etienneb, alexfh, hokein, aaron.ballman, gribozavr2
Patch by: poelmanc
Subscribers: NoQ, MyDeveloperDay, Eugene.Zelenko, dylanmckay, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D69238
Summary:
This patch adds a feature requested in https://reviews.llvm.org/D69238 to enable `readability-redundant-string-init` to take a list of strings to apply the fix to rather than hard-coding `basic_string`. It adds a `StringNames` option of semicolon-delimited names of string classes to which to apply this fix. Tests ensure this works with test class out::TestString as well as std::string and std::wstring as before. It should be applicable to llvm::StringRef, QString, etc.
Reviewers: MyDeveloperDay, aaron.ballman, hokein, alexfh, JonasToth, gribozavr2
Patch by: poelmanc
Subscribers: gribozavr2, xazax.hun, Eugene.Zelenko, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D69548
Summary:
Finds non-static member functions that can be made ``const``
because the functions don't use ``this`` in a non-const way.
The check conservatively tries to preserve logical costness in favor of
physical costness. See readability-make-member-function-const.rst for more
details.
Reviewers: aaron.ballman, gribozavr, hokein, alexfh
Subscribers: mgorny, xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D68074
Checks for types which can be made trivially-destructible by removing
out-of-line defaulted destructor declarations.
The check is motivated by the work on C++ garbage collector in Blink
(rendering engine for Chrome), which strives to minimize destructors and
improve runtime of sweeping phase.
In the entire chromium codebase the check hits over 2000 times.
Differential Revision: https://reviews.llvm.org/D69435
Summary:
These changes were generated by invoking
clang-tools-extra/clang-tidy/add_new_check.py and then reverting the
check that was added.
Reviewers: aaron.ballman
Reviewed By: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D69414
Nathan James