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
Use clang_target_link_libraries() in order to support linking against
libclang-cpp instead of static libraries.
Differential Revision: https://reviews.llvm.org/D68448
llvm-svn: 373786
Summary:
The last glob that matches the string decides whether that string is
included or excluded.
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D66787
llvm-svn: 370028
Summary:
It is a separate abstraction that is used in more contexts than just
a helper for ClangTidyDiagnosticConsumer.
Subscribers: mgorny, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D66747
llvm-svn: 369918
One call `makeRule` is unqualified (and unintentionally relying on ADL to
resolve correctly). This revision qualifies that call for consistency and to
drop use of ADL.
llvm-svn: 369915
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
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:
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:
Currently, `clang::tidy::test::runCheckOnCode()` constructs the check
instances *before* initializing the ClangTidyContext. This ordering causes
problems when the check's constructor accesses the context, for example, through
`getLangOpts()`.
This revision moves the construction to after the context initialization, which
follows the pattern used in the clang tidy tool itself.
Reviewers: gribozavr
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D63784
llvm-svn: 364435
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:
Change the namespace for llvm checkers from 'llvm' to
'llvm_check', and modify add_new_check.py and rename_check.py to
support the new namespace. Checker, file, and directory names remain
unchanged.
Used new version of rename_check.py to make the change in existing
llvm checkers, but had to fix LLVMTidyModule.cpp and
LLVMModuleTest.cpp by hand.
The changes made by rename_check.py are idempotent, so if accidentally
run multiple times, it won't do anything.
Reviewed By: aaron.ballman
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D60629
llvm-svn: 360450
Summary:
Motivation/Context: in the code review system integrating with clang-tidy,
clang-tidy doesn't provide a human-readable description of the fix. Usually
developers have to preview a code diff (before vs after apply the fix) to
understand what the fix does before applying a fix.
This patch proposes that each clang-tidy check provides a short and
actional fix description that can be shown in the UI, so that users can know
what the fix does without previewing diff.
This patch extends clang-tidy framework to support fix descriptions (will add implementations for
existing checks in the future). Fix descriptions and fixes are emitted via diagnostic::Note (rather than
attaching the main warning diagnostic).
Before this patch:
```
void MyCheck::check(...) {
...
diag(loc, "my check warning") << FixtItHint::CreateReplacement(...);
}
```
After:
```
void MyCheck::check(...) {
...
diag(loc, "my check warning"); // Emit a check warning
diag(loc, "fix description", DiagnosticIDs::Note) << FixtItHint::CreateReplacement(...); // Emit a diagnostic note and a fix
}
```
Reviewers: sammccall, alexfh
Reviewed By: alexfh
Subscribers: MyDeveloperDay, Eugene.Zelenko, aaron.ballman, JonasToth, xazax.hun, jdoerfert, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D59932
llvm-svn: 358576
Summary:
All in-tree clang-tidy checks have been migrated to the new
ClangTidyCheck::registerPPCallbacks method. Time to drop the old one.
Reviewers: sammccall, hokein
Reviewed By: hokein
Subscribers: xazax.hun, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60197
llvm-svn: 357582
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
Summary:
Clang's hierarchy is CompilerInstance -> DiagnosticsEngine -> DiagnosticConsumer.
(Ownership is optional/shared, but this structure is fairly clear).
Currently ClangTidyDiagnosticConsumer *owns* the DiagnosticsEngine:
- this inverts the hierarchy, which is confusing
- this means ClangTidyDiagnosticConsumer() mutates the passed-in context, which
is both surprising and limits flexibility
- it's not possible to use a different DiagnosticsEngine with ClangTidy
This means a little bit more code in the places ClangTidy is used standalone,
but more flexibility in using ClangTidy with other diagnostics configurations.
Reviewers: hokein
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D54033
llvm-svn: 346418
Summary:
Currently ClangTidyContext::diag() sends the diagnostics to a
DiagnosticsEngine, which probably delegates to a ClangTidyDiagnosticsConsumer,
which is supposed to go back and populate ClangTidyContext::Errors.
After this patch, the diagnostics are stored in the ClangTidyDiagnosticsConsumer
itself and can be retrieved from there.
Why?
- the round-trip from context -> engine -> consumer -> context is confusing
and makes it harder to establish layering between these things.
- context does too many things, and makes it hard to use clang-tidy as a library
- everyone who actually wants the diagnostics has access to the ClangTidyDiagnosticsConsumer
The most natural implementation (ClangTidyDiagnosticsConsumer::take()
finalizes diagnostics) causes a test failure: clang-tidy-run-with-database.cpp
asserts that clang-tidy exits successfully when trying to process a file
that doesn't exist.
In clang-tidy today, this happens because finish() is never called, so the
diagnostic is never flushed. This looks like a bug to me.
For now, this patch carefully preserves that behavior, but I'll ping the
authors to see whether it's deliberate and worth preserving.
Reviewers: hokein
Subscribers: xazax.hun, cfe-commits, alexfh
Differential Revision: https://reviews.llvm.org/D53953
llvm-svn: 345961
This patch moves the virtual file system form clang to llvm so it can be
used by more projects.
Concretely the patch:
- Moves VirtualFileSystem.{h|cpp} from clang/Basic to llvm/Support.
- Moves the corresponding unit test from clang to llvm.
- Moves the vfs namespace from clang::vfs to llvm::vfs.
- Formats the lines affected by this change, mostly this is the result of
the added llvm namespace.
RFC on the mailing list:
http://lists.llvm.org/pipermail/llvm-dev/2018-October/126657.html
Differential revision: https://reviews.llvm.org/D52783
llvm-svn: 344140
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
That broke every single .clang-tidy config out there
which happened to specify AnalyzeTemporaryDtors option:
YAML:5:24: error: unknown key 'AnalyzeTemporaryDtors'
AnalyzeTemporaryDtors: false
^~~~~
Error parsing <...>/.clang-tidy: Invalid argument
More so, that error isn't actually a error, the
clang-tidy does not exit with $? != 0, it continues
with the default config.
Surely this breakage isn't the intended behavior.
But if it is, feel free to revert this commit.
llvm-svn: 331822
Remove the `AnalyzeTemporaryDtors` option, since the corresponding
`cfg-temporary-dtors` option of the Static Analyzer defaults to `true` since
r326461.
llvm-svn: 331456
Summary:
I'm not sure whether there are any principal reasons why it returns raw owning pointer,
or it is just a old code that was not updated post-C++11.
I'm not too sure what testing i should do, because `check-all` is not error clean here for some reason,
but it does not //appear// asif those failures are related to these changes.
This is Clang-tools-extra part.
Clang part is D43779.
Reviewers: klimek, bkramer, alexfh, pcc
Reviewed By: alexfh
Subscribers: ioeric, jkorous-apple, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D43780
llvm-svn: 326202
We currently use target_link_libraries without an explicit scope
specifier (INTERFACE, PRIVATE or PUBLIC) when linking executables.
Dependencies added in this way apply to both the target and its
dependencies, i.e. they become part of the executable's link interface
and are transitive.
Transitive dependencies generally don't make sense for executables,
since you wouldn't normally be linking against an executable. This also
causes issues for generating install export files when using
LLVM_DISTRIBUTION_COMPONENTS. For example, clang has a lot of LLVM
library dependencies, which are currently added as interface
dependencies. If clang is in the distribution components but the LLVM
libraries it depends on aren't (which is a perfectly legitimate use case
if the LLVM libraries are being built static and there are therefore no
run-time dependencies on them), CMake will complain about the LLVM
libraries not being in export set when attempting to generate the
install export file for clang. This is reasonable behavior on CMake's
part, and the right thing is for LLVM's build system to explicitly use
PRIVATE dependencies for executables.
Unfortunately, CMake doesn't allow you to mix and match the keyword and
non-keyword target_link_libraries signatures for a single target; i.e.,
if a single call to target_link_libraries for a particular target uses
one of the INTERFACE, PRIVATE, or PUBLIC keywords, all other calls must
also be updated to use those keywords. This means we must do this change
in a single shot. I also fully expect to have missed some instances; I
tested by enabling all the projects in the monorepo (except dragonegg),
and configuring both with and without shared libraries, on both Darwin
and Linux, but I'm planning to rely on the buildbots for other
configurations (since it should be pretty easy to fix those).
Even after this change, we still have a lot of target_link_libraries
calls that don't specify a scope keyword, mostly for shared libraries.
I'm thinking about addressing those in a follow-up, but that's a
separate change IMO.
Differential Revision: https://reviews.llvm.org/D40823
llvm-svn: 319840
Summary:
This is part 3 of 3 of a series of changes to improve Objective-C
linting in clang-tidy.
This adds a new clang-tidy check `objc-forbidden-subclassing` which
ensures clients do not create subclasses of Objective-C classes which
are not designed to be subclassed.
(Note that for code under your control, you should use
__attribute__((objc_subclassing_restricted)) instead -- this
is intended for third-party APIs which cannot be modified.)
By default, the following classes (which are publicly documented
as not supporting subclassing) are forbidden from subclassing:
ABNewPersonViewController
ABPeoplePickerNavigationController
ABPersonViewController
ABUnknownPersonViewController
NSHashTable
NSMapTable
NSPointerArray
NSPointerFunctions
NSTimer
UIActionSheet
UIAlertView
UIImagePickerController
UITextInputMode
UIWebView
Clients can set a CheckOption
`objc-forbidden-subclassing.ClassNames` to a semicolon-separated
list of class names, which overrides this list.
Test Plan: `ninja check-clang-tools`
Patch by Ben Hamilton!
Reviewers: hokein, alexfh
Reviewed By: hokein
Subscribers: saidinwot, Wizard, srhines, mgorny, xazax.hun
Differential Revision: https://reviews.llvm.org/D39142
llvm-svn: 316744
Summary:
This is part 2 of 3 of a series of changes to improve
Objective-C linting in clang-tidy.
Currently, `clang::tidy::test::runCheckOnCode()` assumes all files
are C++ and unconditionally adds `-std=c++11` to the list of
`clang-tidy` options.
This updates the logic to check the extension of the source file
and only add `-std=c++11` if the extension indicates C++ or
Objective-C++.
Depends On D39188
Test Plan:
ninja ClangTidyTests && \
./tools/clang/tools/extra/unittests/clang-tidy/ClangTidyTests
Patch by Ben Hamilton!
Reviewers: hokein, alexfh
Reviewed By: hokein
Subscribers: Wizard
Differential Revision: https://reviews.llvm.org/D39189
llvm-svn: 316645
Summary:
This is part 1 of 3 of a series of changes to improve Objective-C
linting in clang-tidy.
This introduces a new clang-tidy module, `objc`, specifically for
Objective-C / Objective-C++ checks.
The module is currently empty; D39142 adds the first check.
Test Plan: `ninja check-clang-tools`
Patch by Ben Hamilton!
Reviewers: hokein, alexfh
Reviewed By: hokein
Subscribers: Wizard, mgorny
Differential Revision: https://reviews.llvm.org/D39188
llvm-svn: 316643
Yitzhak Mandelbaum