diff options
4 files changed, 126 insertions, 59 deletions
diff --git a/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoActions.cpp b/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoActions.cpp index 3eac4db468a..47050a7b84d 100644 --- a/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoActions.cpp +++ b/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoActions.cpp @@ -20,7 +20,6 @@ using namespace clang::ast_matchers; using namespace clang::tooling; using namespace clang; - void IteratorReplacer::run(const MatchFinder::MatchResult &Result) { const DeclStmt *D = Result.Nodes.getNodeAs<DeclStmt>(IteratorDeclStmtId); assert(D && "Bad Callback. No node provided"); @@ -49,16 +48,16 @@ void IteratorReplacer::run(const MatchFinder::MatchResult &Result) { const Expr *E = Construct->arg_begin()->IgnoreParenImpCasts(); if (E != E->IgnoreConversionOperator()) // We hit a conversion operator. Early-out now as they imply an implicit - // conversion from a different type. Could also mean an explicit conversion - // from the same type but that's pretty rare. + // conversion from a different type. Could also mean an explicit + // conversion from the same type but that's pretty rare. return; if (const CXXConstructExpr *NestedConstruct = dyn_cast<CXXConstructExpr>(E)) // If we ran into an implicit conversion constructor, can't convert. // // FIXME: The following only checks if the constructor can be used - // implicitly, not if it actually was. Cases where the converting constructor - // was used explicitly won't get converted. + // implicitly, not if it actually was. Cases where the converting + // constructor was used explicitly won't get converted. if (NestedConstruct->getConstructor()->isConvertingConstructor(false)) return; if (!Result.Context->hasSameType(V->getType(), E->getType())) @@ -78,46 +77,69 @@ void IteratorReplacer::run(const MatchFinder::MatchResult &Result) { } void NewReplacer::run(const MatchFinder::MatchResult &Result) { - const VarDecl *D = Result.Nodes.getNodeAs<VarDecl>(DeclWithNewId); + const DeclStmt *D = Result.Nodes.getNodeAs<DeclStmt>(DeclWithNewId); assert(D && "Bad Callback. No node provided"); SourceManager &SM = *Result.SourceManager; if (!SM.isFromMainFile(D->getLocStart())) return; - const CXXNewExpr *NewExpr = Result.Nodes.getNodeAs<CXXNewExpr>(NewExprId); - assert(NewExpr && "Bad Callback. No CXXNewExpr bound"); - - // If declaration and initializer have exactly the same type, just replace - // with 'auto'. - if (Result.Context->hasSameType(D->getType(), NewExpr->getType())) { - TypeLoc TL = D->getTypeSourceInfo()->getTypeLoc(); - CharSourceRange Range(TL.getSourceRange(), /*IsTokenRange=*/ true); - // Space after 'auto' to handle cases where the '*' in the pointer type - // is next to the identifier. This avoids changing 'int *p' into 'autop'. - Replace.insert(tooling::Replacement(SM, Range, "auto ")); - ++AcceptedChanges; - return; - } + const VarDecl *FirstDecl = cast<VarDecl>(*D->decl_begin()); + // Ensure that there is at least one VarDecl within de DeclStmt. + assert(FirstDecl && "No VarDecl provided"); - // If the CV qualifiers for the pointer differ then we still use auto, just - // need to leave the qualifier behind. - if (Result.Context->hasSameUnqualifiedType(D->getType(), - NewExpr->getType())) { - TypeLoc TL = D->getTypeSourceInfo()->getTypeLoc(); - CharSourceRange Range(TL.getSourceRange(), /*IsTokenRange=*/ true); - // Space after 'auto' to handle cases where the '*' in the pointer type - // is next to the identifier. This avoids changing 'int *p' into 'autop'. - Replace.insert(tooling::Replacement(SM, Range, "auto ")); - ++AcceptedChanges; - return; - } + const QualType FirstDeclType = FirstDecl->getType().getCanonicalType(); + + std::vector<SourceLocation> StarLocations; + for (clang::DeclStmt::const_decl_iterator DI = D->decl_begin(), + DE = D->decl_end(); + DI != DE; ++DI) { + + const VarDecl *V = cast<VarDecl>(*DI); + // Ensure that every DeclStmt child is a VarDecl. + assert(V && "No VarDecl provided"); + + const CXXNewExpr *NewExpr = + cast<CXXNewExpr>(V->getInit()->IgnoreParenImpCasts()); + // Ensure that every VarDecl has a CXXNewExpr initializer. + assert(NewExpr && "No CXXNewExpr provided"); + + // If VarDecl and Initializer have mismatching unqualified types. + if (!Result.Context->hasSameUnqualifiedType(V->getType(), + NewExpr->getType())) + return; + + // Remove explicitly written '*' from declarations where there's more than + // one declaration in the declaration list. + if (DI == D->decl_begin()) + continue; - // The VarDecl and Initializer have mismatching types. - return; + // All subsequent delcarations should match the same non-decorated type. + if (FirstDeclType != V->getType().getCanonicalType()) + return; + + PointerTypeLoc Q = + V->getTypeSourceInfo()->getTypeLoc().getAs<PointerTypeLoc>(); + while (!Q.isNull()) { + StarLocations.push_back(Q.getStarLoc()); + Q = Q.getNextTypeLoc().getAs<PointerTypeLoc>(); + } + } + // Remove '*' from declarations using the saved star locations. + for (std::vector<SourceLocation>::iterator I = StarLocations.begin(), + E = StarLocations.end(); + I != E; ++I) { + Replace.insert(tooling::Replacement(SM, *I, 1, "")); + } // 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. + CharSourceRange Range( + FirstDecl->getTypeSourceInfo()->getTypeLoc().getSourceRange(), true); + // Space after 'auto' to handle cases where the '*' in the pointer type + // is next to the identifier. This avoids changing 'int *p' into 'autop'. + Replace.insert(tooling::Replacement(SM, Range, "auto ")); + ++AcceptedChanges; } diff --git a/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.cpp b/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.cpp index 246378ac9aa..60d29b24584 100644 --- a/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.cpp +++ b/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.cpp @@ -258,29 +258,29 @@ StatementMatcher makeIteratorDeclMatcher() { ).bind(IteratorDeclStmtId); } -DeclarationMatcher makeDeclWithNewMatcher() { - return varDecl( - hasInitializer( - ignoringParenImpCasts( - newExpr().bind(NewExprId) - ) - ), - - // FIXME: TypeLoc information is not reliable where CV qualifiers are - // concerned so these types can't be handled for now. - unless(hasType(pointerType(pointee(hasLocalQualifiers())))), +StatementMatcher makeDeclWithNewMatcher() { + return declStmt( + has(varDecl()), + unless(has(varDecl( + anyOf( + unless(hasInitializer( + ignoringParenImpCasts(newExpr()) + )), + // FIXME: TypeLoc information is not reliable where CV qualifiers are + // concerned so these types can't be handled for now. + hasType(pointerType(pointee(hasCanonicalType(hasLocalQualifiers())))), - // FIXME: Handle function pointers. For now we ignore them because - // the replacement replaces the entire type specifier source range - // which includes the identifier. - unless( - hasType( - pointsTo( - pointsTo( - parenType(innerType(functionType())) - ) - ) - ) - ) - ).bind(DeclWithNewId); + // FIXME: Handle function pointers. For now we ignore them because + // the replacement replaces the entire type specifier source range + // which includes the identifier. + hasType( + pointsTo( + pointsTo( + parenType(innerType(functionType())) + ) + ) + ) + ) + ))) + ).bind(DeclWithNewId); } diff --git a/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.h b/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.h index d02a6a71f9d..f2c8f84f262 100644 --- a/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.h +++ b/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.h @@ -28,6 +28,6 @@ clang::ast_matchers::StatementMatcher makeIteratorDeclMatcher(); /// \brief Create a matcher that matches variable declarations that are /// initialized by a C++ new expression. -clang::ast_matchers::DeclarationMatcher makeDeclWithNewMatcher(); +clang::ast_matchers::StatementMatcher makeDeclWithNewMatcher(); #endif // LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_USE_AUTO_MATCHERS_H diff --git a/clang-tools-extra/test/cpp11-migrate/UseAuto/new.cpp b/clang-tools-extra/test/cpp11-migrate/UseAuto/new.cpp index 911c0cd76bd..3fca05df29b 100644 --- a/clang-tools-extra/test/cpp11-migrate/UseAuto/new.cpp +++ b/clang-tools-extra/test/cpp11-migrate/UseAuto/new.cpp @@ -49,4 +49,49 @@ int main(int argc, char **argv) { MyType *g{new MyType}; // CHECK: MyType *g{new MyType}; + + // Test for declaration lists. + { + MyType *a = new MyType(), *b = new MyType(), *c = new MyType(); + // CHECK: auto a = new MyType(), b = new MyType(), c = new MyType(); + + // Non-initialized declaration should not be transformed. + MyType *d = new MyType(), *e; + // CHECK: MyType *d = new MyType(), *e; + + MyType **f = new MyType*(), **g = new MyType*(); + // CHECK: auto f = new MyType*(), g = new MyType*(); + + // Mismatching types in declaration lists should not be transformed. + MyType *h = new MyType(), **i = new MyType*(); + // CHECK: 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*(); + // CHECK: MyType *j = new MyType(), *k = new MyType(), **l = new MyType*(); + } + + // Test for typedefs. + { + typedef int * int_p; + + int_p a = new int; + // CHECK: auto a = new int; + int_p *b = new int*; + // CHECK: auto b = new int*; + + // Test for typedefs in declarations lists. + int_p c = new int, d = new int; + // CHECK: auto c = new int, d = new int; + + // Different types should not be transformed. + int_p e = new int, *f = new int_p; + // CHECK: int_p e = new int, *f = new int_p; + + int_p *g = new int*, *h = new int_p; + // CHECK: auto g = new int*, h = new int_p; + } + + return 0; } |
