This changes the behavious of `RenamerClangTidyCheck` based checks by grouping declarations of the same thing into 1 warning where it is first declared.
This cleans up clang-tidy output and prevents issues where 1 fix-it couldn't be applied, yet all other warnings(and fix-its) for the same declaration would be applied.
The old behaviour of forward declaring a class without defining it isn't affected, i.e. no warnings will be emitted for that case.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D82059
Refactor out the double lookup in `IncludeInserter` when trying to get the `IncludeSorter` for a specified `FileID`.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D82004
Fix various tool libraries not to link to clang's .a libraries and dylib
simultaneously. This may cause breakage, in particular through
duplicate command-line option declarations.
Differential Revision: https://reviews.llvm.org/D81967
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:
The new option allows the user to specify which file naming convention is used
by the source code ('llvm' or 'google').
Reviewers: gribozavr2
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D79380
Summary:
This revision simplifies the representation of edits in rewrite rules. The
simplified form is more general, allowing the user more flexibility in building
custom edit specifications.
The changes extend the API, without changing the signature of existing
functions. So this only risks breaking users that directly accessed the
`RewriteRule` struct.
Reviewers: gribozavr2
Subscribers: jfb, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77419
This reverts commit 97aa593a83 as it
causes problems (PR45453) https://reviews.llvm.org/D77574#1966321.
This additionally adds an explicit reference to FrontendOpenMP to
clang-tidy where ASTMatchers is used.
This is hopefully just a temporary solution. The dependence on
`FrontendOpenMP` from `ASTMatchers` should be handled by CMake
implicitly, not us explicitly.
Reviewed By: aheejin
Differential Revision: https://reviews.llvm.org/D77666
Summary: Change all checks that take enums as configuration to use enum specific methods in `ClangTidyCheck::OptionsView`.
Reviewers: aaron.ballman, alexfh
Reviewed By: aaron.ballman
Subscribers: wuzish, nemanjai, kbarton, arphaman, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D76606
This is how it should've been and brings it more in line with
std::string_view. There should be no functional change here.
This is mostly mechanical from a custom clang-tidy check, with a lot of
manual fixups. It uncovers a lot of minor inefficiencies.
This doesn't actually modify StringRef yet, I'll do that in a follow-up.
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
The only part of ASTContext.h that requires most AST types to be
complete is the parent map. Nothing in Clang proper uses the ParentMap,
so split it out into its own class. Make ASTContext own the
ParentMapContext so there is still a one-to-one relationship.
After this change, 562 fewer files depend on ASTTypeTraits.h, and 66
fewer depend on TypeLoc.h:
$ diff -u deps-before.txt deps-after.txt | \
grep '^[-+] ' | sort | uniq -c | sort -nr | less
562 - ../clang/include/clang/AST/ASTTypeTraits.h
340 + ../clang/include/clang/AST/ParentMapContext.h
66 - ../clang/include/clang/AST/TypeLocNodes.def
66 - ../clang/include/clang/AST/TypeLoc.h
15 - ../clang/include/clang/AST/TemplateBase.h
...
I computed deps-before.txt and deps-after.txt with `ninja -t deps`.
This removes a common and key dependency on TemplateBase.h and
TypeLoc.h.
This also has the effect of breaking the ParentMap RecursiveASTVisitor
instantiation into its own file, which roughly halves the compilation
time of ASTContext.cpp (29.75s -> 17.66s). The new file takes 13.8s to
compile.
I left behind forwarding methods for getParents(), but clients will need
to include a new header to make them work:
#include "clang/AST/ParentMapContext.h"
I noticed that this parent map functionality is unfortunately duplicated
in ParentMap.h, which only works for Stmt nodes.
Reviewed By: rsmith
Differential Revision: https://reviews.llvm.org/D71313
Before this patch, readability-identifier-naming contained a significant amount
of logic for (a) checking the style of identifiers, followed by (b) renaming/
applying fix-its. This patch factors out (b) into a separate base class so that
it can be reused by other checks that want to do renaming. This also cleans up
readability-identifier-naming significantly, since now it only needs to be
concerned with the interesting details of (a).
Summary:
This patch extends the already existing facility to add 'const' to variables
to be more flexible and correct. The previous version did not consider pointers
as value AND pointee. For future automatic introduction for const-correctness
this shortcoming needs to be fixed.
It always allows configuration where the 'const' token is inserted, either on
the left side (if possible) or the right side.
It adds many unit-tests to the utility-function that did not exist before, as
the function was implicitly tested through clang-tidy checks. These
tests were not changed, as the API is still compatible.
Reviewers: aaron.ballman, hokein, alexfh, shuaiwang, lebedev.ri
Reviewed By: aaron.ballman
Subscribers: jdoerfert, mgorny, xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D54395
Replace tidy::utils::lexer::getConstQualifyingToken with a corrected and also
generalized to other qualifiers variant - getQualifyingToken.
Fixes PR44326
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:
This revision introduces a new interface `MatchComputation` which generalizes
the `Stencil` interface and replaces the `std::function` interface of
`MatchConsumer`. With this revision, `Stencil` (as an abstraction) becomes just
one collection of implementations of
`MatchComputation<std::string>`. Correspondingly, we remove the `Stencil` class
entirely in favor of a simple type alias, deprecate `MatchConsumer` and change
all functions that accepted `MatchConsumer<std::string>` to use
`MatchComputation<std::string>` instead.
Reviewers: gribozavr
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D69802
Summary:
Updates the relevant source files to use bindings in `clang::transformer` rather
than `clang::tooling`.
Reviewers: gribozavr
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D69804
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:
The Transformer library has been growing inside of
lib/Tooling/Refactoring. However, it's not really related to anything else in
that directory. This revision moves all Transformer-related files into their own
include & lib directories. A followup revision will (temporarily) add
forwarding headers to help any users migrate their code to the new location.
Reviewers: gribozavr
Subscribers: mgorny, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D68637
llvm-svn: 374271
Summary:
The bugprone-use-after-move check exhibits false positives for certain uses of
the C++17 if/switch init statements. These false positives are caused by a bug
in the ExprSequence calculations.
This revision adds tests for the false positives and fixes the corresponding
sequence calculation.
Reviewers: gribozavr
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D67292
llvm-svn: 371396
Summary:
The recordIsTriviallyDefaultConstructible may cause an infinite loop when
running on an ill-formed decl.
Reviewers: gribozavr
Subscribers: nemanjai, xazax.hun, kbarton, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D66874
llvm-svn: 370200
Summary:
This patch changes the location specified to the
`ClangTidyCheck::diag()`. Currently, the beginning of the matched range is
used. This patch uses the beginning of the first fix's range. This change both
simplifies the code and (hopefully) gives a more intuitive result: the reported
location aligns with the fix(es) provided, rather than the (arbitrary) range of
the rule's match.
N.B. this patch will break the line offset numbers in lit tests if the first fix
is not at the beginning of the match.
Reviewers: gribozavr
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D66676
llvm-svn: 369914
Summary:
We've been running doxygen with the autobrief option for a couple of
years now. This makes the \brief markers into our comments
redundant. Since they are a visual distraction and we don't want to
encourage more \brief markers in new code either, this patch removes
them all.
Patch produced by
for i in $(git grep -l '\\brief'); do perl -pi -e 's/\\brief //g' $i & done
[This is analogous to LLVM r331272 and CFE r331834]
Subscribers: srhines, nemanjai, javed.absar, kbarton, MaskRay, jkorous, arphaman, jfb, kadircet, jsji, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D66578
llvm-svn: 369643
Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
of (hopefully) all the llvm::make_unique instances across the monorepo.
Differential revision: https://reviews.llvm.org/D66259
llvm-svn: 368944
Summary:
`buildMatchers` is the new, more general way to extract the matcher from a rule.
This change migrates the code to use it instead of `buildMatcher`.
Reviewers: gribozavr
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D65879
llvm-svn: 368700
Summary:
This revision implements support for the `AddedIncludes` field in
RewriteRule cases; that is, it supports specifying the addition of include
directives in files modified by the clang tidy check.
Reviewers: ilya-biryukov, gribozavr
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D63893
llvm-svn: 364922
Summary: Tidy check behavior often depends on language and/or clang-tidy options. This revision allows a user of TranformerClangTidyCheck to pass rule _generator_ in place of a rule, where the generator takes both the language and clang-tidy options. Additionally, the generator returns an `Optional` to allow for the case where the check is deemed irrelevant/disable based on those options.
Reviewers: gribozavr
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D63288
llvm-svn: 364442
Summary:
A range-for was added in r361647 where the range variable was only used in an
assertion. As a result, it warned for Release builds. This revision
restructures the assertion to avoid the problem.
Patch by Yitzhak Mandelbaum.
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: xazax.hun, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D62412
llvm-svn: 361749
Summary:
In general, the `Explanation` field is optional in `RewriteRule` cases. But,
because the primary purpose of clang-tidy checks is to provide users with
diagnostics, we assume that a missing explanation is a bug. This change adds an
assertion that checks all cases for an explanation, and updates the code to rely
on that assertion correspondingly.
Reviewers: ilya-biryukov
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D62340
llvm-svn: 361647
This revision introduces an adaptor from Transformer's rewrite rules
(`clang::tooling::RewriteRule`) to `ClangTidyCheck`. For example, given a
RewriteRule `MyCheckAsRewriteRule`, it lets one define a tidy check as follows:
```
class MyTidyCheck : public TransformerClangTidyCheck {
public:
MyTidyCheck(StringRef Name, ClangTidyContext *Context)
: TransformerClangTidyCheck(MyCheckAsRewriteRule, Name, Context) {}
};
```
Reviewers: aaron.ballman
Subscribers: mgorny, xazax.hun, cfe-commits, ilya-biryukov
Tags: #clang
Differential Revision: https://reviews.llvm.org/D61386
llvm-svn: 361418
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:
D59466 wants to analyse the `Stmt`, and `ExceptionEscapeCheck` does not
have that as a possible entry point.
This simplifies addition of `Stmt` analysis entry point.
Reviewers: baloghadamsoftware, JonasToth, gribozavr
Reviewed By: gribozavr
Subscribers: rnkovacs, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D59650
llvm-svn: 356799
Summary:
The analsis on the throwing behvaiour on functions and statements gave only
a binary answer whether an exception could occur and if yes which types are
thrown.
This refactoring allows keeping track if there is a unknown factor, because the
code calls to some functions with unavailable source code with no `noexcept`
information.
This 'potential Unknown' information is propagated properly and can be queried
separately.
Reviewers: lebedev.ri, aaron.ballman, baloghadamsoftware, alexfh
Reviewed By: lebedev.ri, baloghadamsoftware
Subscribers: xazax.hun, rnkovacs, a.sidorin, Szelethus, donat.nagy, dkrupp, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D57883
llvm-svn: 354517
Summary:
The check `bugprone-exception-escape` does an AST-based analysis to determine
if a function might throw an exception and warns based on that information.
The analysis part is refactored into a standalone class similiar to
`ExprMutAnalyzer` that is generally useful.
I intent to use that class in a new check to automatically introduce `noexcept`
if possible.
Reviewers: aaron.ballman, alexfh, hokein, baloghadamsoftware, lebedev.ri
Reviewed By: baloghadamsoftware, lebedev.ri
Subscribers: lebedev.ri, mgorny, xazax.hun, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D57100
llvm-svn: 352741
to reflect the new license.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351636
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:
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 option added to these three checks to be able to silence false positives on
types that are intentionally passed by value or copied. Such types are e.g.
intrusive reference counting pointer types like llvm::IntrusiveRefCntPtr. The
new option is named WhiteListTypes and can contain a semicolon-separated list of
names of these types. Regular expressions are allowed. Default is empty.
Differential Revision: https://reviews.llvm.org/D52727
llvm-svn: 344340
Summary:
This patch is a small refactoring necessary for
'readability-isolate-declaration' and does not introduce functional changes.
It allows to use the utility functions without a full `ASTContext` and requires only the `SourceManager` and the `LangOpts`.
Reviewers: alexfh, aaron.ballman, hokein
Reviewed By: alexfh
Subscribers: nemanjai, xazax.hun, kbarton, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52684
llvm-svn: 343850
Summary:
Before this fix, the bugprone-use-after-move check could incorrectly
conclude that a use and move in a function template were not sequenced.
For details, see
https://bugs.llvm.org/show_bug.cgi?id=39149
Reviewers: alexfh, hokein, aaron.ballman, JonasToth
Reviewed By: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52782
llvm-svn: 343768
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
Nathan James