<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 13/01/2020 16:39, Dmitri Gribenko
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CA+Y5xYespmh4s5dTk7vV2qikSa_GBmH+u9SpgXyMniOuYD1G1Q@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div dir="ltr">
<div dir="ltr">On Fri, Jan 10, 2020 at 4:34 PM Aaron Ballman via
cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org"
target="_blank" moz-do-not-send="true">cfe-dev@lists.llvm.org</a>>
wrote:<br>
</div>
<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">I've not seen much to
suggest the community is not in favor of this<br>
direction, so I think you're all set to post patches for
review. If<br>
there are community concerns, we can address them during the
review<br>
process. Thank you for working on this!<br>
</blockquote>
<div><br>
</div>
<div>Sorry, I just came back from vacation. I'm not sure if it
is a good idea, in particular, for the following reasons:</div>
<div><br>
</div>
<div>- The document specifies the goals in terms of a
solution: "Goal: Users should not have to account for
implicit AST nodes when writing AST Matchers". <br>
</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>The goal is to make AST Matchers more accessible/easy to use for
newcomers/non-experts in the clang AST by no-longer confronting
them with 100% of the complexity on first use.<br>
</p>
<p>I think it's important to bridge that gap. See my EuroLLVM talk
for more.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:CA+Y5xYespmh4s5dTk7vV2qikSa_GBmH+u9SpgXyMniOuYD1G1Q@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div>- The complexity for implementers (although it also
applies to <a href="https://reviews.llvm.org/D61837"
moz-do-not-send="true">https://reviews.llvm.org/D61837</a>,
which landed a while ago, and I didn't see it). Allowing to
run the same matcher in multiple modes increases our support
and testing matrix.</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>I don't think it increases the test matrix more than any other
feature. Can you expand on this?<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:CA+Y5xYespmh4s5dTk7vV2qikSa_GBmH+u9SpgXyMniOuYD1G1Q@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
</div>
<div>- The complexity cliff for users. Many non-trivial
matchers need to explicitly manage implicit AST nodes.</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>You just described the complexity cliff which is (after my
change) not the first thing a newcomer hits.</p>
<p>It's good to move that cliff back so that the newcomer is not
confronted with it immediately.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:CA+Y5xYespmh4s5dTk7vV2qikSa_GBmH+u9SpgXyMniOuYD1G1Q@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
</div>
<div>- The cost of ClangTidy. ClangTidy runs all matchers
simultaneously in one AST traversal. If we implement
implicit/no-implicit modes as separate AST traversals,
ClangTidy would need to run two traversals, one for "easier"
matchers (skipping implicit code), and one for "expert"
matchers (traversing implicit code). We would also need to
build two parent maps.</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>Will Syntax Trees ever be used in clang-tidy? Or will their use
be forbidden in clang-tidy for the same reason of adding to a
runtime cost when used in a mixture?</p>
<p>If your concern is valid it would seem to apply to Syntax Trees
to a greater extent than this patch does.<br>
</p>
<p>That said, I think there is some scope for optimization in my
approach. The optimizations I have in mind (being more efficient
with parent maps) wouldn't apply to Syntax Trees though as that is
a completely separate system.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:CA+Y5xYespmh4s5dTk7vV2qikSa_GBmH+u9SpgXyMniOuYD1G1Q@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
</div>
<div>My best suggestion is to investigate implementing AST
Matchers for syntax trees, and allow jumping between syntax
and semantic nodes in one matcher -- to match syntax first,
and then validate the necessary semantic constraints.</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>My previous email quoted the Syntax Trees RFC saying that the
first step is to "use [the current ASTMatchers API] to find
interesting Clang AST nodes". You seem to be suggesting that the
opposite would be done. Maybe things have evolved since that RFC,
or maybe both approaches should work equally well.<br>
</p>
<p><br>
</p>
<p>As long as the current AST Matchers APIs exist, they should be
improved. Perhaps they will be less important in the future when
we have Syntax Trees for these use-cases instead. However, for
now, I don't think the ease of use improvement in my patches
should be prevented based existence of a future alternative
system.<br>
</p>
<p>Thanks,</p>
<p>Stephen.<br>
</p>
</body>
</html>