[clang-tidy] modernize-use-auto: don't remove stars by default

Summary:
By default, modernize-use-auto check will retain stars when replacing an explicit type with `auto`: `MyType *t = new MyType;` will be changed to `auto *t = new MyType;`, thus resulting in more consistency with the recommendations to use `auto *` for iterating over pointers in range-based for loops: http://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto

The new  `RemoveStars` option allows to revert to the old behavior: with the new option turned on the check will change `MyType *t = new MyType;` to `auto t = new MyType;`.

Reviewers: aaron.ballman, sbenza

Subscribers: Eugene.Zelenko, cfe-commits

Differential Revision: http://reviews.llvm.org/D20917

llvm-svn: 271739
This commit is contained in:
Alexander Kornienko 2016-06-03 21:22:58 +00:00
parent 94edaaaefb
commit c0308c451b
5 changed files with 175 additions and 37 deletions

View File

@ -243,6 +243,14 @@ StatementMatcher makeDeclWithNewMatcher() {
} // namespace
UseAutoCheck::UseAutoCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
RemoveStars(Options.get("RemoveStars", 0)) {}
void UseAutoCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "RemoveStars", RemoveStars ? 1 : 0);
}
void UseAutoCheck::registerMatchers(MatchFinder *Finder) {
// Only register the matchers for C++; the functionality currently does not
// provide any benefit to other languages, despite being benign.
@ -311,7 +319,7 @@ void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context) {
const QualType FirstDeclType = FirstDecl->getType().getCanonicalType();
std::vector<SourceLocation> StarLocations;
std::vector<FixItHint> StarRemovals;
for (const auto *Dec : D->decls()) {
const auto *V = cast<VarDecl>(Dec);
// Ensure that every DeclStmt child is a VarDecl.
@ -327,39 +335,44 @@ void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context) {
if (!Context->hasSameUnqualifiedType(V->getType(), NewExpr->getType()))
return;
// All subsequent variables in this declaration should have the same
// canonical type. For example, we don't want to use `auto` in
// `T *p = new T, **pp = new T*;`.
if (FirstDeclType != V->getType().getCanonicalType())
return;
if (RemoveStars) {
// Remove explicitly written '*' from declarations where there's more than
// one declaration in the declaration list.
if (Dec == *D->decl_begin())
continue;
// All subsequent declarations should match the same non-decorated type.
if (FirstDeclType != V->getType().getCanonicalType())
return;
auto Q = V->getTypeSourceInfo()->getTypeLoc().getAs<PointerTypeLoc>();
while (!Q.isNull()) {
StarLocations.push_back(Q.getStarLoc());
StarRemovals.push_back(FixItHint::CreateRemoval(Q.getStarLoc()));
Q = Q.getNextTypeLoc().getAs<PointerTypeLoc>();
}
}
}
// FIXME: There is, however, one case we can address: when the VarDecl pointee
// is the same as the initializer, just more CV-qualified. However, TypeLoc
// information is not reliable where CV qualifiers are concerned so we can't
// do anything about this case for now.
SourceRange Range(
FirstDecl->getTypeSourceInfo()->getTypeLoc().getSourceRange());
TypeLoc Loc = FirstDecl->getTypeSourceInfo()->getTypeLoc();
if (!RemoveStars) {
while (Loc.getTypeLocClass() == TypeLoc::Pointer ||
Loc.getTypeLocClass() == TypeLoc::Qualified)
Loc = Loc.getNextTypeLoc();
}
SourceRange Range(Loc.getSourceRange());
auto Diag = diag(Range.getBegin(), "use auto when initializing with new"
" to avoid duplicating the type name");
// Space after 'auto' to handle cases where the '*' in the pointer type is
// next to the identifier. This avoids changing 'int *p' into 'autop'.
Diag << FixItHint::CreateReplacement(Range, "auto ");
// Remove '*' from declarations using the saved star locations.
for (const auto &Loc : StarLocations) {
Diag << FixItHint::CreateReplacement(Loc, "");
}
Diag << FixItHint::CreateReplacement(Range, RemoveStars ? "auto " : "auto")
<< StarRemovals;
}
void UseAutoCheck::check(const MatchFinder::MatchResult &Result) {

View File

@ -18,15 +18,16 @@ namespace modernize {
class UseAutoCheck : public ClangTidyCheck {
public:
UseAutoCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
UseAutoCheck(StringRef Name, ClangTidyContext *Context);
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
void replaceIterators(const DeclStmt *D, ASTContext *Context);
void replaceNew(const DeclStmt *D, ASTContext *Context);
const bool RemoveStars;
};
} // namespace modernize

View File

@ -124,7 +124,7 @@ pointee type has to be written twice: in the declaration type and in the
// becomes
auto my_pointer = new TypeName(my_param);
auto *my_pointer = new TypeName(my_param);
The check will also replace the declaration type in multiple declarations, if
the following conditions are satisfied:
@ -141,7 +141,7 @@ the following conditions are satisfied:
// becomes
auto my_first_pointer = new TypeName, my_second_pointer = new TypeName;
auto *my_first_pointer = new TypeName, *my_second_pointer = new TypeName;
Known Limitations
-----------------
@ -150,3 +150,20 @@ Known Limitations
* User-defined iterators are not handled at this time.
RemoveStars option
------------------
If the option is set to non-zero (default is `0`), the check will remove stars
from the non-typedef pointer types when replacing type names with ``auto``.
Otherwise, the check will leave stars. For example:
.. code-block:: c++
TypeName *my_first_pointer = new TypeName, *my_second_pointer = new TypeName;
// RemoveStars = 0
auto *my_first_pointer = new TypeName, *my_second_pointer = new TypeName;
// RemoveStars = 1
auto my_first_pointer = new TypeName, my_second_pointer = new TypeName;

View File

@ -0,0 +1,106 @@
// RUN: %check_clang_tidy %s modernize-use-auto %t -- \
// RUN: -config="{CheckOptions: [{key: modernize-use-auto.RemoveStars, value: '1'}]}" \
// RUN: -- -std=c++11
class MyType {};
class MyDerivedType : public MyType {};
// FIXME: the replacement sometimes results in two consecutive spaces after
// the word 'auto' (due to the presence of spaces at both sides of '*').
void auto_new() {
MyType *a_new = new MyType();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
// CHECK-FIXES: auto a_new = new MyType();
static MyType *a_static = new MyType();
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use auto when initializing with new
// CHECK-FIXES: static auto a_static = new MyType();
MyType *derived = new MyDerivedType();
void *vd = new MyType();
// CV-qualifier tests.
//
// NOTE : the form "type const" is expected here because of a deficiency in
// TypeLoc where CV qualifiers are not considered part of the type location
// info. That is, all that is being replaced in each case is "MyType *" and
// not "MyType * const".
static MyType * const d_static = new MyType();
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use auto when initializing with new
// CHECK-FIXES: static auto const d_static = new MyType();
MyType * const a_const = new MyType();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
// CHECK-FIXES: auto const a_const = new MyType();
MyType * volatile vol = new MyType();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
// CHECK-FIXES: auto volatile vol = new MyType();
struct SType {} *stype = new SType;
int (**func)(int, int) = new (int(*[5])(int,int));
int *array = new int[5];
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
// CHECK-FIXES: auto array = new int[5];
MyType *ptr(new MyType);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
// CHECK-FIXES: auto ptr(new MyType);
MyType *ptr2{new MyType};
{
// Test for declaration lists.
MyType *a = new MyType(), *b = new MyType(), *c = new MyType();
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
// CHECK-FIXES: auto a = new MyType(), b = new MyType(), c = new MyType();
// Non-initialized declaration should not be transformed.
MyType *d = new MyType(), *e;
MyType **f = new MyType*(), **g = new MyType*();
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
// CHECK-FIXES: auto f = new MyType*(), g = new MyType*();
// Mismatching types in declaration lists should not be transformed.
MyType *h = new MyType(), **i = new MyType*();
// '*' shouldn't be removed in case of mismatching types with multiple
// declarations.
MyType *j = new MyType(), *k = new MyType(), **l = new MyType*();
}
{
// Test for typedefs.
typedef int * int_p;
// CHECK-FIXES: typedef int * int_p;
int_p a = new int;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
// CHECK-FIXES: auto a = new int;
int_p *b = new int*;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
// CHECK-FIXES: auto b = new int*;
// Test for typedefs in declarations lists.
int_p c = new int, d = new int;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
// CHECK-FIXES: auto c = new int, d = new int;
// Different types should not be transformed.
int_p e = new int, *f = new int_p;
int_p *g = new int*, *h = new int_p;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
// CHECK-FIXES: auto g = new int*, h = new int_p;
}
// Don't warn when 'auto' is already being used.
auto aut = new MyType();
auto *paut = new MyType();
const auto *pcaut = new MyType();
}

View File

@ -9,11 +9,11 @@ class MyDerivedType : public MyType {};
void auto_new() {
MyType *a_new = new MyType();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
// CHECK-FIXES: auto a_new = new MyType();
// CHECK-FIXES: auto *a_new = new MyType();
static MyType *a_static = new MyType();
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use auto when initializing with new
// CHECK-FIXES: static auto a_static = new MyType();
// CHECK-FIXES: static auto *a_static = new MyType();
MyType *derived = new MyDerivedType();
@ -27,15 +27,15 @@ void auto_new() {
// not "MyType * const".
static MyType * const d_static = new MyType();
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use auto when initializing with new
// CHECK-FIXES: static auto const d_static = new MyType();
// CHECK-FIXES: static auto * const d_static = new MyType();
MyType * const a_const = new MyType();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
// CHECK-FIXES: auto const a_const = new MyType();
// CHECK-FIXES: auto * const a_const = new MyType();
MyType * volatile vol = new MyType();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
// CHECK-FIXES: auto volatile vol = new MyType();
// CHECK-FIXES: auto * volatile vol = new MyType();
struct SType {} *stype = new SType;
@ -43,11 +43,11 @@ void auto_new() {
int *array = new int[5];
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
// CHECK-FIXES: auto array = new int[5];
// CHECK-FIXES: auto *array = new int[5];
MyType *ptr(new MyType);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
// CHECK-FIXES: auto ptr(new MyType);
// CHECK-FIXES: auto *ptr(new MyType);
MyType *ptr2{new MyType};
@ -55,14 +55,14 @@ void auto_new() {
// Test for declaration lists.
MyType *a = new MyType(), *b = new MyType(), *c = new MyType();
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
// CHECK-FIXES: auto a = new MyType(), b = new MyType(), c = new MyType();
// CHECK-FIXES: auto *a = new MyType(), *b = new MyType(), *c = new MyType();
// Non-initialized declaration should not be transformed.
MyType *d = new MyType(), *e;
MyType **f = new MyType*(), **g = new MyType*();
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
// CHECK-FIXES: auto f = new MyType*(), g = new MyType*();
// CHECK-FIXES: auto **f = new MyType*(), **g = new MyType*();
// Mismatching types in declaration lists should not be transformed.
MyType *h = new MyType(), **i = new MyType*();
@ -75,13 +75,14 @@ void auto_new() {
{
// Test for typedefs.
typedef int * int_p;
// CHECK-FIXES: typedef int * int_p;
int_p a = new int;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
// CHECK-FIXES: auto a = new int;
int_p *b = new int*;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
// CHECK-FIXES: auto b = new int*;
// CHECK-FIXES: auto *b = new int*;
// Test for typedefs in declarations lists.
int_p c = new int, d = new int;
@ -93,7 +94,7 @@ void auto_new() {
int_p *g = new int*, *h = new int_p;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
// CHECK-FIXES: auto g = new int*, h = new int_p;
// CHECK-FIXES: auto *g = new int*, *h = new int_p;
}
// Don't warn when 'auto' is already being used.