Summary:
This is 2/2 of moving ExprMutationAnalyzer from clangtidy to clang/Analysis.
ExprMutationAnalyzer is moved to clang/Analysis in D51948.
This diff migrates existing usages within clangtidy to point to the new
location and remove the old copy of ExprMutationAnalyzer.
Reviewers: george.karpenkov, JonasToth
Reviewed By: george.karpenkov
Subscribers: mgorny, a.sidorin, Szelethus, cfe-commits
Differential Revision: https://reviews.llvm.org/D51950
llvm-svn: 342006
Summary:
This handles cases like this:
```
typedef int& IntRef;
void mutate(IntRef);
void f() {
int x;
mutate(x);
}
```
where the param type is a sugared type (`TypedefType`) instead of a
reference type directly.
Note that another category of similar but different cases are already
handled properly before:
```
typedef int Int;
void mutate(Int&);
void f() {
int x;
mutate(x);
}
```
Reviewers: aaron.ballman, alexfh, george.karpenkov
Subscribers: xazax.hun, a.sidorin, Szelethus, cfe-commits
Differential Revision: https://reviews.llvm.org/D50953
llvm-svn: 341986
Summary:
For smart pointers like std::unique_ptr which uniquely owns the
underlying object, treat the mutation of the pointee as mutation of the
smart pointer itself.
This gives better behavior for cases like this:
```
void f(std::vector<std::unique_ptr<Foo>> v) { // undesirable analyze result of `v` as not mutated.
for (auto& p : v) {
p->mutate(); // only const member function `operator->` is invoked on `p`
}
}
```
Reviewers: hokein, george.karpenkov
Subscribers: xazax.hun, a.sidorin, Szelethus, cfe-commits
Differential Revision: https://reviews.llvm.org/D50883
llvm-svn: 341967
This is the same as D50619 plus fixes for buildbot failures on windows.
The test failures on windows are caused by -fdelayed-template-parsing
and is fixed by forcing -fno-delayed-template-parsing on test cases that
requires AST for uninstantiated templates.
llvm-svn: 341891
Summary:
I have hit this the rough way, while trying to use this in D51870.
There is no particular point in storing the pointers, and moreover
the pointers are assumed to be non-null, and that assumption is not
enforced. If they are null, it won't be able to do anything good
with them anyway.
Initially i thought about simply adding asserts() that they are
not null, but taking/storing references looks like even cleaner solution?
Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=38888 | PR38888 ]]
Reviewers: JonasToth, shuaiwang, alexfh, george.karpenkov
Reviewed By: shuaiwang
Subscribers: xazax.hun, a.sidorin, Szelethus, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D51884
llvm-svn: 341854
Summary:
- If a function is unresolved, assume it mutates its arguments
- Follow unresolved member expressions for nested mutations
Reviewers: aaron.ballman, JonasToth, george.karpenkov
Subscribers: xazax.hun, a.sidorin, cfe-commits
Differential Revision: https://reviews.llvm.org/D50619
llvm-svn: 341848
Summary:
(Originally started as a clang-tidy check but there's already D45444 so shifted to just adding ExprMutationAnalyzer)
`ExprMutationAnalyzer` is a generally useful helper that can be used in different clang-tidy checks for checking whether a given expression is (potentially) mutated within a statement (typically the enclosing compound statement.)
This is a more general and more powerful/accurate version of isOnlyUsedAsConst, which is used in ForRangeCopyCheck, UnnecessaryCopyInitialization.
It should also be possible to construct checks like D45444 (suggest adding const to variable declaration) or https://bugs.llvm.org/show_bug.cgi?id=21981 (suggest adding const to member function) using this helper function.
This function is tested by itself and is intended to stay generally useful instead of tied to any particular check.
Reviewers: hokein, alexfh, aaron.ballman, ilya-biryukov, george.karpenkov
Reviewed By: aaron.ballman
Subscribers: lebedev.ri, shuaiwang, rnkovacs, hokein, alexfh, aaron.ballman, a.sidorin, Eugene.Zelenko, xazax.hun, JonasToth, klimek, mgorny, cfe-commits
Tags: #clang-tools-extra
Patch by Shuai Wang.
Differential Revision: https://reviews.llvm.org/D45679
llvm-svn: 335736
ExprMutationAnalyzer is a generally useful helper that can be used in different clang-tidy checks for checking whether a given expression is (potentially) mutated within a statement (typically the enclosing compound statement.) This is a more general and more powerful/accurate version of isOnlyUsedAsConst, which is used in ForRangeCopyCheck, UnnecessaryCopyInitialization.
Patch by Shuai Wang
llvm-svn: 334604
This commit relands r331905.
r331904 added SrcMgr::CharacteristicKind to the InclusionDirective
callback, this revision updates instances of it in clang-tools-extra.
llvm-svn: 332023
[revision] added SrcMgr::CharacteristicKind to the InclusionDirective
callback, this revision updates instances of it in clang-tools-extra.
Differential Revision: https://reviews.llvm.org/D46615
llvm-svn: 331905
Summary:
The following Objective-C code currently incorrectly triggers
clang-tidy's performance-unnecessary-value-param check:
```
% cat /tmp/performance-unnecessary-value-param-arc.m
void foo(id object) { }
clang-tidy /tmp/performance-unnecessary-value-param-arc.m
-checks=-\*,performance-unnecessary-value-param -- -xobjective-c
-fobjc-abi-version=2 -fobjc-arc
1 warning generated.
/src/llvm/tools/clang/tools/extra/test/clang-tidy/performance-unnecessary-value-param-arc.m:10:13:
warning: the parameter 'object' is copied for each invocation but only
used as a const reference; consider making it a const reference
[performance-unnecessary-value-param]
void foo(id object) { }
~~ ^
const &
```
This is wrong for a few reasons:
1) Objective-C doesn't have references, so `const &` is not going to help
2) ARC heavily optimizes the "expensive" copy which triggers the warning
This fixes the issue by disabling the warning for non-C++, as well as
disabling it for objects under ARC memory management for
Objective-C++.
Fixes https://bugs.llvm.org/show_bug.cgi?id=32075
Test Plan: New tests added. Ran tests with `make -j12 check-clang-tools`.
Reviewers: alexfh, hokein
Reviewed By: hokein
Subscribers: stephanemoore, klimek, xazax.hun, cfe-commits, Wizard
Differential Revision: https://reviews.llvm.org/D42812
llvm-svn: 324097
This check implements the typebased semantic of `gsl::owner`.
Meaning, that
- only `gsl::owner` is allowed to get `delete`d
- `new` expression must be assigned to `gsl::owner`
- function calls that expect `gsl::owner` as argument, must get either an owner
or a newly created and recognized resource (in the moment only `new`ed memory)
- assignment to `gsl::owner` must be either a resource or another owner
- functions returning an `gsl::owner` are considered as factories, and their result
must be assigned to an `gsl::owner`
- classes that have an `gsl::owner`-member must declare a non-default destructor
There are some problems that occur when typededuction is in place.
For example `auto Var = function_that_returns_owner();` the type of `Var` will not be
an `gsl::owner`. This case is catched, and explicitly noted.
But cases like fully templated functions
```
template <typename T>
void f(T t) { delete t; }
// ...
f(gsl::owner<int*>(new int(42)));
```
Will created false positive (the deletion is problematic), since the type deduction
removes the wrapping `typeAlias`.
Codereview in D36354
llvm-svn: 313067
This check implements the typebased semantic of `gsl::owner`.
Meaning, that
- only `gsl::owner` is allowed to get `delete`d
- `new` expression must be assigned to `gsl::owner`
- function calls that expect `gsl::owner` as argument, must get either an owner
or a newly created and recognized resource (in the moment only `new`ed memory)
- assignment to `gsl::owner` must be either a resource or another owner
- functions returning an `gsl::owner` are considered as factories, and their result
must be assigned to an `gsl::owner`
- classes that have an `gsl::owner`-member must declare a non-default destructor
There are some problems that occur when typededuction is in place.
For example `auto Var = function_that_returns_owner();` the type of `Var` will not be
an `gsl::owner`. This case is catched, and explicitly noted.
But cases like fully templated functions
```
template <typename T>
void f(T t) { delete t; }
// ...
f(gsl::owner<int*>(new int(42)));
```
Will created false positive (the deletion is problematic), since the type deduction
removes the wrapping `typeAlias`.
Please give your comments :)
llvm-svn: 313043
Summary: socket() is better to include SOCK_CLOEXEC in its type argument to avoid the file descriptor leakage.
Reviewers: chh, Eugene.Zelenko, alexfh, hokein, aaron.ballman
Reviewed By: chh, alexfh
Subscribers: srhines, mgorny, JDevlieghere, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D34913
llvm-svn: 307818
The check doesn't really know if the code it is warning about came before
or after the header guard, so phrase it more neutral instead of complaining
about code before the header guard. The location for the warning is still
not optimal, but I don't think fixing that is worth the effort, the
preprocessor doesn't give us a better location.
Differential Revision: https://reviews.llvm.org/D30191
llvm-svn: 295715
Summary:
Prevents misclassifying includes based on the command-line filename (e.g. if a project is in a subdirectory).
This is slightly more robust than the additional duplicate detection, however the current classification scheme is still kind of brittle for a lot of code.
Reviewers: hokein, alexfh
Subscribers: cfe-commits, #clang-tools-extra
Patch by Julian Bangert!
Differential Revision: https://reviews.llvm.org/D26015
llvm-svn: 291767
Summary: IsExpensiveToCopy can return false positives for incomplete types, so ignore them.
All existing ClangTidy tests that depend on this function still pass as the types are complete.
Reviewers: alexfh, aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D26195
llvm-svn: 286008
Summary: This adds helper classes to add using declaractions and namespace aliases to function bodies. These help making function calls to deeply nested functions concise (e.g. when calling helpers in a refactoring)
Patch by Julian Bangert!
Reviewers: alexfh, hokein
Subscribers: beanz, mgorny, cfe-commits
Differential Revision: https://reviews.llvm.org/D24997
llvm-svn: 284368
Summary: This adds helper classes to add using declaractions and namespace aliases to function bodies. These help making function calls to deeply nested functions concise (e.g. when calling helpers in a refactoring)
Patch by Julian Bangert!
Reviewers: alexfh, hokein
Subscribers: cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D24997
llvm-svn: 283981
Classes with virtual methods or virtual bases are not trivially default constructible, so their members and bases need to be initialized.
Patch by Malcolm Parsons.
llvm-svn: 283224
Summary:
This fixes https://llvm.org/bugs/show_bug.cgi?id=30487 where
```
warning: uninitialized record type: 's' [cppcoreguidelines-pro-type-member-init]
```
is emitted on
```
struct MyStruct
{
int a = 5;
int b = 7;
};
int main()
{
MyStruct s;
}
```
Reviewers: alexfh, aaron.ballman
Subscribers: nemanjai, cfe-commits
Differential Revision: https://reviews.llvm.org/D24848
llvm-svn: 282625
Changed the extension check to include the option of ",h,hh,hpp,hxx" instead of just returning whether the file ended with ".h".
Differential revision: https://reviews.llvm.org/D20512
llvm-svn: 279803
Summary:
Make check more useful in the following two cases:
The parameter is passed by non-const value, has a non-deleted move constructor and is only referenced once in the function as argument to the type's copy constructor.
The parameter is passed by non-const value, has a non-deleted move assignment operator and is only referenced once in the function as argument of the the type's copy assignment operator.
In this case suggest a fix to move the parameter which avoids the unnecessary copy and is closest to what the user might have intended.
Reviewers: alexfh, sbenza
Subscribers: cfe-commits, Prazek
Differential Revision: http://reviews.llvm.org/D20277
llvm-svn: 274380
Instead of forming char ranges that patch made us form token ranges,
which behave subtly different. Sadly I'm only seeing this as part of a
larger test case that I haven't fully reduced yet.
llvm-svn: 269896
Summary:
This patch is lifting matchers used by more than one checkers
to the common namespace.
Reviewers: aaron.ballman, alexfh
Subscribers: aaron.ballman, cfe-commits
Differential Revision: http://reviews.llvm.org/D19841
llvm-svn: 269804
Summary:
Cleanup some code by using appropriate APIs.
Some coding style cleanups.
There is no behavior changes.
- Function `IncludeSorter::CreateFixIt` can be replaced by `FixItHint::CreateReplacement`.
- Function `cleanPath` is a wrapper for `llvm::sys::path::remove_dots`.
Reviewers: alexfh
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D20279
llvm-svn: 269656
Summary:
Lift some common code used by multiple checkers.
This function is also used by checkers that are coming.
It is quite common for a checker to parse a list of names.
Reviewers: alexfh
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D19846
llvm-svn: 269065
Shuai Wang