================
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to