[cfe-dev] RFC: Easier AST Matching by Default

David Rector via cfe-dev cfe-dev at lists.llvm.org
Tue Jun 30 14:48:04 PDT 2020


I will bow out here and leave it to those wiser and more experienced, but first I’ll try to address your points as best I can.

This is your previous response which describes your goals best:
... I want to make clang-query tell the user what matchers they can use with the nodes in the binding list.

For example, if I write

 returnStmt()

in clang-query it should tell me that I can write 

 hasReturnValue(integerLiteral())

inside the expr():

 http://ce.steveire.com/z/9EF8x0 <http://ce.steveire.com/z/9EF8x0>
That's how I as a newcomer quickly discover hasReturnValue() and integerLiteral().

You have no idea how overwhelming 

 http://clang.llvm.org/docs/LibASTMatchersReference.html <http://clang.llvm.org/docs/LibASTMatchersReference.html>
is to someone who has zero familiarity with the clang AST.

But the above link is interactive and it aids *discovery* which is what my talk was about and which is what I'm trying to make easier with AST Matchers. The above suggested clang-query feature (not upstream yet) tells me at every step which matchers I can use in the context of what I am trying to achieve.

However, if all of the implicit nodes should also appear in that output, and if in

`returnStmt(hasReturnValue(expr().bind("theReturnVal")))`

the expr() should match the exprWithCleanups(), then I don't know how to make that feature work. If the expr() is the exprWithCleanups() then the tool definitely shouldn't tell the user they can write integerLiteral() there. 

Why not?  Why can’t it suggest both?  Or why can’t you just manually filter out the implicit matcher possibilities when desired — see below.
The output for implicit nodes would add lots of noise and would have to be neutral in terms of what it recommends.

The entire point of what I'm trying to do is not present the user with exprWithCleanups() and let them achieve their goals without it. I don't know if that's possible with the ideas floating around at the moment about making AST Matchers present all of the implicit nodes.

I disagree that just because implicit nodes add noise, we must prevent them from matching at all — you’re taking a way a huge amount of the potential additional usefulness of this feature by insisting upon that, unless I misunderstand you.

To tell the users what they can write in place of an expr(), you could just loop over the implicit stack of nodes, what I’m calling the "salt", via the desalt() method I proposed:

```
Expr *E = theReturnVal; 
OS << "Possible matchers:\n"
while (E->isImplicit()) {
	if (shouldDisplayImplicitMatcherPossibilities)
		OS << "|-(implicit) " << getExprMatcherName(E->getKind()) << "()\n"; //e.g. "exprWithCleanups()"
	E = E->desalt();
}
OS << "`-" << getExprMatcherName(E->getKind()) << "()"; // e.g. "integerLiteral()"
```

Easy to omit the stuff you don’t want users to see as you wish.  Not sure why we have to totally prevent implicit nodes from even being matchable at all in this mode just to make that work -- misses a big opportunity to make this useful for experts as well; that is why I describe it as a half measure.

FWIW, for Types and TypeLocs, I believe the solution is what I suggested for Stmts, since a getAs<T>() is already in place, and there is no semantic loss from using it, so hard to argue against letting the user get or match a RecordType from an ElaboratedType etc.

Ultimately Stephen, you put in the work, and are entitled to an up or down vote — that’s fair.  Thank you for that work, and good luck to you and others,

Dave



> 
> 
> 
> On 30/06/2020 00:22, David Rector wrote:
>> Bottom line: you’ve raised a great but subtle point, doggedly pursued it, but now, let’s just get rid of the half measures — let’s go all out with a solution that is easy and lifts all boats.
> 
> I don't agree that it's a half-measure if we don't go with your proposal. I deliberately didn't pursue your proposal long ago. 
> 
> I don't think you pointed out anything in your proposal regarding the clang-query feature.
> 
> I agree that typeloc matching can also be improved. I've focussed on Exprs so far.
> 
> I think it would be great to conclude this thread with either 
> 
> 1) at least a reversal of the default (perhaps so that you can pursue your design) or 
> 
> 2) intention to proceed in the way things are already going with my design.
> 
> I don't mind which we do. 
> 
> I don't want to be left with the blame of having my design half-implemented though :). There are bugs that I can not fix, partly because this is not concluded. 
> 
> If I can't move forward, then reversal is the only way (to give others the space to validate their designs) and I'd like to do that soon, so that this thread is not an open item for me (or anyone else) to deal with.
> 
> Thanks,
> 
> Stephen.
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200630/4bd9918d/attachment.html>


More information about the cfe-dev mailing list