diff options
7 files changed, 325 insertions, 26 deletions
diff --git a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp index 8c43e6b754f..29d21f07505 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp @@ -23,6 +23,7 @@ namespace { const char IteratorDeclStmtId[] = "iterator_decl"; const char DeclWithNewId[] = "decl_new"; +const char DeclWithCastId[] = "decl_cast"; /// \brief Matches variable declarations that have explicit initializers that /// are not initializer lists. @@ -208,27 +209,18 @@ TypeMatcher iteratorFromUsingDeclaration() { /// \brief This matcher returns declaration statements that contain variable /// declarations with written non-list initializer for standard iterators. StatementMatcher makeIteratorDeclMatcher() { - return declStmt( - // At least one varDecl should be a child of the declStmt to ensure - // it's a declaration list and avoid matching other declarations, - // e.g. using directives. - has(varDecl()), - unless(has(varDecl(anyOf( - unless(hasWrittenNonListInitializer()), hasType(autoType()), - unless(hasType( - isSugarFor(anyOf(typedefIterator(), nestedIterator(), - iteratorFromUsingDeclaration()))))))))) + return declStmt(unless(has( + varDecl(anyOf(unless(hasWrittenNonListInitializer()), + unless(hasType(isSugarFor(anyOf( + typedefIterator(), nestedIterator(), + iteratorFromUsingDeclaration()))))))))) .bind(IteratorDeclStmtId); } StatementMatcher makeDeclWithNewMatcher() { return declStmt( - has(varDecl()), unless(has(varDecl(anyOf( unless(hasInitializer(ignoringParenImpCasts(cxxNewExpr()))), - // Skip declarations that are already using auto. - anyOf(hasType(autoType()), - hasType(pointerType(pointee(autoType())))), // FIXME: TypeLoc information is not reliable where CV // qualifiers are concerned so these types can't be // handled for now. @@ -243,6 +235,26 @@ StatementMatcher makeDeclWithNewMatcher() { .bind(DeclWithNewId); } +StatementMatcher makeDeclWithCastMatcher() { + return declStmt( + unless(has(varDecl(unless(hasInitializer(explicitCastExpr())))))) + .bind(DeclWithCastId); +} + +StatementMatcher makeCombinedMatcher() { + return declStmt( + // At least one varDecl should be a child of the declStmt to ensure + // it's a declaration list and avoid matching other declarations, + // e.g. using directives. + has(varDecl()), + // Skip declarations that are already using auto. + unless(has(varDecl(anyOf(hasType(autoType()), + hasType(pointerType(pointee(autoType()))), + hasType(referenceType(pointee(autoType()))))))), + anyOf(makeIteratorDeclMatcher(), makeDeclWithNewMatcher(), + makeDeclWithCastMatcher())); +} + } // namespace UseAutoCheck::UseAutoCheck(StringRef Name, ClangTidyContext *Context) @@ -257,8 +269,7 @@ 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. if (getLangOpts().CPlusPlus) { - Finder->addMatcher(makeIteratorDeclMatcher(), this); - Finder->addMatcher(makeDeclWithNewMatcher(), this); + Finder->addMatcher(makeCombinedMatcher(), this); } } @@ -313,7 +324,9 @@ void UseAutoCheck::replaceIterators(const DeclStmt *D, ASTContext *Context) { << FixItHint::CreateReplacement(Range, "auto"); } -void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context) { +void UseAutoCheck::replaceExpr(const DeclStmt *D, ASTContext *Context, + std::function<QualType(const Expr *)> GetType, + StringRef Message) { const auto *FirstDecl = dyn_cast<VarDecl>(*D->decl_begin()); // Ensure that there is at least one VarDecl within the DeclStmt. if (!FirstDecl) @@ -328,13 +341,13 @@ void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context) { if (!V) return; - const auto *NewExpr = cast<CXXNewExpr>(V->getInit()->IgnoreParenImpCasts()); - // Ensure that every VarDecl has a CXXNewExpr initializer. - if (!NewExpr) + const auto *Expr = V->getInit()->IgnoreParenImpCasts(); + // Ensure that every VarDecl has an initializer. + if (!Expr) return; // If VarDecl and Initializer have mismatching unqualified types. - if (!Context->hasSameUnqualifiedType(V->getType(), NewExpr->getType())) + if (!Context->hasSameUnqualifiedType(V->getType(), GetType(Expr))) return; // All subsequent variables in this declaration should have the same @@ -367,9 +380,13 @@ void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context) { Loc.getTypeLocClass() == TypeLoc::Qualified) Loc = Loc.getNextTypeLoc(); } + while (Loc.getTypeLocClass() == TypeLoc::LValueReference || + Loc.getTypeLocClass() == TypeLoc::RValueReference || + 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"); + auto Diag = diag(Range.getBegin(), Message); // Space after 'auto' to handle cases where the '*' in the pointer type is // next to the identifier. This avoids changing 'int *p' into 'autop'. @@ -382,7 +399,19 @@ void UseAutoCheck::check(const MatchFinder::MatchResult &Result) { replaceIterators(Decl, Result.Context); } else if (const auto *Decl = Result.Nodes.getNodeAs<DeclStmt>(DeclWithNewId)) { - replaceNew(Decl, Result.Context); + replaceExpr(Decl, Result.Context, + [](const Expr *Expr) { return Expr->getType(); }, + "use auto when initializing with new to avoid " + "duplicating the type name"); + } else if (const auto *Decl = + Result.Nodes.getNodeAs<DeclStmt>(DeclWithCastId)) { + replaceExpr( + Decl, Result.Context, + [](const Expr *Expr) { + return cast<ExplicitCastExpr>(Expr)->getTypeAsWritten(); + }, + "use auto when initializing with a cast to avoid duplicating the type " + "name"); } else { llvm_unreachable("Bad Callback. No node provided."); } diff --git a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.h b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.h index c63db06b503..882e757a1b9 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.h @@ -25,7 +25,9 @@ public: private: void replaceIterators(const DeclStmt *D, ASTContext *Context); - void replaceNew(const DeclStmt *D, ASTContext *Context); + void replaceExpr(const DeclStmt *D, ASTContext *Context, + std::function<QualType(const Expr *)> GetType, + StringRef Message); const bool RemoveStars; }; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index bdb0138cf66..6b703d8fbb8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -81,6 +81,10 @@ Improvements to clang-tidy Warns if an object is used after it has been moved, without an intervening reinitialization. +- The `modernize-use-auto + <http://clang.llvm.org/extra/clang-tidy/checks/modernize.html>`_ check + now warns about variable declarations that are initialized with a cast. + - New `mpi-buffer-deref <http://clang.llvm.org/extra/clang-tidy/checks/mpi-buffer-deref.html>`_ check diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize-use-auto.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize-use-auto.rst index ce0363255ec..377005d9a98 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize-use-auto.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize-use-auto.rst @@ -114,7 +114,7 @@ New expressions --------------- Frequently, when a pointer is declared and initialized with ``new``, the -pointee type has to be written twice: in the declaration type and in the +pointee type is written twice: in the declaration type and in the ``new`` expression. In this cases, the declaration type can be replaced with ``auto`` improving readability and maintainability. @@ -143,6 +143,25 @@ the following conditions are satisfied: auto *my_first_pointer = new TypeName, *my_second_pointer = new TypeName; +Cast expressions +---------------- + +Frequently, when a variable is declared and initialized with a cast, the +variable type is written twice: in the declaration type and in the +cast expression. In this cases, the declaration type can be replaced with +``auto`` improving readability and maintainability. + +.. code-block:: c++ + + TypeName *my_pointer = static_cast<TypeName>(my_param); + + // becomes + + auto *my_pointer = static_cast<TypeName>(my_param); + +The check handles ``static_cast``, ``dynamic_cast``, ``const_cast``, +``reinterpret_cast``, functional casts and C-style casts. + Known Limitations ----------------- @@ -151,6 +170,9 @@ Known Limitations * User-defined iterators are not handled at this time. +* Function templates that behave as casts, such as ``llvm::dyn_cast``, + ``boost::lexical_cast`` or ``gsl::narrow_cast`` are not handled. + Options ------- diff --git a/clang-tools-extra/test/clang-tidy/modernize-use-auto-cast-remove-stars.cpp b/clang-tools-extra/test/clang-tidy/modernize-use-auto-cast-remove-stars.cpp new file mode 100644 index 00000000000..9541bb4dee7 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/modernize-use-auto-cast-remove-stars.cpp @@ -0,0 +1,118 @@ +// RUN: %check_clang_tidy %s modernize-use-auto %t -- \ +// RUN: -config="{CheckOptions: [{key: modernize-use-auto.RemoveStars, value: '1'}]}" \ +// RUN: -- -std=c++11 + +struct A { + virtual ~A() {} +}; + +struct B : public A {}; + +struct C {}; + +void f_static_cast() { + long l = 1; + int i1 = static_cast<int>(l); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto i1 = static_cast<int>(l); + + const int i2 = static_cast<int>(l); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: const auto i2 = static_cast<int>(l); + + long long ll = static_cast<long long>(l); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto ll = static_cast<long long>(l); + unsigned long long ull = static_cast<unsigned long long>(l); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto ull = static_cast<unsigned long long>(l); + unsigned int ui = static_cast<unsigned int>(l); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto ui = static_cast<unsigned int>(l); + long double ld = static_cast<long double>(l); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto ld = static_cast<long double>(l); + + A *a = new B(); + B *b1 = static_cast<B *>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto b1 = static_cast<B *>(a); + + B *const b2 = static_cast<B *>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto const b2 = static_cast<B *>(a); + + const B *b3 = static_cast<const B *>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto b3 = static_cast<const B *>(a); + + B &b4 = static_cast<B &>(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto &b4 = static_cast<B &>(*a); + + const B &b5 = static_cast<const B &>(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: const auto &b5 = static_cast<const B &>(*a); + + B &b6 = static_cast<B &>(*a), &b7 = static_cast<B &>(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto &b6 = static_cast<B &>(*a), &b7 = static_cast<B &>(*a); + + // Don't warn when auto is already being used. + auto i3 = static_cast<int>(l); + auto *b8 = static_cast<B *>(a); + auto &b9 = static_cast<B &>(*a); +} + +void f_dynamic_cast() { + A *a = new B(); + B *b1 = dynamic_cast<B *>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto b1 = dynamic_cast<B *>(a); + + B &b2 = dynamic_cast<B &>(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto &b2 = dynamic_cast<B &>(*a); +} + +void f_reinterpret_cast() { + auto *a = new A(); + C *c1 = reinterpret_cast<C *>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto c1 = reinterpret_cast<C *>(a); + + C &c2 = reinterpret_cast<C &>(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto &c2 = reinterpret_cast<C &>(*a); +} + +void f_const_cast() { + const A *a1 = new A(); + A *a2 = const_cast<A *>(a1); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto a2 = const_cast<A *>(a1); + A &a3 = const_cast<A &>(*a1); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto &a3 = const_cast<A &>(*a1); +} + +void f_cstyle_cast() { + auto *a = new A(); + C *c1 = (C *)a; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto c1 = (C *)a; + + C &c2 = (C &)*a; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto &c2 = (C &)*a; +} + +void f_functional_cast() { + long l = 1; + int i1 = int(l); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto i1 = int(l); + + B b; + A a = A(b); +} diff --git a/clang-tools-extra/test/clang-tidy/modernize-use-auto-cast.cpp b/clang-tools-extra/test/clang-tidy/modernize-use-auto-cast.cpp new file mode 100644 index 00000000000..06ab3a20075 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/modernize-use-auto-cast.cpp @@ -0,0 +1,120 @@ +// RUN: %check_clang_tidy %s modernize-use-auto %t -- -- \ +// RUN: -std=c++11 -I %S/Inputs/modernize-use-auto + +struct A { + virtual ~A() {} +}; + +struct B : public A {}; + +struct C {}; + +void f_static_cast() { + long l = 1; + int i1 = static_cast<int>(l); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto i1 = static_cast<int>(l); + + const int i2 = static_cast<int>(l); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: const auto i2 = static_cast<int>(l); + + long long ll = static_cast<long long>(l); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto ll = static_cast<long long>(l); + unsigned long long ull = static_cast<unsigned long long>(l); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto ull = static_cast<unsigned long long>(l); + unsigned int ui = static_cast<unsigned int>(l); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto ui = static_cast<unsigned int>(l); + long double ld = static_cast<long double>(l); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto ld = static_cast<long double>(l); + + A *a = new B(); + B *b1 = static_cast<B *>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto *b1 = static_cast<B *>(a); + + B *const b2 = static_cast<B *>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto *const b2 = static_cast<B *>(a); + + const B *b3 = static_cast<const B *>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: const auto *b3 = static_cast<const B *>(a); + + B &b4 = static_cast<B &>(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto &b4 = static_cast<B &>(*a); + + const B &b5 = static_cast<const B &>(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: const auto &b5 = static_cast<const B &>(*a); + + B &b6 = static_cast<B &>(*a), &b7 = static_cast<B &>(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto &b6 = static_cast<B &>(*a), &b7 = static_cast<B &>(*a); + + // Don't warn when non-cast involved + long double cast = static_cast<long double>(l), noncast = 5; + + // Don't warn when auto is already being used. + auto i3 = static_cast<int>(l); + auto *b8 = static_cast<B *>(a); + auto &b9 = static_cast<B &>(*a); +} + +void f_dynamic_cast() { + A *a = new B(); + B *b1 = dynamic_cast<B *>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto *b1 = dynamic_cast<B *>(a); + + B &b2 = dynamic_cast<B &>(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto &b2 = dynamic_cast<B &>(*a); +} + +void f_reinterpret_cast() { + auto *a = new A(); + C *c1 = reinterpret_cast<C *>(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto *c1 = reinterpret_cast<C *>(a); + + C &c2 = reinterpret_cast<C &>(*a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto &c2 = reinterpret_cast<C &>(*a); +} + +void f_const_cast() { + const A *a1 = new A(); + A *a2 = const_cast<A *>(a1); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto *a2 = const_cast<A *>(a1); + A &a3 = const_cast<A &>(*a1); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto &a3 = const_cast<A &>(*a1); +} + +void f_cstyle_cast() { + auto *a = new A(); + C *c1 = (C *)a; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto *c1 = (C *)a; + + C &c2 = (C &)*a; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto &c2 = (C &)*a; +} + +void f_functional_cast() { + long l = 1; + int i1 = int(l); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with a cast to avoid duplicating the type name + // CHECK-FIXES: auto i1 = int(l); + + B b; + A a = A(b); +} diff --git a/clang-tools-extra/test/clang-tidy/modernize-use-auto-new.cpp b/clang-tools-extra/test/clang-tidy/modernize-use-auto-new.cpp index 7ced0a4735e..f6984a19b83 100644 --- a/clang-tools-extra/test/clang-tidy/modernize-use-auto-new.cpp +++ b/clang-tools-extra/test/clang-tidy/modernize-use-auto-new.cpp @@ -15,6 +15,10 @@ void auto_new() { // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use auto when initializing with new // CHECK-FIXES: static auto *a_static = new MyType(); + long long *ll = new long long(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new + // CHECK-FIXES: auto *ll = new long long(); + MyType *derived = new MyDerivedType(); void *vd = new MyType(); |
