<div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">My 2cents: </div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">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. 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.</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif"><br>re: context-sensitive suggestions. </div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">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? Why doesn't your concern already apply to the generic matchers like "anyOf", "hasParent", etc.</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif"><br></div><span class="gmail_default" style="font-family:arial,helvetica,sans-serif">Overall, </span>I'm<span class="gmail_default" style="font-family:arial,helvetica,sans-serif"> </span>pessimistic ab<span class="gmail_default" style="font-family:arial,helvetica,sans-serif"></span>out making the current matchers 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.<span style="font-family:arial,helvetica,sans-serif"> I have opinions on alternatives (like Syntax Trees, touched on above, but also other ideas), but this is not the place. Even for your auto-suggestions<span class="gmail_default" style="font-family:arial,helvetica,sans-serif"> (which I really like)</span>, I think it would be more beneficial applied to such alternatives.</span><div class="gmail_default" style="font-family:arial,helvetica,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">Cheers,</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">yitzhak</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 22, 2020 at 5:51 PM Stephen Kelly via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<p><br>
</p>
<div class="gmail_quote">On 21/06/2020 19:59, Richard Smith wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<p>If I understand your suggestion, given</p>
<p><br>
```<br>
struct B<br>
{<br>
B(int);<br>
};<br>
<br>
B func1() {<br>
return 42;<br>
}<br>
```<br>
</p>
<p>a matcher like
`returnStmt(hasReturnValue(fooExpr()))`<br>
</p>
<p>would match for `fooExpr` as any of<br>
</p>
<br>
* exprWithCleanups()<br>
* cxxConstructExpr() (twice)<br>
* materializeTemporaryExpr()<br>
* implicitCastExpr()<br>
* integerLiteral()
<p>right?<br>
</p>
</div>
</blockquote>
<div>Yes.</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<p> </p>
<p>and a matcher like
`returnStmt(hasReturnValue(expr().bind("theReturnVal")))`</p>
<p>would bind "theReturnVal" to the
`exprWithCleanups()` right?<br>
</p>
<p>Your suggestion may be that it should bind to the
integerLiteral() in that case. That may have more
scope for confusion though?</p>
</div>
</blockquote>
<div>I think that's an interesting question. My
inclination would be to bind to the ExprWithCleanups.
<br>
</div>
</div>
</div>
</blockquote>
</div>
</blockquote>
</div>
<p><br>
</p>
<p>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.<br>
</p>
<p>For example, if I write<br>
</p>
<p> returnStmt()</p>
<p>in clang-query it should tell me that I can write <br>
</p>
<p> hasReturnValue(integerLiteral())<br>
</p>
<p>inside the expr():</p>
<p> <a href="http://ce.steveire.com/z/9EF8x0" target="_blank">http://ce.steveire.com/z/9EF8x0</a></p>
<p>That's how I as a newcomer quickly discover hasReturnValue() and
integerLiteral().</p>
<p>You have no idea how overwhelming <br>
</p>
<p> <a href="http://clang.llvm.org/docs/LibASTMatchersReference.html" target="_blank">http://clang.llvm.org/docs/LibASTMatchersReference.html</a><br>
</p>
<p>is to someone who has zero familiarity with the clang AST.</p>
<p>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.<br>
</p>
<p>However, if all of the implicit nodes should also appear in that
output, and if in<br>
</p>
<p>`returnStmt(hasReturnValue(expr().bind("theReturnVal")))`</p>
<p>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.<br>
</p>
<p>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.</p>
<p>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.<br>
</p>
<p>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. <br>
</p>
<p>That might be the best compromise.<br>
</p>
<p>Thanks,</p>
<p>Stephen<br>
</p>
<p><br>
</p>
</div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>