[PATCH] Fix UseAuto replacing declaration lists with new expressions

[Ariel Bernal]

Hi revane,

UseAuto used to replace declaration lists with new expressions where some variables aren't initialized with new.

http://llvm-reviews.chandlerc.com/D839

Files:
  cpp11-migrate/UseAuto/UseAutoActions.cpp
  cpp11-migrate/UseAuto/UseAutoMatchers.cpp
  cpp11-migrate/UseAuto/UseAutoMatchers.h
  test/cpp11-migrate/UseAuto/new.cpp

Index: cpp11-migrate/UseAuto/UseAutoActions.cpp
===================================================================
--- cpp11-migrate/UseAuto/UseAutoActions.cpp
+++ cpp11-migrate/UseAuto/UseAutoActions.cpp
@@ -78,46 +78,53 @@
 }
 
 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;
-  }
+  for (clang::DeclStmt::const_decl_iterator DI = D->decl_begin(),
+                                            DE = D->decl_end();
+       DI != DE; ++DI) {
 
-  // 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 VarDecl *V = cast<VarDecl>(*DI);
+    assert(V && "Bad Callback. No VarDecl provided");
 
-  // The VarDecl and Initializer have mismatching types.
-  return;
+    const CXXNewExpr *NewExpr = cast<CXXNewExpr>(V->getInit()
+                                                  ->IgnoreParenImpCasts());
+    assert(NewExpr && "Bad Callback. No CXXNewExpr provided");
 
+    // If VarDecl and Initializer have mismatching unqualified types.
+    if (!Result.Context->hasSameUnqualifiedType(V->getType(),
+                                                NewExpr->getType()))
+      return;
+
+    if (V->getType()->isPointerType()) {
+      // Find and remove the '*' from pointer types.
+      const MemberPointerTypeLoc Q = cast<MemberPointerTypeLoc>(
+                                          V->getTypeSourceInfo()->getTypeLoc());
+      SourceLocation SS = Q.getStarLoc();
+      SourceRange ReplaceRange(SS, SS);
+      Replace.insert(Replacement(SM,
+                             CharSourceRange::getTokenRange(ReplaceRange), ""));
+    }
+  }
   // 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.
+  const VarDecl *V = cast<VarDecl>(*D->decl_begin());
+  TypeLoc TL = V->getTypeSourceInfo()->getTypeLoc();
+
+  CharSourceRange Range(TL.getSourceRange(), true);
+  // Two spaces after 'auto' to handle cases where the '*' in the pointer type
+  // is next to the identifier. Since we alredy remove the '*', this avoids
+  // changing 'int *p' into autop'.
+  if(!V->getType()->isPointerType())
+    Replace.insert(tooling::Replacement(SM, Range, "auto"));
+  else
+    Replace.insert(tooling::Replacement(SM, Range, "auto  "));
+  ++AcceptedChanges;
 }
Index: cpp11-migrate/UseAuto/UseAutoMatchers.cpp
===================================================================
--- cpp11-migrate/UseAuto/UseAutoMatchers.cpp
+++ cpp11-migrate/UseAuto/UseAutoMatchers.cpp
@@ -258,29 +258,29 @@
   ).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);
 }
Index: cpp11-migrate/UseAuto/UseAutoMatchers.h
===================================================================
--- cpp11-migrate/UseAuto/UseAutoMatchers.h
+++ cpp11-migrate/UseAuto/UseAutoMatchers.h
@@ -28,6 +28,6 @@
 
 /// \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
Index: test/cpp11-migrate/UseAuto/new.cpp
===================================================================
--- test/cpp11-migrate/UseAuto/new.cpp
+++ test/cpp11-migrate/UseAuto/new.cpp
@@ -49,4 +49,13 @@
 
   MyType *g{new MyType};
   // CHECK: MyType *g{new MyType};
+
+  // Test for declaration lists.
+  {
+    MyType *a = new MyType(), *b = new MyType();
+    // CHECK: auto a = new MyType(), b = new MyType();
+    MyType *c = new MyType(), *d;
+    // CHECK: MyType *c = new MyType(), *d;
+  }
+  return 0;
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D839.1.patch
Type: text/x-patch
Size: 7042 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130521/854f0159/attachment.bin>