[PATCH] Introducing initial UseAuto transform

[Edwin Vane]

================
Comment at: cpp11-migrate/UseAuto/UseAutoActions.cpp:38
@@ +37,3 @@
+  // Drill down to the as-written initializer.
+  const Expr *E = *Construct->arg_begin();
+
----------------
Dmitri Gribenko wrote:
> An assertion to check the number of arguments would be nice.
The matcher this callback is used with guarantees there is an arg but I'll add an assert to make that clear.

================
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();
+    }
+  }
+
----------------
Dmitri Gribenko wrote:
> ParenExpr handling is missing.  Maybe use IgnoreParenImpCasts() ?
> 
I'll play with this and see if I can find a better solution now I have a bit more experience from the other patches.

================
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;
----------------
Dmitri Gribenko wrote:
> 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.
By 'this' do you mean the matcher/callback as a whole? If so, that's by design. As per your note about iterators and how to identify them I purposefully targeted only std iterators which, according to the spec, do either have these type traits directly or because they were inherited from std::iterator. If custom iterators exist in the wild which inherit from std::iterator then they get the transform for free. I figured this will handle the bulk of the most useful cases.

================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:40-42
@@ +39,5 @@
+  const Expr *Init = Node.getAnyInitializer();
+  if (!Init) {
+    return false;
+  }
+
----------------
Dmitri Gribenko wrote:
> Extra braces (and a few cases below).
I presume you mean no braces around single-line control flow? Will fix.

================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:51-52
@@ +50,4 @@
+    }
+    ImplicitInit = Construct->getNumArgs() == 0 ||
+                   Construct->getArg(0)->isDefaultArgument();
+  } else {
----------------
Dmitri Gribenko wrote:
> Please add tests for this.
Do you mean add tests for default constructor and constructor with only default args? Or do you mean 'this' in a broader sense?

================
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<
----------------
Dmitri Gribenko wrote:
> This should probably go into ASTMatchers library.
I thought this matcher was too specific for the library but I can probably break it down into re-usable chunks if that's the consensus.

================
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()};
+
----------------
Dmitri Gribenko wrote:
> Please also add a test for `...::iterator begin2(blah.begin());`
will do.


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