[cfe-commits] [PATCH] Implements hasAncestor.

[Daniel Jasper]

================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:461
@@ +460,3 @@
+                            llvm::is_base_of<Stmt, T>::value),
+                           only_Decl_or_Stmt_allowed_for_recursive_matching);
+    return matchesAncestorOf(ast_type_traits::DynTypedNode::create(Node),
----------------
Michael Diamond wrote:
> Daniel Jasper wrote:
> > Michael Diamond wrote:
> > > Manuel Klimek wrote:
> > > > Daniel Jasper wrote:
> > > > > Unlike descendants, all node types should have ancestors. Is it intentional to not allow this for QualTypes or possibly other types? 
> > > > > - If there is a good reason for it: Comment.
> > > > > - If it would be significant additional work: Command + FIXME
> > > > > - Otherwise: Implement ;-)
> > > > Like with the other methods here, there are many more nodes for which we want them implemented than are currently implemented.
> > > > 
> > > > You're right that for hasAncestor probably pretty much all nodes make sense. Added a class-level comment that outlines the differences.
> > > I really need this to work for QualTypes. Any chance of getting that in this CL? We don't have to update both ancestor and child matchers together necessarily.
> > Could you quickly elaborate on what you are in need of matching? This is actually not quite as easy (from a design perspective) and I think it should be in a different patch. But maybe we can find a way around it :-).
> If it has to hold of to another patch, alright, but I need it ASAP. :)
> 
> Basically, I am refactoring from using one class to using a parent of it. However, for references and pointers, I want to use an interface that is a parent of that class.
> 
> Example:
> Interface
>   |
> Parent
>   |
> CurrentClass
> 
> I want "CurrentClass*" to become "Interface*", but "new CurrentClass" to become "new Parent".
Interesting. I don't really see why you would need hasAncestor for QualTypes to solve this. More precisely, I don't see how QualType matching is going to help at all. The problem with QualTypes (what I also meant above) is that they don't have a natural identity and we don't yet know what to do about that with hasAncestor. More importantly, QualTypes don't have a SourceLocation you could use for refactoring.

I think what would help a lot would be TypeLoc-Matchers, but maybe that is not necessary. Some thoughts:

  DeclarationMatcher current = recordDecl(hasName("CurrentClass"));
  TypeMatcher pointerOrReference =
    anyOf(pointsTo(current), references(current));
  MatchFinder->addMatcher(decl(anyOf(
    declaratorDecl(hasType(pointerOrReference)).bind("decl"),
    functionDecl(returns(pointerOrReference)).bind("decl"))), Callback);

If decl is a VarDecl or FieldDecl, you can get the SourceLocation of the type out of the result of getTypeSourceInfo(). For FunctionDecls, you have to look at the result and I am not entirely sure how to get to the TypeSourceInfo of that. Also, pointerOrReference needs to be slightly extended to handle arrays and you can do something similar for non-pointer-or-reference cases.

Then you will need to match template parameters.. That should be fairly straight forward.

Then there are CXXConstructExprs (new CurrentClass()).. I think they don't involve QualTypes or TypeLocs. You basically have to

  addMatcher(constructExpr(hasDeclaration(
    constructorDecl(ofClass(current)))).bind("constructExpr"), Callback);

.. and then hope that CXXConstructExpr::getLocation() returns the Token that you need to replace :-/.

And then you might need to worry about static members of the class being accessed (no QualTypes / TypeLocs involved).

Ah, and using declarations ..

So lots of stuff to do, I hope I have not rambled too much. But I don't see imminent need for hasAncestor() on QualTypes. hasDescendant() might actually help more with the pointerOrReferenceOrArrayOr..-business. But of course, I might have overlooked a really easy thing to do with hasAncestor().



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