Fix UseAuto replacing declaration lists with new expressions

UseAuto used to replace declarion lists with new expressons where some
variable were not initialized with new.
This fix checks that every DeclStmt has a VarDecl with an initializer and it
also ensures that all declarations have the same type.

Added tests for multiple declarations and for typedefs.

llvm-svn: 182736

1 files changed, 56 insertions, 34 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;
 }