[PATCH] Introducing initial UseAuto transform

Dmitri Gribenko gribozavr at gmail.com
Mon Feb 11 11:31:44 PST 2013



================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:51-52
@@ +50,4 @@
+    }
+    ImplicitInit = Construct->getNumArgs() == 0 ||
+                   Construct->getArg(0)->isDefaultArgument();
+  } else {
----------------
Edwin Vane wrote:
> 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?
Just the former.

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

================
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;
----------------
Edwin Vane wrote:
> 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.
By 'this' I mean replacing the type using getSourceRange on TypeLoc.  It will not work correctly for function pointer types spelled directly -- `void (*f)(int)` -- the SourceRange will be the whole declaration, including the identifier `f`.

No, even iterators for standard containers are not required to have those members.  One should use std::iterator_traits to use those traits.  For example, if vector::iterator is just a plain pointer `T*`, then a standard specialization of iterator_traits<T*> will take care of that, while using member references is an error.

Of course, I see how checking if std::iterator is a parent class, helps to easily identify types that are iterators.  But what I'm saying is that there are other cases where this is not sufficient (and there are standard libraries that fall into these cases).


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



More information about the cfe-commits mailing list