<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<div class="moz-cite-prefix">On 17/06/2020 22:47, Richard Smith
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAOfiQqm1bSbP6cCuRgocKYfHz1g9MF+SBaZZT8e7d_YYGv2fFQ@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
Consider the example from Yitzhak Mandelbaum -- the issue there is
actually not whether we have a template or not; rather, it's a
difference between one- and two-argument constructors:<br>
</blockquote>
<blockquote type="cite"
cite="mid:CAOfiQqm1bSbP6cCuRgocKYfHz1g9MF+SBaZZT8e7d_YYGv2fFQ@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div>I hope you can agree that this change in behavior, for an
example such as this, is a regression for the ability for
beginners to understand how matchers work and effectively
write them!</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>Yes, I see your point. The current state doesn't account well for
this case. <br>
</p>
<p>Would it resolve the problem if the CXXConstructExpr was not
excluded in this case (in the initializer of a VarDecl), assuming
that can be implemented? <br>
</p>
<p>I think it would remain better for newcomers not to see (and
match on) the CXXConstructExprs in <a class="moz-txt-link-freetext" href="https://godbolt.org/z/TvUTgy">https://godbolt.org/z/TvUTgy</a> .<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:CAOfiQqm1bSbP6cCuRgocKYfHz1g9MF+SBaZZT8e7d_YYGv2fFQ@mail.gmail.com">
<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">
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div>Perhaps as an alternative, we should not skip
implicit nodes when trying to match a node that
might be implicit (analogous to how Type::getAs
steps over sugar nodes except when looking for
that type sugar node), and instead of hiding
implicit nodes in the AST dump we should dim them
out but still show them?</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>Color wouldn't be particularly distinctive, especially
in godbolt for example, but it also would mean that the
confusing stuff is still *there* in the output. It can
be overwhelming when absolutely everything in the output
is new and confusing.</p>
<p>(I also note that newcomers to Matchers don't need to
see the AST at all if the clang-query shows them the
matchers they can use instead: <a
href="http://ce.steveire.com/z/9EF8x0" target="_blank"
moz-do-not-send="true">http://ce.steveire.com/z/9EF8x0</a>
see my other posts on the topic for more)<br>
</p>
</div>
</blockquote>
<div>Perhaps we should consider the AST dump to be an
expert-level feature and not try to optimize it for beginner
usage, especially if we have better output methods. I'm not
sure, though; I think telling beginners to look at the AST
dump and match what they see there has been somewhat
effective.<br>
</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>Yes, I think it makes sense for newcomers to be able to get
non-overwhelming exposure to the AST but not have to be confronted
with it from the very beginning. That is where making clang-query
output matchers could be useful as a beginning point. That is not
yet upstream yet however.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:CAOfiQqm1bSbP6cCuRgocKYfHz1g9MF+SBaZZT8e7d_YYGv2fFQ@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div>I think we should assume that we can ask the maintainers
of compiler explorer to render the output nicely, if we find
that's a problem. (They already apply some postprocessing to
it, to remove pointer values and the like.)</div>
</div>
</div>
</blockquote>
<p>The "AST Output" on Compiler Explorer filters out pointers, but
the clang-query tool does not. I have designs to have a way to
make clang-query omit them anyway (using enable output simple-ast
instead of detailed-ast - that is not upstream yet, but I think it
means we don't need to do that kind of filtering in Compiler
Explorer).<br>
</p>
<p>I think colors in the output might already show up properly, but
that would still leave the problem of the things being *there*,
even if they are greyed out or surrounded in square brackets
(which I would find even more confusing and noisy).<br>
</p>
<br>
<blockquote type="cite"
cite="mid:CAOfiQqm1bSbP6cCuRgocKYfHz1g9MF+SBaZZT8e7d_YYGv2fFQ@mail.gmail.com">
<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> </p>
<p>Nevertheless, this change in the direction that I
consider easier hasn't been 100% popular among other
clang developers (though I wonder how much of that is
due to being experts?). At this point, if someone wishes
to reverse the change of default I don't think I would
attempt to oppose it. It's not clear to me whether my
vision for making things easier for newcomers (which I
set out in EuroLLVM and which included ignoring these
nodes) is shared. It might be better at this point for
others to decide.</p>
</div>
</blockquote>
<div>I think it's a good goal. But I think the implementation
needs some refinement. Did you have any thoughts on my
suggestion of how to fix this? (That is: try matching
against implicit nodes, but skip them if they don't match.)
I don't think you commented on it.</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>I think the best refinement for now would be to restore the
CXXConstructExpr in the case of a varDecl initializer, if that is
possible (may not be).<br>
</p>
<p><br>
</p>
<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>
<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?<br>
</p>
<p>Thanks,</p>
<p>Stephen<br>
</p>
<p><br>
</p>
</body>
</html>