Fix UseAuto replacing variable declaration lists containing non-initialized

variables.

UseAuto used to match initialized variable declarations independently of
whether they were defined in a declaration list or as a single declaration.
Now it matches declaration statements where every variable declaration is
initialized.

llvm-svn: 182114

4 files changed, 81 insertions, 53 deletions

diff --git a/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoActions.cpp b/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoActions.cpp
index bd2599c4685..27d8acc6f76 100644
--- a/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoActions.cpp
+++ b/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoActions.cpp
@@ -20,54 +20,60 @@ using namespace clang::ast_matchers;
 using namespace clang::tooling;
 using namespace clang;
 
-void IteratorReplacer::run(const MatchFinder::MatchResult &Result) {
-  const VarDecl *D = Result.Nodes.getNodeAs<VarDecl>(IteratorDeclId);
 
+void IteratorReplacer::run(const MatchFinder::MatchResult &Result) {
+  const DeclStmt *D = Result.Nodes.getNodeAs<DeclStmt>(IteratorDeclStmtId);
   assert(D && "Bad Callback. No node provided");
 
   SourceManager &SM = *Result.SourceManager;
   if (!SM.isFromMainFile(D->getLocStart()))
     return;
 
-  const Expr *ExprInit = D->getInit();
+  for (clang::DeclStmt::const_decl_iterator I  = D->decl_begin(),
+        E = D->decl_end(); I != E; ++I) {
+    const VarDecl *V = cast<VarDecl>(*I);
 
-  // Skip expressions with cleanups from the initializer expression.
-  if (const ExprWithCleanups *E = dyn_cast<ExprWithCleanups>(ExprInit))
-    ExprInit = E->getSubExpr();
+    const Expr *ExprInit = V->getInit();
 
-  const CXXConstructExpr *Construct = cast<CXXConstructExpr>(ExprInit);
+    // Skip expressions with cleanups from the initializer expression.
+    if (const ExprWithCleanups *E = dyn_cast<ExprWithCleanups>(ExprInit))
+      ExprInit = E->getSubExpr();
 
-  assert(Construct->getNumArgs() == 1u &&
-         "Expected constructor with single argument");
+    const CXXConstructExpr *Construct = cast<CXXConstructExpr>(ExprInit);
 
-  // Drill down to the as-written initializer.
-  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.
-    return;
+    assert(Construct->getNumArgs() == 1u &&
+           "Expected constructor with single argument");
 
-  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.
-    if (NestedConstruct->getConstructor()->isConvertingConstructor(false))
+    // Drill down to the as-written initializer.
+    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.
       return;
 
-  if (Result.Context->hasSameType(D->getType(), E->getType())) {
-    TypeLoc TL = D->getTypeSourceInfo()->getTypeLoc();
+    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.
+      if (NestedConstruct->getConstructor()->isConvertingConstructor(false))
+        return;
+    if (!Result.Context->hasSameType(V->getType(), E->getType()))
+      return;
+  }
+  // Get the type location using the first declartion.
+  const VarDecl *V = cast<VarDecl>(*D->decl_begin());
+  TypeLoc TL = V->getTypeSourceInfo()->getTypeLoc();
 
-    // WARNING: TypeLoc::getSourceRange() will include the identifier for things
-    // like function pointers. Not a concern since this action only works with
-    // iterators but something to keep in mind in the future.
+  // WARNING: TypeLoc::getSourceRange() will include the identifier for things
+  // like function pointers. Not a concern since this action only works with
+  // iterators but something to keep in mind in the future.
 
-    CharSourceRange Range(TL.getSourceRange(), true);
-    Replace.insert(tooling::Replacement(SM, Range, "auto"));
-    ++AcceptedChanges;
-  }
+  CharSourceRange Range(TL.getSourceRange(), true);
+  Replace.insert(tooling::Replacement(SM, Range, "auto"));
+  ++AcceptedChanges;
 }
 
 void NewReplacer::run(const MatchFinder::MatchResult &Result) {
@@ -77,7 +83,7 @@ void NewReplacer::run(const MatchFinder::MatchResult &Result) {
   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");
 
diff --git a/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.cpp b/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.cpp
index 08f9a7e52f6..246378ac9aa 100644
--- a/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.cpp
+++ b/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.cpp
@@ -18,7 +18,7 @@
 using namespace clang::ast_matchers;
 using namespace clang;
 
-const char *IteratorDeclId = "iterator_decl";
+const char *IteratorDeclStmtId = "iterator_decl";
 const char *DeclWithNewId = "decl_new";
 const char *NewExprId = "new_expr";
 
@@ -232,20 +232,30 @@ TypeMatcher iteratorFromUsingDeclaration() {
 }
 } // namespace
 
-DeclarationMatcher makeIteratorDeclMatcher() {
-  return varDecl(allOf(
-                   hasWrittenNonListInitializer(),
-                   unless(hasType(autoType())),
-                   hasType(
-                     isSugarFor(
-                       anyOf(
-                         typedefIterator(),
-                         nestedIterator(),
-                         iteratorFromUsingDeclaration()
-                       )
-                     )
-                   )
-                 )).bind(IteratorDeclId);
+// \brief This matcher returns delaration 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()
+            )
+          )
+        ))
+      )
+    )))
+  ).bind(IteratorDeclStmtId);
 }
 
 DeclarationMatcher makeDeclWithNewMatcher() {
diff --git a/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.h b/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.h
index 1f09ad407cd..d02a6a71f9d 100644
--- a/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.h
+++ b/clang-tools-extra/cpp11-migrate/UseAuto/UseAutoMatchers.h
@@ -17,14 +17,14 @@
 
 #include "clang/ASTMatchers/ASTMatchers.h"
 
-extern const char *IteratorDeclId;
+extern const char *IteratorDeclStmtId;
 extern const char *DeclWithNewId;
 extern const char *NewExprId;
 
-/// \brief Create a matcher that matches variable declarations where the type
-/// is an iterator for an std container and has an explicit initializer of the
-/// same type.
-clang::ast_matchers::DeclarationMatcher makeIteratorDeclMatcher();
+/// \brief Create a matcher that matches declaration staments that have
+/// variable declarations where the type is an iterator for an std container
+/// and has an explicit initializer of the same type.
+clang::ast_matchers::StatementMatcher makeIteratorDeclMatcher();
 
 /// \brief Create a matcher that matches variable declarations that are
 /// initialized by a C++ new expression.
diff --git a/clang-tools-extra/test/cpp11-migrate/UseAuto/iterator.cpp b/clang-tools-extra/test/cpp11-migrate/UseAuto/iterator.cpp
index 59c2979b32e..7c4b5aa4678 100644
--- a/clang-tools-extra/test/cpp11-migrate/UseAuto/iterator.cpp
+++ b/clang-tools-extra/test/cpp11-migrate/UseAuto/iterator.cpp
@@ -156,5 +156,17 @@ int main(int argc, char **argv) {
     // CHECK: auto I = MapFind.find("foo");
   }
 
+  // Test for declaration lists
+  {
+    // Ensusre declaration lists that matches the declaration type with written
+    // no-list initializer are transformed.
+    std::vector<int>::iterator I = Vec.begin(), E = Vec.end();
+    // CHECK: auto I = Vec.begin(), E = Vec.end();
+
+    // Declaration lists with non-initialized variables should not be
+    // transformed.
+    std::vector<int>::iterator J = Vec.begin(), K;
+    // CHECK: std::vector<int>::iterator J = Vec.begin(), K;
+  }
   return 0;
 }