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:
New checker called bugprone-not-null-terminated-result. This checker finds
function calls where it is possible to cause a not null-terminated result.
Usually the proper length of a string is `strlen(src) + 1` or equal length
of this expression, because the null terminator needs an extra space.
Without the null terminator it can result in undefined behaviour when the
string is read.
The following and their respective `wchar_t` based functions are checked:
`memcpy`, `memcpy_s`, `memchr`, `memmove`, `memmove_s`, `strerror_s`,
`strncmp`, `strxfrm`
The following is a real-world example where the programmer forgot to
increase the passed third argument, which is `size_t length`.
That is why the length of the allocated memory is not enough to hold the
null terminator.
```
static char *stringCpy(const std::string &str) {
char *result = reinterpret_cast<char *>(malloc(str.size()));
memcpy(result, str.data(), str.size());
return result;
}
```
In addition to issuing warnings, fix-it rewrites all the necessary code.
It also tries to adjust the capacity of the destination array:
```
static char *stringCpy(const std::string &str) {
char *result = reinterpret_cast<char *>(malloc(str.size() + 1));
strcpy(result, str.data());
return result;
}
```
Note: It cannot guarantee to rewrite every of the path-sensitive memory
allocations.
Reviewed By: JonasToth, aaron.ballman, whisperity, alexfh
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D45050
llvm-svn: 374707
This checks finds all primitive type local variables (integers, doubles, pointers) that are declared without an initial value. Includes fixit functionality to initialize said variables with a default value. This is zero for most types and NaN for floating point types. The use of NaNs is copied from the D programming language.
Patch by Jussi Pakkanen.
llvm-svn: 373489
Summary:
OSSpinLock* are Apple/Darwin functions, but were previously located with ObjC checks as those were most closely tied to Apple platforms before.
Now that there's a specific Darwin module, relocating the check there.
This change was prepared by running rename_check.py.
Contributed By: mwyman
Reviewers: stephanemoore, dmaclach
Reviewed By: stephanemoore
Subscribers: Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra, #clang, #llvm
Differential Revision: https://reviews.llvm.org/D68148
llvm-svn: 373392
Summary:
Creates a new darwin ClangTidy module and adds the darwin-dispatch-once-nonstatic check that warns about dispatch_once_t variables not in static or global storage. This catches a missing static for local variables in e.g. singleton initialization behavior, and also warns on storing dispatch_once_t values in Objective-C instance variables. C/C++ struct/class instances may potentially live in static/global storage, and are ignored for this check.
The osx.API static analysis checker can find the non-static storage use of dispatch_once_t; I thought it useful to also catch this issue in clang-tidy when possible.
This is a re-land of https://reviews.llvm.org/D67567
Reviewers: thakis, gribozavr, stephanemoore
Subscribers: Eugene.Zelenko, mgorny, xazax.hun, jkorous, arphaman, kadircet, usaxena95
Tags: #clang-tools-extra, #clang, #llvm
Differential Revision: https://reviews.llvm.org/D68109
llvm-svn: 373065
Summary:
Creates a new darwin ClangTidy module and adds the darwin-dispatch-once-nonstatic check that warns about dispatch_once_t variables not in static or global storage. This catches a missing static for local variables in e.g. singleton initialization behavior, and also warns on storing dispatch_once_t values in Objective-C instance variables. C/C++ struct/class instances may potentially live in static/global storage, and are ignored for this check.
The osx.API static analysis checker can find the non-static storage use of dispatch_once_t; I thought it useful to also catch this issue in clang-tidy when possible.
Contributed By: mwyman
Reviewers: benhamilton, hokein, stephanemoore, aaron.ballman, gribozavr
Reviewed By: stephanemoore, gribozavr
Subscribers: jkorous, arphaman, kadircet, usaxena95, NoQ, xazax.hun, lebedev.ri, mgorny, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D67567
llvm-svn: 373028
Finding infinite loops is well-known to be impossible (halting problem).
However, it is possible to detect some obvious infinite loops, for example,
if the loop condition is not changed. Detecting such loops is beneficial
since the tests will hang on programs containing infinite loops so
testing-time detection may be costly in large systems. Obvious cases are
where the programmer forgets to increment/decrement the counter or
increments/decrements the wrong variable.
Differential Revision: https://reviews.llvm.org/D64736
-------
Broke some buildbots "No SOURCES given to target: obj.clangTidyBugproneModule"
llvm-svn: 372704
Finding infinite loops is well-known to be impossible (halting problem).
However, it is possible to detect some obvious infinite loops, for example,
if the loop condition is not changed. Detecting such loops is beneficial
since the tests will hang on programs containing infinite loops so
testing-time detection may be costly in large systems. Obvious cases are
where the programmer forgets to increment/decrement the counter or
increments/decrements the wrong variable.
Differential Revision: https://reviews.llvm.org/D64736
llvm-svn: 372693
Summary:
Apple documentation states that:
"If two objects are equal, they must have the same hash value. This last
point is particularly important if you define isEqual: in a subclass and
intend to put instances of that subclass into a collection. Make sure
you also define hash in your subclass."
https://developer.apple.com/documentation/objectivec/1418956-nsobject/1418795-isequal?language=objc
In many or all versions of libobjc, -[NSObject isEqual:] is a pointer
equality check and -[NSObject hash] returns the messaged object's
pointer. A relatively common form of developer error is for a developer to
override -isEqual: in a subclass without overriding -hash to ensure that
hashes are equal for objects that are equal.
It is assumed that an override of -isEqual: is a strong signal for
changing the object's equality operator to something other than pointer
equality which implies that a missing override of -hash could result in
distinct objects being equal but having distinct hashes because they are
independent instances. This added check flags classes that override
-isEqual: but inherit NSObject's implementation of -hash to warn of the
potential for unexpected behavior.
The proper implementation of -hash is the responsibility of the
developer and the check will only verify that the developer made an
effort to properly implement -hash. Developers can set up unit tests
to verify that their implementation of -hash is appropriate.
Test Notes:
Ran check-clang-tools.
Reviewers: aaron.ballman, benhamilton
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D67737
llvm-svn: 372445
This check now also checks if any calls to pthread_* functions expect negative return values. These functions return either 0 on success or an errno on failure, which is positive only.
llvm-svn: 372037
The clang-tidy-vs visual studio plugin in clang-tools-extra contains a
security vulnerability in the YamlDotNet package [1]. I posted to cfe-dev [2],
asking if there was anyone who was interested in updating the the plugin
to address the vulnerability. Reid mentioned that Zach (the original committer),
said that there's another plugin (Clang Power Tools) that provides clang-tidy support,
with additional extra features, so it would be ok to remove clang-tidy-vs.
This commit removes the plugin to address the security vulnerability, and adds
a section to the release notes that mentions that the plugin was removed, and
suggests to use Clang Power Tools.
Fixes PR 41791.
[1]: https://nvd.nist.gov/vuln/detail/CVE-2018-1000210
[2]: http://lists.llvm.org/pipermail/cfe-dev/2019-August/063196.html
Differential Revision: https://reviews.llvm.org/D66813
llvm-svn: 370096
Finds instances where variables with static storage are initialized dynamically in header files.
Reviewed By: aaron.ballman, alexfh
Patch by Charles Zhang!
Differential Revision: https://reviews.llvm.org/D62829
llvm-svn: 369568
HTML generator has been included in clang-tools-extra release notes.
clang-doc documentation file has been updated.
Differential Revision: https://reviews.llvm.org/D65622
llvm-svn: 367743
Summary:
This changes ReleaseNotes.txt to have the first sentence of the full
documentation from linuxkernel-must-use-errs.rst.
This addresses a comment from the review of rL367071 in
https://reviews.llvm.org/D59963.
Reviewers: Eugene.Zelenko
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D65343
llvm-svn: 367333
Introduce a new check to upgrade user code based on API changes in Googletest.
The check finds uses of old Googletest APIs with "case" in their name and replaces them with the new APIs named with "suite".
Patch by Alex Strelnikov (strel@google.com)
Reviewed as D62977.
llvm-svn: 367263
Summary:
Now that clang is going to be able to build the Linux kernel again on
x86, and we have gen_compile_commands.py upstream for generating
compile_commands.json, clang-tidy can be used on the Linux kernel
source.
To that end, this commit adds a new clang-tidy module to be used for
checks specific to Linux kernel source. The Linux kernel follows its own
style of C, and it will be useful to separate those checks into their
own module.
This also adds an initial check that makes sure that return values from
the kernel error functions like PTR_ERR and ERR_PTR are checked. It also
makes sure that any functions that directly return values from these
functions are checked.
Subscribers: xazax.hun, gribozavr, Eugene.Zelenko, lebedev.ri, mgorny, jdoerfert, cfe-commits
Tags: #clang, #clang-tools-extra
Reviewers: aaron.ballman, alexfh, hokein, JonasToth
Differential Revision: https://reviews.llvm.org/D59963
llvm-svn: 367071
Summary:
Finds non-static member functions that can be made ``static``.
I have run this check (repeatedly) over llvm-project. It made 1708 member functions
``static``. Out of those, I had to exclude 22 via ``NOLINT`` because their address
was taken and stored in a variable of pointer-to-member type (e.g. passed to
llvm::StringSwitch).
It also made 243 member functions ``const``. (This is currently very conservative
to have no false-positives and can hopefully be extended in the future.)
You can find the results here: https://github.com/mgehre/llvm-project/commits/static_const_eval
Reviewers: alexfh, aaron.ballman
Subscribers: mgorny, xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D61749
llvm-svn: 366265
Summary:
Checks if any calls to posix functions (except posix_openpt) expect negative return values.
These functions return either 0 on success or an errno on failure, which is positive only.
Reviewers: JonasToth, gribozavr, alexfh, hokein
Reviewed By: gribozavr
Subscribers: Eugene.Zelenko, lebedev.ri, llozano, george.burgess.iv, xazax.hun, srhines, mgorny, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D63623
Patch by Jian Cai.
llvm-svn: 365007
Splits fuchsia-default-arguments check into two checks. fuchsia-default-arguments-calls warns if a function or method is called with default arguments. fuchsia-default-arguments-declarations warns if a function or method is declared with default parameters.
Committed on behalf of @diegoast (Diego Astiazarán).
Resolves b38051.
Differential Revision: https://reviews.llvm.org/D62437
llvm-svn: 363712
On Android, pipe() is better to be replaced by pipe2() with O_CLOEXEC
flag to avoid file descriptor leakage.
Patch by Jian Cai!
Differential Revision: https://reviews.llvm.org/D61967
llvm-svn: 362673
On Android, pipe2() is better to set O_CLOEXEC flag to avoid file
descriptor leakage.
Patch by Jian Cai!
Differential Revision: https://reviews.llvm.org/D62049
llvm-svn: 362672
Summary:
Added WarnOnlyIfThisHasSuspiciousField option to allow
to catch any copy assignment operator independently from
the container class's fields.
Added the cert alias using this option.
Reviewers: aaron.ballman
Reviewed By: aaron.ballman
Subscribers: mgorny, Eugene.Zelenko, xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D62192
llvm-svn: 361550
Catching trivial objects by value is not dangerous but may be
inefficient if they are too large. This patch adds an option
`WarnOnLargeObject` to the checker to also warn if such an object
is caught by value. An object is considered as "large" if its
size is greater than `MaxSize` which is another option. Default
value is the machine word of the architecture (size of the type
`size_t`).
Differential Revision: https://reviews.llvm.org/D61851
llvm-svn: 361225
Implement a check for detecting if/else if/else chains where two or more
branches are Type I clones of each other (that is, they contain identical code)
and for detecting switch statements where two or more consecutive branches are
Type I clones of each other.
Patch by Donát Nagy!
Differential Revision: https://reviews.llvm.org/D54757
llvm-svn: 360779
Summary:
This check searches for copy assignment operators which might not handle self-assignment properly. There are three patterns of
handling a self assignment situation: self check, copy-and-swap or the less common copy-and-move. The new check warns if none of
these patterns is found in a user defined implementation.
See also:
OOP54-CPP. Gracefully handle self-copy assignment
https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP54-CPP.+Gracefully+handle+self-copy+assignment
Reviewers: JonasToth, alexfh, hokein, aaron.ballman
Subscribers: riccibruno, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D60507
llvm-svn: 360540
Add the modernize-use-trailing-return check to rewrite function signatures to use trailing return types.
Patch by Bernhard Manfred Gruber.
llvm-svn: 360438
Summary:
Looks at conditionals and finds cases of ``cast<>``, which will
assert rather than return a null pointer, and ``dyn_cast<>`` where
the return value is not captured. Additionally, finds cases that
match the pattern ``var.foo() && isa<X>(var.foo())``, where the
method is called twice and could be expensive.
.. code-block:: c++
// Finds cases like these:
if (auto x = cast<X>(y)) <...>
if (cast<X>(y)) <...>
// But not cases like these:
if (auto f = cast<Z>(y)->foo()) <...>
if (cast<Z>(y)->foo()) <...>
Reviewers: alexfh, rjmccall, hokein, aaron.ballman, JonasToth
Reviewed By: aaron.ballman
Subscribers: xbolva00, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D59802
llvm-svn: 359142
Summary:
This check aims to address a relatively common benign error where
Objective-C subclass initializers call -self on their superclass instead
of invoking a superclass initializer, typically -init. The error is
typically benign because libobjc recognizes that improper initializer
chaining is common¹.
One theory for the frequency of this error might be that -init and -self
have the same return type which could potentially cause inappropriate
autocompletion to -self instead of -init. The equal selector lengths and
triviality of common initializer code probably contribute to errors like
this slipping through code review undetected.
This check aims to flag errors of this form in the interests of
correctness and reduce incidence of initialization failing to chain to
-[NSObject init].
[1] "In practice, it will be hard to rely on this function.
Many classes do not properly chain -init calls."
From _objc_rootInit in https://opensource.apple.com/source/objc4/objc4-750.1/runtime/NSObject.mm.auto.html.
Test Notes:
Verified via `make check-clang-tools`.
Subscribers: mgorny, xazax.hun, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59806
llvm-svn: 358620
Summary:
The bugprone-too-small-loop-variable check often catches loop variables which can represent "big enough" values, so we don't actually need to worry about that this variable will overflow in a loop when the code iterates through a container. For example a 32 bit signed integer type's maximum value is 2 147 483 647 and a container's size won't reach this maximum value in most of the cases.
So the idea of this option to allow the user to specify an upper limit (using magnitude bit of the integer type) to filter out those catches which are not interesting for the user, so he/she can focus on the more risky integer incompatibilities.
Next to the option I replaced the term "positive bits" to "magnitude bits" which seems a better naming both in the code and in the name of the new option.
Reviewers: JonasToth, alexfh, aaron.ballman, hokein
Reviewed By: JonasToth
Subscribers: Eugene.Zelenko, xazax.hun, jdoerfert, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D59870
llvm-svn: 358356
Makes the name of this directory consistent with the names of the other
directories in clang-tools-extra.
Similar to r356254. No intended behavior change.
Differential Revision: https://reviews.llvm.org/D59750
llvm-svn: 356897
Summary:
Finally, we are here!
Analyzes OpenMP Structured Blocks and checks that no exception escapes
out of the Structured Block it was thrown in.
As per the OpenMP specification, structured block is an executable statement,
possibly compound, with a single entry at the top and a single exit at the
bottom. Which means, ``throw`` may not be used to to 'exit' out of the
structured block. If an exception is not caught in the same structured block
it was thrown in, the behaviour is undefined / implementation defined,
the program will likely terminate.
Reviewers: JonasToth, aaron.ballman, baloghadamsoftware, gribozavr
Reviewed By: aaron.ballman, gribozavr
Subscribers: mgorny, xazax.hun, rnkovacs, guansong, jdoerfert, cfe-commits, ABataev
Tags: #clang-tools-extra, #openmp, #clang
Differential Revision: https://reviews.llvm.org/D59466
llvm-svn: 356802
Summary:
Finds OpenMP directives that are allowed to contain `default` clause,
but either don't specify it, or the clause is specified but with the kind
other than `none`, and suggests to use `default(none)` clause.
Using `default(none)` clause changes the default variable visibility from
being implicitly determined, and thus forces developer to be explicit about the
desired data scoping for each variable.
Reviewers: JonasToth, aaron.ballman, xazax.hun, hokein, gribozavr
Reviewed By: JonasToth, aaron.ballman
Subscribers: jdoerfert, openmp-commits, klimek, sbenza, arphaman, Eugene.Zelenko, ABataev, mgorny, rnkovacs, guansong, cfe-commits
Tags: #clang-tools-extra, #openmp, #clang
Differential Revision: https://reviews.llvm.org/D57113
llvm-svn: 356801
Summary:
In contrast to Google C++, Objective-C often uses built-in integer types
other than `int`. In fact, the Objective-C runtime itself defines the
types NSInteger¹ and NSUInteger² which are variant types depending on
the target architecture. The Objective-C style guide indicates that
usage of system types with variant sizes is appropriate when handling
values provided by system interfaces³. Objective-C++ is commonly the
result of conversion from Objective-C to Objective-C++ for the purpose
of integrating C++ functionality. The opposite of Objective-C++ being
used to expose Objective-C functionality to C++ is less common,
potentially because Objective-C has a signficantly more uneven presence
on different platforms compared to C++. This generally predisposes
Objective-C++ to commonly being more Objective-C than C++. Forcing
Objective-C++ developers to perform conversions between variant system types
and fixed size integer types depending on target architecture when
Objective-C++ commonly uses variant system types from Objective-C is
likely to lead to more bugs and overhead than benefit. For that reason,
this change proposes to disable google-runtime-int in Objective-C++.
[1] https://developer.apple.com/documentation/objectivec/nsinteger?language=objc
[2] https://developer.apple.com/documentation/objectivec/nsuinteger?language=objc
[3] "Types long, NSInteger, NSUInteger, and CGFloat vary in size between
32- and 64-bit builds. Use of these types is appropriate when handling
values exposed by system interfaces, but they should be avoided for most
other computations."
https://github.com/google/styleguide/blob/gh-pages/objcguide.md#types-with-inconsistent-sizes
Subscribers: xazax.hun, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59336
llvm-svn: 356627
Summary:
-ignore specifies a list of PP callbacks to ignore. It cannot express a
whitelist, which may be more useful than a blacklist.
Add a new option -callbacks to replace it.
-ignore= (default) => -callbacks='*' (default)
-ignore=FileChanged,FileSkipped => -callbacks='*,-FileChanged,-FileSkipped'
-callbacks='Macro*' : print only MacroDefined,MacroExpands,MacroUndefined,...
Reviewers: juliehockett, aaron.ballman, alexfh, ioeric
Reviewed By: aaron.ballman
Subscribers: nemanjai, kbarton, jsji, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59296
llvm-svn: 356366
This is an analog of the abseil-duration-comparison check, but for the
absl::Time domain. It has a similar implementation and tests.
Differential Revision: https://reviews.llvm.org/D58977
llvm-svn: 355835
Summary:
The usefulness of **modernize-use-override** can be reduced if you have to live in an environment where you support multiple compilers, some of which sadly are not yet fully C++11 compliant
some codebases have to use override as a macro OVERRIDE e.g.
```
// GCC 4.7 supports explicit virtual overrides when C++11 support is enabled.
```
This allows code to be compiled with C++11 compliant compilers and get warnings and errors that clang, MSVC,gcc can give, while still allowing other legacy pre C++11 compilers to compile the code. This can be an important step towards modernizing C++ code whilst living in a legacy codebase.
When it comes to clang tidy, the use of the **modernize-use-override** is one of the most useful checks, but the messages reported are inaccurate for that codebase if the standard approach is to use the macros OVERRIDE and/or FINAL.
When combined with fix-its that introduce the C++11 override keyword, they become fatal, resulting in the modernize-use-override check being turned off to prevent the introduction of such errors.
This revision, allows the possibility for the replacement **override **to be a macro instead, Allowing the clang-tidy check to be run on both pre and post C++11 code, and allowing fix-its to be applied.
Reviewers: alexfh, JonasToth, hokein, Eugene.Zelenko, aaron.ballman
Reviewed By: alexfh, JonasToth
Subscribers: lewmpk, malcolm.parsons, jdoerfert, xazax.hun, cfe-commits, llvm-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D57087
llvm-svn: 355132
Summary:
The Acronyms and IncludeDefaultAcronyms options were deprecated in
https://reviews.llvm.org/D51832. These options can be removed.
Tested by running the clang-tidy tests.
This is an amended resubmission of https://reviews.llvm.org/D56945.
Reviewers: Eugene.Zelenko, aaron.ballman
Reviewed By: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D57080
llvm-svn: 354195
bugprone-argument-comment only supports identifying those comments which do not match the function parameter name
This revision add 3 options to adding missing argument comments to literals (granularity on type is added to control verbosity of fixit)
```
CheckOptions:
- key: bugprone-argument-comment.CommentBoolLiterals
value: '1'
- key: bugprone-argument-comment.CommentFloatLiterals
value: '1'
- key: bugprone-argument-comment.CommentIntegerLiterals
value: '1'
- key: bugprone-argument-comment.CommentStringLiterals
value: '1'
- key: bugprone-argument-comment.CommentCharacterLiterals
value: '1'
- key: bugprone-argument-comment.CommentUserDefinedLiterals
value: '1'
- key: bugprone-argument-comment.CommentNullPtrs
value: '1'
```
After applying these options, literal arguments will be preceded with /*ParameterName=*/
Reviewers: JonasToth, Eugene.Zelenko, alexfh, hokein, aaron.ballman
Reviewed By: aaron.ballman, Eugene.Zelenko
Differential Revision: https://reviews.llvm.org/D57674
llvm-svn: 353535
Summary:
The `Acronyms` and `IncludeDefaultAcronyms` options were deprecated in
https://reviews.llvm.org/D51832. These options can be removed.
Tested by running the clang-tidy tests.
Reviewers: benhamilton, aaron.ballman
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D56945
llvm-svn: 351921
Summary: Adds a checker to clang-tidy to warn when a non void const member function, taking only parameters passed by value or const reference could be marked as '[[nodiscard]]'
Patch by MyDeveloperDay.
Reviewers: alexfh, stephenkelly, curdeius, aaron.ballman, hokein, JonasToth
Reviewed By: curdeius, JonasToth
Subscribers: Eugene.Zelenko, lefticus, lebedev.ri, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D55433
llvm-svn: 350760
And also enable it by default to be consistent with e.g.
modernize-use-using.
This helps e.g. when running this check on client code where the macro
is provided by the system, so there is no easy way to modify it.
Reviewed By: JonasToth, lebedev.ri
Differential Revision: https://reviews.llvm.org/D56025
llvm-svn: 350056
Summary:
This check uses the context of a subtraction expression as well as knowledge
about the Abseil Time types, to infer the type of the second operand of some
subtraction expressions in Duration conversions. For example:
absl::ToDoubleSeconds(duration) - foo
can become
absl::ToDoubleSeconds(duration - absl::Seconds(foo))
This ensures that time calculations are done in the proper domain, and also
makes it easier to further deduce the types of the second operands to these
expressions.
Reviewed By: JonasToth
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D55245
llvm-svn: 349073
Patch by Alex Strelnikov.
Reviewed as D53830
Introduce a new check to upgrade user code based on upcoming API breaking changes to absl::Duration.
The check finds calls to arithmetic operators and factory functions for absl::Duration that rely on
an implicit user defined conversion to int64_t. These cases will no longer compile after proposed
changes are released. Suggested fixes explicitly cast the argument int64_t.
llvm-svn: 348633
Summary:
Implement a check for detecting if/else if/else chains where two or more
branches are Type I clones of each other (that is, they contain identical code)
and for detecting switch statements where two or more consecutive branches are
Type I clones of each other.
Patch by donat.nagy.
Reviewers: alexfh, hokein, aaron.ballman, JonasToth
Reviewed By: JonasToth
Subscribers: MTC, lebedev.ri, whisperity, xazax.hun, Eugene.Zelenko, mgorny, rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D54757
llvm-svn: 348343
Summary:
§1 Description
This changes the objc-property-declaration check to allow arbitrary acronyms and initialisms instead of using whitelisted acronyms. In Objective-C it is relatively common to use project prefixes in property names for the purposes of disambiguation. For example, the CIColor¹ and CGColor² properties on UIColor both represent symbol prefixes being used in proeprty names outside of Apple's accepted acronyms³. The union of Apple's accepted acronyms and all symbol prefixes that might be used for disambiguation in property declarations effectively allows for any arbitrary sequence of capital alphanumeric characters to be acceptable in property declarations. This change updates the check accordingly.
The test variants with custom configurations are deleted as part of this change because their configurations no longer impact behavior. The acronym configurations are currently preserved for backwards compatibility of check configuration.
[1] https://developer.apple.com/documentation/uikit/uicolor/1621951-cicolor?language=objc
[2] https://developer.apple.com/documentation/uikit/uicolor/1621954-cgcolor?language=objc
[3] https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
§2 Test Notes
Changes verified by:
• Running clang-tidy unit tests.
• Used check_clang_tidy.py to verify expected output of processing objc-property-declaration.m
Reviewers: benhamilton, Wizard
Reviewed By: benhamilton
Subscribers: jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D51832
llvm-svn: 348331
Summary:
This check finds instances where Duration values are being converted to a numeric value in a comparison expression, and suggests that the conversion happen on the other side of the expression to a Duration. See documentation for examples.
This also shuffles some code around so that the new check may perform in sone step simplifications also caught by other checks.
Compilation is unbroken, because the hash-function is now directly
specified for std::unordered_map, as 'enum class' does not compile as
key (seamingly only on some compilers).
Patch by hwright.
Reviewers: aaron.ballman, JonasToth, alexfh, hokein
Reviewed By: JonasToth
Subscribers: sammccall, Eugene.Zelenko, xazax.hun, cfe-commits, mgorny
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D54737
llvm-svn: 348169
Summary:
This check finds instances where Duration values are being converted to a numeric value in a comparison expression, and suggests that the conversion happen on the other side of the expression to a Duration. See documentation for examples.
This also shuffles some code around so that the new check may perform in sone step simplifications also caught by other checks.
Patch by hwright.
Reviewers: aaron.ballman, JonasToth, alexfh, hokein
Reviewed By: JonasToth
Subscribers: sammccall, Eugene.Zelenko, xazax.hun, cfe-commits, mgorny
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D54737
llvm-svn: 348161
Summary:
§1 Description
This check finds function names in function declarations in Objective-C files that do not follow the naming pattern described in the Google Objective-C Style Guide. Function names should be in UpperCamelCase and functions that are not of static storage class should have an appropriate prefix as described in the Google Objective-C Style Guide. The function `main` is a notable exception. Function declarations in expansions in system headers are ignored.
Example conforming function definitions:
```
static bool IsPositive(int i) { return i > 0; }
static bool ABIsPositive(int i) { return i > 0; }
bool ABIsNegative(int i) { return i < 0; }
```
A fixit hint is generated for functions of static storage class but otherwise the check does not generate a fixit hint because an appropriate prefix for the function cannot be determined.
§2 Test Notes
* Verified clang-tidy tests pass successfully.
* Used check_clang_tidy.py to verify expected output of processing google-objc-function-naming.m
Reviewers: benhamilton, hokein, Wizard, aaron.ballman
Reviewed By: benhamilton
Subscribers: Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D51575
llvm-svn: 347132
Summary:
[[ https://bugs.llvm.org/show_bug.cgi?id=39224 | PR39224 ]]
As discussed, we can't always do the transform automatically due to that array-to-pointer decay of C array.
In order to detect whether we can do said transform, we'd need to be able to see all usages of said array,
which is, i would say, rather impossible if e.g. it is in the header.
Thus right now no fixit exists.
Exceptions: `extern "C"` code.
References:
* [[ https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es27-use-stdarray-or-stack_array-for-arrays-on-the-stack | CPPCG ES.27: Use std::array or stack_array for arrays on the stack ]]
* [[ https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon1-prefer-using-stl-array-or-vector-instead-of-a-c-array | CPPCG SL.con.1: Prefer using STL array or vector instead of a C array ]]
* HICPP `4.1.1 Ensure that a function argument does not undergo an array-to-pointer conversion`
* MISRA `5-2-12 An identifier with array type passed as a function argument shall not decay to a pointer`
Reviewers: aaron.ballman, JonasToth, alexfh, hokein, xazax.hun
Reviewed By: JonasToth
Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53771
llvm-svn: 346835
The new checker searches for those for loops which has a loop variable with a "too small" type which means this type can't represent all values which are part of the iteration range.
For example:
```
int main() {
long size = 300000;
for( short int i = 0; i < size; ++i) {}
}
```
The short type leads to infinite loop here because it can't store all values in the `[0..size]` interval. In a real use case, size means a container's size which depends on the user input. Which means for small amount of objects the algorithm works, but with a larger user input the software will freeze.
The idea of the checker comes from the LibreOffice project, where the same check was implemented as a clang compiler plugin, called `LoopVarTooSmall` (LLVM licensed).
The idea is the same behind this check, but the code is different because of the different framework.
Patch by ztamas.
Reviewers: alexfh, hokein, aaron.ballman, JonasToth, xazax.hun, whisperity
Reviewed By: JonasToth, whisperity
Differential Revision: https://reviews.llvm.org/D53974
llvm-svn: 346665
This check flags function top-level const-qualified return types and suggests removing the mostly-superfluous const qualifier where possible.
Patch by Yitzhak Mandelbaum.
llvm-svn: 345764
Summary:
This patch introduces a new clang-tidy check that matches on all `declStmt` that declare more then one variable
and transform them into one statement per declaration if possible.
It currently only focusses on variable declarations but should be extended to cover more kinds of declarations in the future.
It is related to https://reviews.llvm.org/D27621 and does use it's extensive test-suite. Thank you to firolino for his work!
Reviewers: rsmith, aaron.ballman, alexfh, hokein, kbobyrev
Reviewed By: aaron.ballman
Subscribers: ZaMaZaN4iK, mgehre, nemanjai, kbarton, lebedev.ri, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D51949
llvm-svn: 345735
Summary:
Detects when the integral literal or floating point (decimal or hexadecimal)
literal has non-uppercase suffix, and suggests to make the suffix uppercase,
with fix-it.
All valid combinations of suffixes are supported.
```
auto x = 1; // OK, no suffix.
auto x = 1u; // warning: integer literal suffix 'u' is not upper-case
auto x = 1U; // OK, suffix is uppercase.
...
```
This is a re-commit, the original was reverted by me in
rL345305 due to discovered bugs. (implicit code, template instantiation)
Tests were added, and the bugs were fixed.
I'm unable to find any further bugs, hopefully there aren't any..
References:
* [[ https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152241 | CERT DCL16-C ]]
* MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a literal suffix
* MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case
Reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52670
llvm-svn: 345381
There are some lurking issues with the handling of the SourceManager.
Somehow sometimes we end up extracting completely wrong
portions of the source buffer.
Reverts r344772, r44760, r344758, r344755.
llvm-svn: 345305
Summary:
This check finds cases where calls to an absl::Duration factory could use the more efficient integer overload.
For example:
// Original - Providing a floating-point literal.
absl::Duration d = absl::Seconds(10.0);
// Suggested - Use an integer instead.
absl::Duration d = absl::Seconds(10);
Patch by hwright.
Reviewers: alexfh, hokein, aaron.ballman, JonasToth
Reviewed By: hokein, JonasToth
Subscribers: zturner, xazax.hun, Eugene.Zelenko, mgorny, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53339
llvm-svn: 345167
And also enable it by default to be consistent with e.g. modernize-use-using.
This helps e.g. when running this check on client code where the macro is
provided by the system, so there is no easy way to modify it.
Reviewed By: JonasToth
Differential Revision: https://reviews.llvm.org/D53454
llvm-svn: 344871
Summary:
Detects when the integral literal or floating point (decimal or hexadecimal)
literal has non-uppercase suffix, and suggests to make the suffix uppercase,
with fix-it.
All valid combinations of suffixes are supported.
```
auto x = 1; // OK, no suffix.
auto x = 1u; // warning: integer literal suffix 'u' is not upper-case
auto x = 1U; // OK, suffix is uppercase.
...
```
References:
* [[ https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152241 | CERT DCL16-C ]]
* MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a literal suffix
* MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case
Reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52670
llvm-svn: 344755
New checker called bugprone-not-null-terminated-result. This check finds function calls where it is possible to cause a not null-terminated result. Usually the proper length of a string is strlen(src) + 1 or equal length of this expression, because the null terminator needs an extra space. Without the null terminator it can result in undefined behaviour when the string is read.
The following function calls are checked:
memcpy, wmemcpy, memcpy_s, wmemcpy_s, memchr, wmemchr, memmove, wmemmove, memmove_s, wmemmove_s, memset, wmemset, strerror_s, strncmp, wcsncmp, strxfrm, wcsxfrm
The following is a real-world example where the programmer forgot to increase the passed third argument, which is size_t length. That is why the length of the allocated memory is problematic too.
static char *StringCpy(const std::string &str) {
char *result = reinterpret_cast<char *>(malloc(str.size()));
memcpy(result, str.data(), str.size());
return result;
}
After running the tool fix-it rewrites all the necessary code according to the given options. If it is necessary, the buffer size will be increased to hold the null terminator.
static char *StringCpy(const std::string &str) {
char *result = reinterpret_cast<char *>(malloc(str.size() + 1));
strcpy(result, str.data());
return result;
}
Patch by Charusso.
Differential ID: https://reviews.llvm.org/D45050
llvm-svn: 344374
This check warns the uses of the deprecated member types of std::ios_base
and replaces those that have a non-deprecated equivalent.
Patch by andobence!
Reviewd by: alexfh
Revision ID: https://reviews.llvm.org/D51332
llvm-svn: 343848
Summary:
Finds instances of namespaces concatenated using explicit syntax, such as `namespace a { namespace b { [...] }}` and offers fix to glue it to `namespace a::b { [...] }`.
Properly handles `inline` and unnamed namespaces. ~~Also, detects empty blocks in nested namespaces and offers to remove them.~~
Test with common use cases included.
I ran the check against entire llvm repository. Except for expected `nested namespace definitions only available with -std=c++17 or -std=gnu++17` warnings I noticed no issues when the check was performed.
Example:
```
namespace a { namespace b {
void test();
}}
```
can become
```
namespace a::b {
void test();
}
```
Patch by wgml!
Reviewers: alexfh, aaron.ballman, hokein
Reviewed By: aaron.ballman
Subscribers: JonasToth, Eugene.Zelenko, lebedev.ri, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52136
llvm-svn: 343000
Finds instances where the user depends on internal details and warns them against doing so.
Should not be run on internal Abseil files or Abseil source code.
Patch by hugoeg!
Differential Revision: https://reviews.llvm.org/D50542
llvm-svn: 340928
This flags redundant calls to absl::StrCat where the result is being passed to another call to absl::StrCat or absl::StrAppend. Patch by Hugo Gonzalez and Samuel Benzaquen.
llvm-svn: 340918
This check ensures that users of Abseil do not open namespace absl in their code, as that violates our compatibility guidelines.
AbseilMatcher.h written by Hugo Gonzalez.
Patch by Deanna Garcia!
llvm-svn: 340800
This check is an abseil specific check that checks for code using single character string literals as delimiters and transforms the code into characters.
The check was developed internally and has been running at google, this is just
a move to open source the check. It was originally written by @sbenza.
Patch by Deanna Garcia!
llvm-svn: 340411
This check is an abseil specific test that tests to ensure users utilize abseil specific floating point division when trying to divide with abseil duration types.
Patch by Deanna Garcia!
llvm-svn: 340038
Finds functions which may throw an exception directly or indirectly, but they
should not: Destructors, move constructors, move assignment operators, the
main() function, swap() functions, functions marked with throw() or noexcept
and functions given as option to the checker.
Differential Revision: https://reviews.llvm.org/D33537
llvm-svn: 336997
These checks flag use of random number generators with poor seeds that would possibly lead to degraded random number generation.
Patch by Borsik Gábor
llvm-svn: 336301
I don't remember why I added it, but it's definitely not needed, since the check
doesn't have any options and the check doesn't have any special relation to the
Google C++ style.
llvm-svn: 335252
Summary:
Continuation of D46504.
Example output:
```
$ clang-tidy -enable-check-profile -store-check-profile=. -checks=-*,readability-function-size source.cpp
$ # Note that there won't be timings table printed to the console.
$ cat *.json
{
"file": "/path/to/source.cpp",
"timestamp": "2018-05-16 16:13:18.717446360",
"profile": {
"time.clang-tidy.readability-function-size.wall": 1.0421266555786133e+00,
"time.clang-tidy.readability-function-size.user": 9.2088400000005421e-01,
"time.clang-tidy.readability-function-size.sys": 1.2418899999999974e-01
}
}
```
There are two arguments that control profile storage:
* `-store-check-profile=<prefix>`
By default reports are printed in tabulated format to stderr. When this option
is passed, these per-TU profiles are instead stored as JSON.
If the prefix is not an absolute path, it is considered to be relative to the
directory from where you have run :program:`clang-tidy`. All `.` and `..`
patterns in the path are collapsed, and symlinks are resolved.
Example:
Let's suppose you have a source file named `example.cpp`, located in
`/source` directory.
* If you specify `-store-check-profile=/tmp`, then the profile will be saved
to `/tmp/<timestamp>-example.cpp.json`
* If you run :program:`clang-tidy` from within `/foo` directory, and specify
`-store-check-profile=.`, then the profile will still be saved to
`/foo/<timestamp>-example.cpp.json`
Reviewers: alexfh, sbenza, george.karpenkov, NoQ, aaron.ballman
Reviewed By: alexfh, george.karpenkov, aaron.ballman
Subscribers: Quuxplusone, JonasToth, aaron.ballman, llvm-commits, rja, Eugene.Zelenko, xazax.hun, mgrang, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D46602
llvm-svn: 334101
Summary:
Checks for narrowing conversions, e.g.
int i = 0;
i += 0.1;
This has what some might consider false positives for:
i += ceil(d);
Reviewers: alexfh, hokein
Subscribers: srhines, nemanjai, mgorny, JDevlieghere, xazax.hun, kbarton
Differential Revision: https://reviews.llvm.org/D38455
llvm-svn: 333066
Currently, diagnoses code that calls container.data()[some_index] when the container exposes a suitable operator[]() method that can be used directly.
Patch by Shuai Wang.
llvm-svn: 332519
Adding a check to restrict system includes to a whitelist. Given a list
of includes that are explicitly allowed, the check issues a fixit to
remove any system include not on that list from the source file.
Differential Revision: https://reviews.llvm.org/D43778
llvm-svn: 332125
Adding a check to restrict system includes to a whitelist. Given a list
of includes that are explicitly allowed, the check issues a fixit to
remove any system include not on that list from the source file.
Differential Revision: https://reviews.llvm.org/D43778
llvm-svn: 331930
Remove the `AnalyzeTemporaryDtors` option, since the corresponding
`cfg-temporary-dtors` option of the Static Analyzer defaults to `true` since
r326461.
llvm-svn: 331456
Summary:
Pretty straight-forward, just count all the variable declarations in the function's body, and if more than the configured threshold - do complain.
Note that this continues perverse practice of disabling the new option by default.
I'm not certain where is the balance point between not being too noisy, and actually enforcing the good practice.
If we really want to not disable this by default, but also to not cause too many new warnings, we could default to 50 or so.
But that is a lot of variables too...
I was able to find one coding style referencing variable count:
- https://www.kernel.org/doc/html/v4.15/process/coding-style.html#functions
> Another measure of the function is the number of local variables. They shouldn’t exceed 5-10, or you’re doing something wrong.
Reviewers: hokein, xazax.hun, JonasToth, aaron.ballman, alexfh
Reviewed By: aaron.ballman
Subscribers: kimgr, Eugene.Zelenko, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D44602
llvm-svn: 329902
This check attempts to catch buggy uses of the `TEMP_FAILURE_RETRY`
macro, which is provided by both Bionic and glibc.
Differential Revision: https://reviews.llvm.org/D45059
llvm-svn: 329759
The threshold option is 'MinTypeNameLength' with default value '5'.
With MinTypeNameLength == 5 'int'/'bool' and 'const int'/'const bool'
will not be converted to 'auto', while 'unsigned' will be.
Differential Revision: https://reviews.llvm.org/D45405
llvm-svn: 329730
Adding alias to google-build-namespaces to the Fuchsia module (checks
for anonymous namespaces in headers).
Differential Revision: https://reviews.llvm.org/D45447
llvm-svn: 329720
class A {...int virtual foo() {...}...};
class B: public A {...int foo() override {...}...};
class C: public B {...int foo() override {... A::foo()...}};
^^^^^^^^ warning: qualified name A::foo refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]
Differential Revision: https://reviews.llvm.org/D44295
llvm-svn: 329448
This is triggering on a pattern that's both too broad (const
std::string& members can be used safely) and too narrow (std::string is
not the only class with this problem). It has a very low true positive
rate, just remove it until we find a better solution for dangling string
references.
llvm-svn: 329292
The original check did break the green buildbot in the sanitizer build.
It took a while to redroduce and understand the issue.
There occured a stackoverflow while parsing the AST. The testcase with
256 case labels was the problem because each case label added another
stackframe. It seemed that the issue occured only in 'RelWithDebInfo' builds
and not in normal sanitizer builds.
To simplify the matchers the recognition for the different kinds of switch
statements has been moved into a seperate function and will not be done with
ASTMatchers. This is an attempt to reduce recursion and stacksize as well.
The new check removed this big testcase. Covering all possible values is still
implemented for bitfields and works there. The same logic on integer types
will lead to the issue.
Running it over LLVM gives the following results:
Differential: https://reviews.llvm.org/D40737
llvm-svn: 328107
Summary:
Detects function calls where the return value is unused.
Checked functions can be configured.
Reviewers: alexfh, aaron.ballman, ilya-biryukov, hokein
Reviewed By: alexfh, aaron.ballman
Subscribers: hintonda, JonasToth, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Patch by Kalle Huttunen!
Differential Revision: https://reviews.llvm.org/D41655
llvm-svn: 327833
Abel Kocsis