ClangTidy check for finding cases when std::move() is called with const or
trivially copyable arguments, that doesn't lead to any move or argument but it
makes copy. FixIt generates patch for removing call of std::move().
Patch by Vadym Doroshenko! (+ a couple of minor fixes)
Differential Revision: http://reviews.llvm.org/D12031
llvm-svn: 254070
Summary:
This check flags all array subscriptions on static arrays and
std::arrays that either have a non-compile-time-constant index or are
out of bounds.
Dynamic accesses into arrays are difficult for both tools and humans to
validate as safe. array_view is a bounds-checked, safe type for
accessing arrays of data. at() is another alternative that ensures
single accesses are bounds-checked. If iterators are needed to access an
array, use the iterators from an array_view constructed over the array.
This rule is part of the "Bounds safety" profile of the C++ Core
Guidelines, see
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-bounds2-only-index-into-arrays-using-constant-expressions
Reviewers: alexfh, sbenza, bkramer, aaron.ballman
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D13746
llvm-svn: 253401
Summary:
This check flags all use of c-style casts that perform a static_cast
downcast, const_cast, or reinterpret_cast.
Use of these casts can violate type safety and cause the program to
access a
variable that is actually of type X to be accessed as if it were of an
unrelated type Z. Note that a C-style (T)expression cast means to
perform
the first of the following that is possible: a const_cast, a
static_cast, a
static_cast followed by a const_cast, a reinterpret_cast, or a
reinterpret_cast followed by a const_cast. This rule bans (T)expression
only when used to perform an unsafe cast.
This rule is part of the "Type safety" profile of the C++ Core
Guidelines, see
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-type4-dont-use-c-style-texpression-casts-that-would-perform-a-static_cast-downcast-const_cast-or-reinterpret_cast.
Reviewers: alexfh, sbenza, bkramer, aaron.ballman
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D14096
llvm-svn: 252425
Summary: Consider a declaration an alias even if it doesn't have the same unqualified type than the container element, as long as one can be converted to the other using only implicit casts.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D14442
llvm-svn: 252315
Summary: Use the old index name in the cases where the check would come up with an invented name.
Reviewers: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D14438
llvm-svn: 252308
Summary: The old index declaration is going to be removed anyway, so we can reuse its name if it is the best candidate for the new index.
Reviewers: klimek
Subscribers: cfe-commits, alexfh
Differential Revision: http://reviews.llvm.org/D14437
llvm-svn: 252303
Summary:
If the container expression was obtained from the point where "size" (which usually is a const method) is invoked, then the topmost node in this expression may be an implicit cast to const.
When the container is a data member, the check was trying to obtain the member expression directly and was failing in the case mentioned above. This is solved by ignoring implicit casts.
Reviewers: klimek
Subscribers: cfe-commits, alexfh
Differential Revision: http://reviews.llvm.org/D14378
llvm-svn: 252278
Summary: "std::unique_ptr<int>" is not the same type as "std::unique_ptr<int, std::default_delete<int>>", unless we insert a "hasCanonicalType" in the middle. Probably it also happens in other cases related to default template argument.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D14291
llvm-svn: 252041
Summary:
I recently found that the variable naming wasn't working as expected with containers that are data members. The new index always received the name "Elem" (or equivalent) regardless of the container's name.
The check was assuming that the container's declaration was a VarDecl, which cannot be converted to a FieldDecl (a data member), and then it could never retrieve its name.
This also fixes some cases where the check failed to find the container at all (so it didn't do any fix) because of the same reason.
Reviewers: klimek
Subscribers: cfe-commits, alexfh
Differential Revision: http://reviews.llvm.org/D14289
llvm-svn: 251943
Summary: The previous change was focused in detecting when a non-const object was used in a constant way. Looks like I forgot the most important and trivial case: when the object is already constant. Failing to detect this cases results in compile errors, due to trying to bind a constant object to a non-const reference in the range-for statement. This change should fix that.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D14282
llvm-svn: 251940
Summary:
Now, it detects that several kinds of usages are can't modify the elements. Examples:
-When an usage is a call to a const member function or operator of the element.
-If the element is used as an argument to a function or constructor that takes a const-reference or a value.
-LValue to RValue conversion, if the element is a fundamental type (which allows the use of most of the builtin operators).
Reviewers: klimek
Subscribers: cfe-commits, alexfh
Differential Revision: http://reviews.llvm.org/D14198
llvm-svn: 251808
Summary: The check was assuming that a definition of a function always has a body, but a declaration that explicitly defaults or deletes a function is a definition too.
Reviewers: alexfh
Subscribers: klimek, cfe-commits
Differential Revision: http://reviews.llvm.org/D14238
llvm-svn: 251807
Summary: When traversing the parent map, the check assumed that all the nodes would be either Stmt or Decl. After r251101, this is no longer true: there can be TypeLoc and NestedNameSpecifierLoc nodes.
Reviewers: alexfh
Subscribers: klimek, cfe-commits
Differential Revision: http://reviews.llvm.org/D14229
llvm-svn: 251803
Summary:
When applying this check to the unit tests, it would hit an assertion:
llvm/tools/clang/lib/Lex/Lexer.cpp:1056: clang::SourceLocation clang::Lexer::getSourceLocation(const char*, unsigned int) const: Assertion `PP && "This doesn't work on raw lexers"' failed.
Reviewers: klimek, LegalizeAdulthood, alexfh
Subscribers: cfe-commits, alexfh
Differential Revision: http://reviews.llvm.org/D14204
llvm-svn: 251792
Summary: the check will now warn when the user provided definitions of this functions is equivalent to the explicitly defaulted ones.
Reviewers: klimek
Subscribers: klimek, cfe-commits, alexfh
Differential Revision: http://reviews.llvm.org/D14145
llvm-svn: 251788
Summary: If the size of the type is above a certain bound, we'll take a const reference. This bound can be set as an option. For now, the default value is 16 bytes.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D14176
llvm-svn: 251694
This check for clang-tidy looks for function with zero arguments declared as (void) and removes the unnecessary void token.
int foo(void);
becomes
int foo();
The check performs no formatting of the surrounding context but uses the lexer to look for the token sequence "(", "void", ")" in the prototype text. If this sequence of tokens is found, a removal is issued for the void token only.
Patch by Richard Thomson!
(+fixed tests, moved the check to the modernize module)
Differential revision: http://reviews.llvm.org/D7639
llvm-svn: 251475
I totally forgot that char can be defined as unsigned on some platforms.
Now I made explicit mention of signed type where necessary in tests.
Also fixed '//RUN: ' header of cxx98 test to correct format.
llvm-svn: 251244
Summary:
This is another check that I ported to clang-tidy from colobot-lint tool.
As previously discussed on cfe-dev mailing list, this is one of those
checks that I think is general and useful enough for contribution to
clang-tidy.
This patch contains implementation of check taken from colobot-lint, but
it is extended a great deal, including FixIt hints for automated
refactoring, exhaustive testcases, and user documentation.
Reviewers: sbenza, aaron.ballman, alexfh
Subscribers: Eugene.Zelenko
Differential Revision: http://reviews.llvm.org/D13635
llvm-svn: 251235
Summary: using "auto" on a loop that iterates over ints is kind of an overkill. Use the real type name instead.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D13982
llvm-svn: 251015
Summary: Take into account the current LangOptions the check has to add back the template argument.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D13983
llvm-svn: 251013
With this, site specific lit configs can inject parameters into the
test scripts if they need site specific parameters.
Next up: enable check_clang_tidy to take a resource dir to enable
non-standard locations for builtin includes.
llvm-svn: 251010
Summary:
This check flags all calls to c-style vararg functions and all use
of va_list, va_start and va_arg.
Passing to varargs assumes the correct type will be read. This is
fragile because it cannot generally be enforced to be safe in the
language and so relies on programmer discipline to get it right.
This rule is part of the "Type safety" profile of the C++ Core
Guidelines, see
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-type8-avoid-reading-from-varargs-or-passing-vararg-arguments-prefer-variadic-template-parameters-instead
This commits also reverts
"[clang-tidy] add cert's VariadicFunctionDefCheck as cppcoreguidelines-pro-type-vararg-def"
because that check makes the SFINAE use of vararg functions impossible.
Reviewers: alexfh, sbenza, bkramer, aaron.ballman
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D13787
llvm-svn: 250939
Summary:
Add a check that replaces empty bodies of special member functions with '= default;'.
For now, it is only implemented for the default constructor and the destructor, which are the easier cases.
The copy-constructor and the copy-assignment operator cases will be implemented later.
I applied this check to the llvm code base and found 627 warnings (385 in llvm, 9 in compiler-rt, 220 in clang and 13 in clang-tools-extra).
Applying the fixes didn't break any build or test, it only caused a -Wpedantic warning in lib/Target/Mips/MipsOptionRecord.h:33 becaused it replaced
virtual ~MipsOptionRecord(){}; to virtual ~MipsOptionRecord()= default;;
Reviewers: klimek
Subscribers: george.burgess.iv, Eugene.Zelenko, alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D13871
llvm-svn: 250897
Summary:
This check flags all access to members of unions. Passing unions as a
whole is not flagged.
Reading from a union member assumes that member was the last one
written, and writing to a union member assumes another member with a
nontrivial destructor had its destructor called. This is fragile because
it cannot generally be enforced to be safe in the language and so relies
on programmer discipline to get it right.
This rule is part of the "Type safety" profile of the C++ Core
Guidelines, see
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-type7-avoid-accessing-members-of-raw-unions-prefer-variant-instead
Reviewers: alexfh, sbenza, bkramer, aaron.ballman
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D13784
llvm-svn: 250537
Summary: Prevent clang-tidy from discarding fixes that are in different files but happen to have the same file offset.
Reviewers: klimek, bkramer
Subscribers: bkramer, alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D13810
llvm-svn: 250523
Summary: modernize-make-unique now correctly supports the different kinds of list initialization.
Reviewers: klimek
Subscribers: cfe-commits, alexfh
Differential Revision: http://reviews.llvm.org/D13590
llvm-svn: 250283
Summary:
This check flags all usage of pointer arithmetic, because it could lead
to an
invalid pointer.
Subtraction of two pointers is not flagged by this check.
Pointers should only refer to single objects, and pointer arithmetic is
fragile and easy to get wrong. array_view is a bounds-checked, safe type
for accessing arrays of data.
This rule is part of the "Bounds safety" profile of the C++ Core
Guidelines, see
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-bounds1-dont-use-pointer-arithmetic-use-array_view-instead
Depends on D13313
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D13311
llvm-svn: 250116
Summary:
This check flags all usages of static_cast, where a base class is casted
to a derived class.
In those cases, a fixit is provided to convert the cast to a
dynamic_cast.
Use of these casts can violate type safety and cause the program to
access a variable that is actually of type X to be accessed as if it
were of an unrelated type Z.
This rule is part of the "Type safety" profile of the C++ Core
Guidelines, see
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-type2-dont-use-static_cast-downcasts-use-dynamic_cast-instead
Depends on D13313
Reviewers: alexfh, sbenza, bkramer, aaron.ballman
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D13368
llvm-svn: 250098
Summary: Add the second template argument to the unique_ptr mock, and update the matcher so that it only matches against cases where the second argument is the default.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D13433
llvm-svn: 249305
Summary: Now that we prioritize copying trivial types over using const-references where possible, I found some cases where, after the transformation, the loop was using the address of the local copy instead of the original object.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D13431
llvm-svn: 249300
Summary:
https://llvm.org/bugs/show_bug.cgi?id=24960
modernize-use-nullptr would hit an assertion in some cases involving macros and initializer lists, due to finding a node with more than one parent (the two forms of the initializer list).
However, this doesn't mean that the replacement is incorrect, so instead of just rejecting this case I tried to find a way to make it work. Looking at the semantic form of the InitListExpr made sense to me (looking at both forms results in false negatives) but I am not sure of the things that we can miss by skipping the syntactic form.
Reviewers: klimek
Subscribers: cfe-commits, alexfh
Differential Revision: http://reviews.llvm.org/D13246
llvm-svn: 249291
This diff requires http://reviews.llvm.org/D13079 to be applied first. I wasn't sure about how to make patch series in Phabricator, and I wanted to keep the two separate for clarity.
It looks like that most cases can be supported with this patch. I'm not totally sure about the actual coverage though. I think that the matchers are very generic, but I'm still not totally fluent with the AST.
Patch by Beren Minor!
Differential revision: http://reviews.llvm.org/D13081
llvm-svn: 248996
Summary:
This fixes https://llvm.org/bugs/show_bug.cgi?id=22196 .
Also add a non-trivially copyable type to fix some tests that were meant to be about using const-refs, but were changed in r248438.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D13292
llvm-svn: 248994
Summary: create a check that replaces 'std::unique_ptr<type>(new type(args...))' with 'std::make_unique<type>(args...)'. It was on the list of "Ideas for new Tools". It needs to be tested more carefully, but first I wanted to know if you think it is worth the effort.
Reviewers: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D13166
llvm-svn: 248785
This is to level the ground a little bit, in preparation for the changes in http://reviews.llvm.org/D13081.
Code factorization replaces all insertions to NamingCheckFailures map with a unique addUsage function that does the job.
There is also no more difference between the declaration and the references to a given identifier, both cases are treated as ranges in the Usage vector. There is also a check to avoid duplicated ranges to be inserted, which sometimes triggered erroneous replacements.
References can now also be added before the declaration of the identifier is actually found; this looks to be the case for example when a templated class uses its parameters to specialize its templated base class.
Patch by Beren Minor!
Differential revision: http://reviews.llvm.org/D13079
llvm-svn: 248700
Summary: Add an option to specify wich style must be followed when choosing the new index name.
Reviewers: alexfh
Subscribers: cfe-commits, klimek
Differential Revision: http://reviews.llvm.org/D13052
llvm-svn: 248517
Previously, we would rewrite:
void f(const vector<int> &v) {
for (size_t i = 0; i < v.size(); ++i) {
to
for (const auto &elem : v) {
Now we rewrite it to:
for (auto elem : v) {
(and similarly for iterator based loops).
llvm-svn: 248438
Previously we would use a non-const loop variable in the range-based
loop for:
void f(const std::vector<int> &v) {
for (size_t i = 0; i < v.size(); ++i) {
Now we use const auto&.
Note that we'll also want to use a copy at least for simple types.
llvm-svn: 248418
Summary: Reorder the code in a more logical and understandable way.
Reviewers: klimek
Subscribers: cfe-commits, alexfh
Differential Revision: http://reviews.llvm.org/D12797
llvm-svn: 248144
Summary: Add the test about replacements in several arguments of the same macro call, now that the problem has been fixed.
Reviewers: alexfh
Subscribers: cfe-commits, klimek
Differential Revision: http://reviews.llvm.org/D12933
llvm-svn: 247889
Summary:
1. Avoid converting loops that iterate over the size of a container and don't use its elements, as this would result in an unused-result warning.
2. Never capture the elements by value on lambdas, thus avoiding doing unnecessary copies and errors with non-copyable types.
3. The 'const auto &' instead of 'auto &' substitution on const containers now works on arrays and pseudoarrays as well.
4. The error about multiple replacements in the same macro call is now documented in the tests (not solved though).
5. Due to [1], I had to add a dummy usage of the range element (like "(void) *It;" or similars) on the tests that had empty loops.
6. I removed the braces from the CHECK comments. I think that there is no need for them, and they confuse vim.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D12734
llvm-svn: 247399
For targeting LLP64, like Windows x86, size_t is not unsigned long.
tools/clang/tools/extra/test/clang-tidy/Output/misc-sizeof-container.cpp.tmp.cpp:33:12: error: target of using declaration conflicts with declaration already in scope [clang-diagnostic-error]
using std::size_t;
^
llvm-svn: 247394
containers.
Summary:
sizeof(some_std_string) is likely to be an error. This check finds this
pattern and suggests using .size() instead.
Reviewers: djasper, klimek, aaron.ballman
Subscribers: aaron.ballman, cfe-commits
Differential Revision: http://reviews.llvm.org/D12759
llvm-svn: 247297
This is first of series of patches, porting code from my project colobot-lint,
as I mentioned recently in cfe-dev mailing list.
This patch adds a new check in readability module:
readability-inconsistent-declaration-parameter-name. I also added appropriate
testcases and documentation.
I chose readability module, as it seems it is the best place for it.
I think I followed the rules of LLVM coding guideline, but I may have missed
something, as I usually use other code formatting style.
http://reviews.llvm.org/D12462
Patch by Piotr Dziwinski!
llvm-svn: 247261
Matthias Gehre