Preprocessor:addCommentHandler() does not take ownership,
so we'd end up leaking the TodoCommentHandler.
This patch makes it owned by the Check object.
Differential Revision: http://reviews.llvm.org/D5402
llvm-svn: 218068
It also suggests a fix-it, taking the name from $USER. This will be made
configurable eventually.
Differential Revision: http://reviews.llvm.org/D5393
llvm-svn: 218049
If we had many header files we would attach the fix-it for all files to all
warnings, oops. This is harmless 99.9% of the time but can confuse the rewriter
in some edge cases. Sadly I failed to create a small test case for this.
While there move fix-its instead of copying.
llvm-svn: 217951
As this is very dependent on the code base it has some ways of configuration.
It's possible to pick between 3 modes of operation:
- Line counting: number of lines including whitespace and comments
- Statement counting: number of statements within compoundStmts.
- Branch counter
In addition a threshold can be picked, warnings are only emitted when it is met.
The thresholds can be configured via a .clang-tidy file.
Differential Revision: http://reviews.llvm.org/D4986
llvm-svn: 217768
Summary:
Each check can implement readOptions and storeOptions methods to read
and store custom options. Each check's options are stored in a local namespace
to avoid name collisions and provide some sort of context to the user.
Reviewers: bkramer, klimek
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D5296
llvm-svn: 217661
Ever wanted to fix all the header guards in clang? Now it's easy.
Make sure clang-tidy is in $PATH and a compilation database is available.
$ ./run-clang-tidy.py -checks=-*,llvm-header-guard -fix
... get coffee (or more CPU cores) ...
$ svn diff
Some may argue that this is just a glorified xargs -P, but it does a bit more ;)
Differential Revision: http://reviews.llvm.org/D5188
llvm-svn: 217368
It was failing with:
C:\bb-win7\ninja-clang-i686-msc17-R\llvm-project\llvm\include\llvm/Support/ErrorOr.h(102) : error C3861: 'make_error_code': identifier not found
C:\bb-win7\ninja-clang-i686-msc17-R\llvm-project\clang-tools-extra\clang-tidy\ClangTidyOptions.cpp(153) : see reference to function template instantiation 'llvm::ErrorOr<T>::ErrorOr<std::errc::errc>(E,void *)' being compiled
with
[
T=clang::tidy::ClangTidyOptions,
E=std::errc::errc
]
llvm-svn: 217201
Summary:
This adds a support for the .clang-tidy file reading using
FileOptionsProvider, -dump-config option, and changes tests to not depend on
default checks set.
Reviewers: klimek, bkramer, djasper
Reviewed By: djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D5186
llvm-svn: 217155
parameter packs.
Summary:
This disables this check for std::bind and similar functions that use
parameter packs to forward arguments to a different function. Name of the
parameter pack argument doesn't matter.
Reviewers: klimek
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D5168
llvm-svn: 217039
we can also fix the original header guard.
We still allow an _ at the end of a header guard since it's so common, but
remove it now when the #endif comment is changed.
llvm-svn: 216462
The implementation is split into a generic part and a LLVM-specific part.
Other codebases can implement it with their own style. The specific features
supported are:
- Verification (and fixing) of header guards against a style based on the file path
- Automatic insertion of header guards for headers that are missing them
- A warning when the header guard doesn't enable our fancy header guard optimization
(e.g. when there's an include preceeding the guard)
- Automatic insertion of a comment with the guard name after #endif.
For the LLVM style we disable #endif comments for now, they're not very common
in the codebase. We also only flag headers in the include directories, there
doesn't seem to be a common style outside.
Differential Revision: http://reviews.llvm.org/D4867
llvm-svn: 215548
After post-commit review and community discussion, this seems like a
reasonable direction to continue, making ownership semantics explicit in
the source using the type system.
llvm-svn: 215324
There are still a couple of rough edges in here but it is working fine
on LLVM and generates the same results as sort_includes.py if there are
no blank lines involved.
Differential Revision: http://reviews.llvm.org/D4741
llvm-svn: 215152
Summary:
Rename ChecksFilter to GlobList, as there's nothing specific to checks in it.
It's a rather generic way to represent sets of strings (or patterns), so it may
be used for something else in ClangTidy. The new name would not look strange
when used to filter other entities.
Reviewers: klimek
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D4806
llvm-svn: 214961
Haven't thought that I ever needed to do this, but in this case comparing the
index using pointer arithmetic turns out to be really ugly. It also generates
nasty sign-compare warnings on 32 bit targets.
llvm-svn: 214705
Summary:
This allows us to copy the parameter name from the definition (as a comment)
or insert /*unused*/ in both places.
Reviewers: alexfh, klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D4772
llvm-svn: 214701
Summary:
class Foo {
Foo() {
Foo(42); // oops
}
Foo(int);
};
This is valid code but it does nothing and we can't emit a warning in clang
because there might be side effects. The checker emits a warning for this
pattern and also for base class initializers written in this style.
There is some overlap with the unused-rtti checker but they follow different
goals and fire in different places most of the time.
Reviewers: alexfh, djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D4667
llvm-svn: 214397
It doesn't make sense to suggest 'virtual' as clang-tidy would complain
about that on the next iteration (we are never issuing warnings for the
base function).
llvm-svn: 214063
ASTMatchers currently have problems mixing bound TypeLoc nodes with Decl/Stmt
nodes. That should be fixed soon but for this checker there we only need the
TypeLoc to generate a fixit so postpone the potentially heavyweight AST walking
until after we know that we're going to emit a warning.
This is covered by existing test cases.
Original message:
[clang-tidy] Add a check for RAII temporaries.
This tries to find code similar that immediately destroys
an object that looks like it's trying to follow RAII.
{
scoped_lock(&global_mutex);
critical_section();
}
This checker will have false positives if someone uses this pattern
to legitimately invoke a destructor immediately (or the statement is
at the end of a scope anyway). To reduce the number we ignore this
pattern in macros (this is heavily used by gtest) and ignore objects
with no user-defined destructor.
llvm-svn: 213738
Summary:
This tries to find code similar that immediately destroys
an object that looks like it's trying to follow RAII.
{
scoped_lock(&global_mutex);
critical_section();
}
This checker will have false positives if someone uses this pattern
to legitimately invoke a destructor immediately (or the statement is
at the end of a scope anyway). To reduce the number we ignore this
pattern in macros (this is heavily used by gtest) and ignore objects
with no user-defined destructor.
Reviewers: alexfh, djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D4615
llvm-svn: 213647
This required a rather ugly workaround for a problem in ASTMatchers where
callee() is only overloaded for Stmt and Decl but not for Expr.
llvm-svn: 213509
This reverts commit r213308.
Reverting to have some on-list discussion/confirmation about the ongoing
direction of smart pointer usage in the LLVM project.
llvm-svn: 213324
If there's memset(x, y, 0) in the code it's most likely a mistake. The
checker suggests a fix-it to swap 'y' and '0'.
I think this has the potential to be promoted into a general clang warning
after some testing in clang-tidy.
Differential Revision: http://reviews.llvm.org/D4535
llvm-svn: 213155
This change contains of two checkers that warn about
1. anonymous namespaces in header files.
2. 'using namespace' directives everywhere.
Differential Revision: http://reviews.llvm.org/D4523
llvm-svn: 213153
Summary:
Those are considered unsafe and should be replaced with simple pointers or
full copies. It recognizes both std::string and ::string.
Reviewers: alexfh, djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D4522
llvm-svn: 213133
Summary:
We still allow the escape hatch foo(int /*x*/) and also suggest this
in a fixit. This is more powerful than the corresponding cpplint.py check
it also flags functions with multiple arguments as naming all arguments is
recommended by the google style guide.
Reviewers: alexfh, djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D4518
llvm-svn: 213075
Those may be incompatible with C++11 and are unnecessary. We suggest
removing the template arguments when they match the types of the make_pair
arguments or replace it with std::pair and explicit template arguments when
not.
Differential Revision: http://reviews.llvm.org/D4497
llvm-svn: 213058
This looks for swapped arguments by looking at implicit conversions of arguments
void Foo(int, double);
Foo(1.0, 3); // Most likely a bug
llvm-svn: 212942
Summary:
This patch implements a subset of possible replacements of C-style
casts with const_cast/static_cast/reinterpret_cast. This should cover a large
portion of cases in real code. Handling of a few more cases may be implemented
eventually.
Reviewers: sbenza, djasper
Reviewed By: djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D4478
llvm-svn: 212924
The goal is to find code like the example below, which is likely a typo
where someone meant to write "if (*b)".
bool *b = SomeFunction();
if (b) {
// b never dereferenced
}
This checker naturally has a relatively high false positive rate so it
applies some heuristics to avoid cases where the pointer is checked for
nullptr before being written.
Differential Revision: http://reviews.llvm.org/D4458
llvm-svn: 212797
- Remove unused includes.
- Minor wording fix.
- Added support to check for clang-tidy messages to check_clang_tidy_fix.sh
= Updated test case.
llvm-svn: 212540
Those often cause use after free bugs and should be generally avoided.
Technically it is safe to have a Twine with >=2 components in a variable
but I don't think it is a good pattern to follow. The almost trivial checker
comes with elaborated fix-it hints that turn the Twine into a std::string
if necessary and otherwise fall back to the original type if the Twine
is created from a single value.
llvm-svn: 212535
Summary:
This patch removes " [check-name]" from the end of
ClangTidyMessage::Message. The " [check-name]" part is only appended when
printing diagnostics on the console. Clang errors are now marked with
"clang-diagnostic-error" check name, remarks and unknown warnings are marked
with "clang-diagnostic-unknown".
Reviewers: djasper
Reviewed By: djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D4356
llvm-svn: 212180
Summary:
The first version failed the SubstNonTypeTempateParmExpr-related test
on some buildbots. This one uses the new substNonTypeTempateParmExpr matcher to
filter out implicit C-style casts.
This patch depends on D4327.
Reviewers: djasper
Reviewed By: djasper
Subscribers: aemerson, cfe-commits
Differential Revision: http://reviews.llvm.org/D4328
llvm-svn: 212002
Summary:
This patch makes it possible for clang-tidy clients to provide
different options for different translation units. The option, which doesn't
make sense to be file-dependent, was moved to a separate ClangTidyGlobalOptions
struct. Added parsing of ClangTidyOptions.
Reviewers: klimek
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D3979
llvm-svn: 210260
Summary:
No filters should affect the display of errors. Fixed a few tests,
which had compile errors.
We need to think what we should do with mapped errors (-Werror).
Reviewers: klimek
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D3982
llvm-svn: 210044
Summary:
This seems like a more appropriate reaction to the user specifying a
single check with a wrong name, for example.
Reviewers: klimek
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D3981
llvm-svn: 210043
This breaks with MSVC.
With IsLateTemplateParsed, FunctionDecl::doesThisDeclarationHaveABody() returns true regardless of Body.
This reinstates what was fixed in r208985.
llvm-svn: 209896
Summary:
This is going to be used for a clang-tidy-diff script to display
warnings in changed lines only. The option uses JSON, as its value is not
intended to be entered manually.
Reviewers: klimek
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D3873
llvm-svn: 209450
Summary:
Handle various forms of existing namespace closing comments, fix
existing comments with wrong namespace name, ignore short namespaces.
The state of this check now seems to be enough to enable it by default to gather
user feedback ;)
Reviewers: klimek
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D3825
llvm-svn: 209141
Summary:
Make checks filtering more intuitive and easy to use. Remove
-disable-checks and change the format of -checks= to a comma-separated list of
globs with optional '-' prefix to denote exclusion. The -checks= option is now
cumulative, so it modifies defaults, not overrides them. Each glob adds or
removes to the current set of checks, so the filter can be refined or overriden
by adding globs.
Example:
The default value for -checks= is
'*,-clang-analyzer-alpha*,-llvm-include-order,-llvm-namespace-comment,-google-*',
which allows all checks except for the ones named clang-analyzer-alpha* and
others specified with the leading '-'. To allow all google-* checks one can
write:
clang-tidy -checks=google-* ...
If one needs only google-* checks, we first need to remove everything (-*):
clang-tidy -checks=-*,google-*
etc.
I'm not sure if we need to change something here, so I didn't touch the docs
yet.
Reviewers: klimek, alexfh
Reviewed By: alexfh
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D3770
llvm-svn: 208883
array. This simplifies usage of ClangTidyContext a bit and seems to be more
consistent.
Reviewers: klimek
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D3685
llvm-svn: 208407
Summary:
Also displays a hint to use -header-filter='.*' in case any warnings
are in non-user code. This will help discoverability of this option.
Reviewers: klimek
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D3621
llvm-svn: 208174
defined in a macro.
Summary:
We shouldn't suggest replacements in macros anyway, as we can't see all
usages of the macro and ensure the replacement is safe for all of them.
Reviewers: klimek
Reviewed By: klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D3611
llvm-svn: 207987
Summary:
Add clang-tidy -header-filter option to specify from which headers we
want diagnostics to be printed. By default we don't print diagnostics from
headers. We always print diagnostics from the main file of each translation
unit.
Reviewers: djasper, klimek
Reviewed By: klimek
Subscribers: klimek, cfe-commits
Differential Revision: http://reviews.llvm.org/D3590
llvm-svn: 207970
Summary:
The Google C++ Style Guide doesn't require copy constructors to be
declared explicit, but some people do this by mistake. Make this check detect
and fix such cases.
Reviewers: djasper
Reviewed By: djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D3541
llvm-svn: 207531
Summary:
These calls are part of the implementation of the smart pointer itself
and chaning it is likely to be wrong.
Example:
T& operator*() const { return *get(); }
Reviewers: djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D3540
llvm-svn: 207525
Summary:
Extend the check to detect patterns like 'ptr.get() == nullptr'
It detects == and != when any argument is a ptr.get() and the other is a
nullptr.
Only supports standard smart pointer types std::unique_ptr and std::shared_ptr.
Does not support the case 'ptr.get() == other.get()' yet.
Reviewers: djasper
CC: cfe-commits, alexfh
Differential Revision: http://llvm-reviews.chandlerc.com/D3294
llvm-svn: 205854
Also make tests slightly less dependent on default flags. Once we have
implemented configuration file support, we might want to store the
clang-tidy configuration for the tests there.
llvm-svn: 205408
The goal is to be able to run clang-tidy on LLVM files without further
configuration for now. Once llvm.org/PR19306 is addressed, we can add a
configuration file instead and choose other defaults.
llvm-svn: 205407
Summary:
This patch implements filtering of clang-tidy diagnostic messages by
the check name, so that "clang-tidy -checks=^llvm-" won't output any clang
warnings, for example. This is also helpful to run specific static-analyzer
checks: static analyzer always needs core checks to be enabled, but the user may
be interested only in the checks he asked for.
This patch also exposes warning option names for built-in diagnostics. We need
to have a namespace for these names to avoid collisions and to allow convenient
filtering, so I prefix them with "-W". I'm not sure it's the best thing to do,
and maybe "W" or "clang-diagnostic-" or something like this would be better.
Reviewers: klimek
Reviewed By: klimek
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D3121
llvm-svn: 204321
This checks that parameters named in comments that appear before arguments in
function and constructor calls match the parameter name used in the callee's
declaration. For example:
void f(int x, int y);
void g() {
f(/*y=*/0, /*z=*/0);
}
contains two violations of the policy, as the names 'x' and 'y' used in the
declaration do not match names 'y' and 'z' used at the call site.
I think there is significant value in being able to check/enforce this policy
as a way of guarding against accidental API misuse and silent breakages
caused by API changes.
Although this pattern appears somewhat frequently in the LLVM codebase,
this policy is not prescribed by the LLVM coding standards at the moment,
so it lives under 'misc'.
Differential Revision: http://llvm-reviews.chandlerc.com/D2914
llvm-svn: 204113
Summary:
This is immediately useful for generating macro expansion notes, and
may be useful for other things later on.
Reviewers: klimek, djasper
Reviewed By: klimek
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D2950
llvm-svn: 203457
Summary: Peter, I guess, this can help you in testing your check.
Reviewers: djasper, pcc, klimek
Reviewed By: klimek
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D2989
llvm-svn: 203451
This removes all references to OwningPtr, which should be fairly
undisruptive to out-of-tree projects since they are unlikely to use
clang-tools-extra as a library instead of a set of tools.
llvm-svn: 203382
The goal is to make it possible for checks to emit diagnostics at levels
other than 'warning'.
Differential Revision: http://llvm-reviews.chandlerc.com/D2913
llvm-svn: 202668
Summary:
Added a naive NOLINT implementation. It doesn't care about specific
linter categories, just the "// NOLINT" on the same line as a diagnostic.
Reviewers: klimek
Reviewed By: klimek
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D2896
llvm-svn: 202452
Summary:
This doesn't have any significant effect on the performance, but it
looks like a good thing to do.
Reviewers: djasper, klimek
Reviewed By: klimek
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D2763
llvm-svn: 201340
Summary:
I'm not absolutely sure this is 100% correct solution, but it seems to
do what I expect.
Reviewers: djasper, klimek
Reviewed By: djasper
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D2756
llvm-svn: 201308
Summary:
This patch introduces several improvements to clang-tidy diagnostic;
1. Make filtering of messages from non-user code more reliable. Output an
error when it or any of the related notes touches user code. This fixes an
assertion when an error has a location in a system header, and one of the
notes relates to user code.
2. In order for 1. to work, subscribe to the static analyzer diagnostics using
a custom PathDiagnosticConsumer.
3. Enable colors on supported terminals.
4. Output FixItHints.
Reviewers: djasper
Reviewed By: djasper
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D2714
llvm-svn: 200924
It was calling the utility wrapper that now requires a constant string
following clang r200132. The StringRef version on DiagnosticIDs appears to have
been what was intended so change to that.
llvm-svn: 200142
This now requires a compile-time constant string so let's build proper
diagnostic IDs and pass through the inputs as arguments.
Tracks clang changes in r200132.
llvm-svn: 200139
Summary:
Pass check names all the way from ClangTidyModule through
ClangTidyCheck and ClangTidyContext to ClangTidyError, and output it in
handleErrors. This allows to find mis-behaving check and disable it easily.
Reviewers: djasper, klimek
Reviewed By: djasper
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D2534
llvm-svn: 199094
Summary:
Moved implementation of classes declared in
ClangTidyDiagnosticConsumer.h to ClangTidyDiagnosticConsumer.cpp.
Added a FIXME note in ClangTidyDiagnosticConsumer::HandleDiagnostic.
Reviewers: klimek, djasper
Reviewed By: klimek
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D2520
llvm-svn: 198807
always produce as pretty of results as it does in LLVM and Clang, but
I don't mind and the value of having a single canonical ordering is very
high IMO.
Let me know if you spot really serious problems here.
llvm-svn: 198703
Summary:
Made ClangTidyAction more slim and moved its declaration to header to
allow easy creation of Clang-tidy ASTConsumer. Don't derive from
clang::ento::AnalysisAction, use clang::ento::CreateAnalysisConsumer instead
(I'll propose making this function a part of a public API in a separate patch).
Use MultiplexConsumer instead of a custom class.
Don't re-filter checkers list for each TU.
Reviewers: klimek
Reviewed By: klimek
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D2481
llvm-svn: 198402
Summary:
Allow disabling checks by regex. By default, disable alpha.* checks,
that are not particularly good tested (e.g. IdempotentOperationChecker, see
http://llvm-reviews.chandlerc.com/D2427).
Fixed a bug, that would disable all analyzer checks, when using a regex more
strict, than 'clang-analyzer-', for example --checks='clang-analyzer-deadcode-'.
Added --list-checks to list all enabled checks. This is useful to test specific
values in --checks/--disable-checks.
Reviewers: djasper, klimek
Reviewed By: klimek
CC: cfe-commits, klimek
Differential Revision: http://llvm-reviews.chandlerc.com/D2444
llvm-svn: 197717
This is implemented in a way that the current static analyzer
architecture allows, in the future we might want to revisit this.
With this change static analyzer checks are available from clang-tidy
by specifying -checks=clang-analyzer-<name>.
This change also fixes the use of the compilation database to allow
clang-tidy to be used like any other clang tool.
llvm-svn: 194707
This is the first version of a possible clang-tidy architecture. The
purpose of clang-tidy is to detect errors in adhering to common coding
patterns, e.g. described in the LLVM Coding Standards.
This is still heavily in flux.
Review: http://llvm-reviews.chandlerc.com/D884
llvm-svn: 187345
Alexander Kornienko