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

[Stephen Kelly via cfe-dev]

On 24/06/2020 21:26, Yitzhak Mandelbaum wrote:
> My 2cents:
>

Thanks for responding about the matcher output from clang-query.


> I mostly agree with Richard and Sam. My only concern w/ Richard's 
> proposal is that the hybrid model may be even more confusing to the 
> beginner, since it may be harder to predict how a given matcher will 
> match against an AST.  The system will be more forgiving, yet it 
> becomes harder to build a mental model of how it works.


In addition to making clang-query output relatable information, I agree 
with what you wrote here about creating a mental model.


> I think we'd need to be sure we can concisely explain how this mode 
> works, without having to reference any code/algorithms. For example, 
> is there a deterministic elaboration from any unadorned matcher to a 
> matcher with explicit `traverse` annotations? That would allow us to 
> give some examples that illuminate what's happening. Otherwise, I'd 
> prefer we just go back to the original default for the library. I'm 
> fine if beginner-oriented tools want to start with "IgnoreUnless..." 
> as their default. It sounds like Stephen is amenable to this approach 
> as well from his last reply.
>

I think reversing the change of default is a worse outcome, but I think 
it's better than the current state of unhappiness from people.

Further development can't happen until this thread is resolved I think.

However, if the default is changed back, I think there is less reason to 
add the matcher output feature to clang-query, because it will either 
need to

* not show ignoring*() matchers in its output

* show output based on each of (and none of) the ignoring*() matchers 
being applied

I think that would be too noisy and I don't think it's a good thing to 
require newcomer users to immediately create a mental model which 
requires those matchers.


> re: context-sensitive suggestions.
> Stephen, if you drop the need to suggest all possible matchers at that 
> point and only provide (beginner) helpful suggestions, does your 
> concern go away?


It doesn't change my concern.

I don't know how you would determine what should be in the output and 
what shouldn't. Also, this content is not just for beginners. I use the 
feature still.

I don't think this is the thread for details such as your other question.


> Overall, I'mpessimistic about making the current matchers easier for 
> users.


I think if AST Matchers are not going to be removed, we should make them 
easier for users.


> I think the AST is challenging for beginners and band aids will not 
> solve that and can often make it worse. If we want beginner-friendly 
> tooling (and I do!) we need to admit its cost and then invest in it.


Yes, I have started such tooling, but there is much more that can be done.

But, as in this thread, I don't think this is fully a tooling issue. 
It's also about discoverability and that means making it easier for 
users to create a mental model that doesn't require them to put 
ignoringImplicit() everywhere.


Thanks Yitzhak for your input. I remain interested in feedback/input 
from others on the content I quote below.


Thanks,

Stephen.


> On Mon, Jun 22, 2020 at 5:51 PM Stephen Kelly via cfe-dev 
> <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
>
>
>     The main reason I'm not supportive of this idea is that 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
>
>     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
>
>     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. 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.
>
>     But, if making IgnoreUnlessSpelledInSource non-default means that
>     I can continue work toward that goal (and eg ignore template
>     instantiations and other implicit nodes which are not skipped
>     yet), then maybe that's a viable way forward. So, that's my
>     preference now I think.
>
>     That way, people who want expert mode get it by default, and
>     newcomers (and anyone else who doesn't want to write
>     ignoringImplicit() everywhere) can relatively easily get the
>     easier mode, and can use the expert mode when wanting to match
>     implicit nodes.
>
>     That might be the best compromise.
>
>     Thanks,
>
>     Stephen
>
>
>     _______________________________________________
>     cfe-dev mailing list
>     cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
>     https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200629/a644719e/attachment-0001.html>