================
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();
----------------
Dmitri Gribenko wrote:
> I think we do want to compare sugared types (unwrapping the sugar until we
> hit std::somethig?), as per note about iterator types below.
Can you tell me more about this? Sugar just means different names for the same
thing right? My understanding of 'canonical' was basically the bottom-most
layer of sugar. Should I do something different?
================
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
----------------
Dmitri Gribenko wrote:
> What about `MyType A(42)`?
Added this case to the comment.
================
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:
> 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).
I added a FIXME to the matcher. I agree that testing if std::iterator_traits<T>
exists or instantiates properly is more robust but for now excluding pointers
is ok with me. According to C++ [iterator.traits] p2 the default template
expects the traits to exist within the class as the matcher currently searches
for.
Handling pointers as iterators will require a bit more work anyway (e.g.
function pointer example you gave). I think that would be fine content for a
second patch.
Since ASTMatchers will only match stuff that's actually instantiated, any
suggestions on how I'd go about testing if std::iterator_traits<T>
instantiates? Or perhaps we assume knowledge of how std::iterator_traits is
implemented and treat all pointers and const pointers as valid iterators along
with classes that have the traits members.
http://llvm-reviews.chandlerc.com/D392
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
