[PATCH] Introducing initial UseAuto transform

[Dmitri Gribenko]

================
Comment at: cpp11-migrate/UseAuto/UseAutoActions.cpp:40-48
@@ +39,11 @@
+
+  if (const MaterializeTemporaryExpr *MTE =
+          dyn_cast<MaterializeTemporaryExpr>(E)) {
+    E = MTE->GetTemporaryExpr();
+  }
+  if (const CastExpr *CE = dyn_cast<CastExpr>(E)) {
+    if (!isa<ExplicitCastExpr>(E)) {
+      E = CE->getSubExprAsWritten();
+    }
+  }
+
----------------
ParenExpr handling is missing.  Maybe use IgnoreParenImpCasts() ?


================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:33-35
@@ +32,5 @@
+/// \code
+///   iterator I = C.begin(); // A
+///   MyType A{2};            // B
+///   MyType B;               // C
+/// \endcode
----------------
What about `MyType A(42)`?

================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:40-42
@@ +39,5 @@
+  const Expr *Init = Node.getAnyInitializer();
+  if (!Init) {
+    return false;
+  }
+
----------------
Extra braces (and a few cases below).

================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:79-80
@@ +78,4 @@
+  return varDecl(allOf(hasWrittenNonListInitializer(),
+                       hasType(recordDecl(anyOf(
+                           isDerivedFrom("std::iterator"),
+                           allOf(has(namedDecl(hasName("value_type"))),
----------------
Iterators are not required to be derived from std::iterator (and they are not required to have any of the members you listed below).  `T *` is a valid iterator type for std::vector<T>.


================
Comment at: cpp11-migrate/UseAuto/UseAutoActions.cpp:50-51
@@ +49,4 @@
+
+  // Compare canonical types so typedefs don't mess the comparison up.
+  if (D->getType().getCanonicalType() == E->getType().getCanonicalType()) {
+    TypeLoc TL = D->getTypeSourceInfo()->getTypeLoc();
----------------
I think we do want to compare sugared types (unwrapping the sugar until we hit std::somethig?), as per note about iterator types below.

================
Comment at: cpp11-migrate/UseAuto/UseAutoActions.cpp:53-54
@@ +52,4 @@
+    TypeLoc TL = D->getTypeSourceInfo()->getTypeLoc();
+    CharSourceRange Range(TL.getSourceRange(), true);
+    Replace.insert(tooling::Replacement(SM, Range, "auto"));
+    ++AcceptedChanges;
----------------
Please note, that while this works for iterators, it will not work in general, for example for function pointers or arrays, or pointers to arrays.

================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:51-52
@@ +50,4 @@
+    }
+    ImplicitInit = Construct->getNumArgs() == 0 ||
+                   Construct->getArg(0)->isDefaultArgument();
+  } else {
----------------
Please add tests for this.

================
Comment at: cpp11-migrate/UseAuto/UseAutoActions.cpp:38
@@ +37,3 @@
+  // Drill down to the as-written initializer.
+  const Expr *E = *Construct->arg_begin();
+
----------------
An assertion to check the number of arguments would be nice.

================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:62-63
@@ +61,4 @@
+
+/// \brief Overload of allOf accepting 5 args.
+template <typename M1, typename M2, typename M3, typename M4, typename M5>
+internal::PolymorphicMatcherWithParam2<
----------------
This should probably go into ASTMatchers library.

================
Comment at: test/cpp11-migrate/UseAuto/iterator.cpp:85-86
@@ +84,4 @@
+
+  std::vector<int>::iterator begin2{blah.begin()};
+  // CHECK: std::vector<int>::iterator begin2{blah.begin()};
+
----------------
Please also add a test for `...::iterator begin2(blah.begin());`


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