[cfe-commits] [PATCH] Implements hasAncestor.

[Michael Diamond]

================
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),
----------------
Daniel Jasper wrote:
> 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().
> 
Yes, it looks like I do want TypeLoc-Matchers. I've tried putting together a series of complicated matchers like you describe, but there really are a lot of cases. It got to the point where I'd rather just use regexes.

I guess what I want is something like (yes, some of these matchers are probably off, but this is the idea):

  addMatcher(typeLocMatcher(hasType(hasAnyQualifiers(),
      pointerOrReferenceTo(class(hasName("CurrentClass"))))), Callback);
  addMatcher(typeLocMatcher(hasType(hasAnyQualifiers(),
      class(hasName("CurrentClass")),
      not(hasAncestor(isPointerOrReference()))), OtherCallback);


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