<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 17, 2020 at 11:21 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>On 16/06/2020 05:16, Richard Smith
      wrote:<br>
    </div>
    <blockquote type="cite">
      
      <div dir="ltr">
        <div class="gmail_quote">
          <div dir="ltr" class="gmail_attr">On Mon, 25 May 2020 at
            14:56, Stephen Kelly via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">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">On 20/12/2019 21:01,
            Stephen Kelly via cfe-dev wrote:<br>
            > <br>
            > Hi,<br>
            > <br>
            > (Apologies if you receive this twice. GMail classified
            the first one as <br>
            > spam)<br>
            > <br>
            > Aaron Ballman and I met by chance in Belfast and we
            discussed a way <br>
            > forward towards making AST Matchers easier to use,
            particularly for C++ <br>
            > developers who are not familiar with the details of the
            Clang AST.<br>
            > <br>
            > For those unaware, I expanded on this in the EuroLLVM
            conference this <br>
            > year, and then expanded on it at ACCU:<br>
            > <br>
            >   <a href="https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching" rel="noreferrer" target="_blank">https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching</a><br>
            > <br>
            > One step in the process of getting there is changing
            the default <br>
            > behavior of AST Matchers to ignore invisible nodes
            while matching using <br>
            > the C++ API, and while matching and dumping AST nodes
            in clang-query.<br>
            > <br>
            > I think this is the most important change in the entire
            proposal as it <br>
            > sets out the intention of making the AST Matchers
            easier to use for C++ <br>
            > developers who are not already familiar with Clang
            APIs.<br>
            > <br>
            > To that end, I've written an AST to motivate the
            change:<br>
            > <br>
            > <br>
            > <a href="https://docs.google.com/document/d/17Z6gAwwc3HoRXvsy0OdwU0X5MFQEuiGeSu3i6ICOB90" rel="noreferrer" target="_blank">https://docs.google.com/document/d/17Z6gAwwc3HoRXvsy0OdwU0X5MFQEuiGeSu3i6ICOB90</a>
            <br>
            > <br>
            > <br>
            > We're looking for feedback before pressing forward with
            the change. I <br>
            > already have some patches written to port clang-tidy
            and unit tests to <br>
            > account for the change of default.<br>
            <br>
            <br>
            This change is now in master.<br>
          </blockquote>
          <div><br>
          </div>
          <div>Is this change responsible for <a href="https://bugs.llvm.org/show_bug.cgi?id=46287" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=46287</a> ?
          </div>
        </div>
      </div>
    </blockquote>
    <p><br>
    </p>
    <p>Yes, I added a message on the bug and pointed out the <br>
    </p>
    <p>set traversal AsIs</p>
    <p>command in clang-query: <a href="https://godbolt.org/z/tIS622" target="_blank">https://godbolt.org/z/tIS622</a></p></div></blockquote><div><br></div><div>Given that we need to write matches with traverse(...) inside, I think we need to get traverse(...) into clang-query. </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>
    <p><br>
    </p>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_quote">
          <div>If so, I think the current behavior is very
            confusing, and seems harmful. By default, a cxxConstructExpr
            matcher no longer matches all CXXConstructExprs, which seems
            very strange and surprising, and likewise it's alarming that
            an AST dump from clang-query no longer correctly dumps the
            AST.</div>
        </div>
      </div>
    </blockquote>
    <p><br>
    </p>
    <p>The intention is to be make the AST Matchers more approachable by
      newcomers.  CXXConstructExpr and CXXMemberCallExpr nodes show up
      in places which are non-intuitive for people who are new to the
      clang AST, but are nevertheless familiar enough with C++ to work
      professionally with it. <br>
    </p>
    <p>Consider <br>
    </p>
    <p><a href="https://godbolt.org/z/TvUTgy" target="_blank">https://godbolt.org/z/TvUTgy</a></p>
    <p>and <br>
    </p>
    <p><a href="https://godbolt.org/z/me3EQu" target="_blank">https://godbolt.org/z/me3EQu</a></p>
    <p>and imagine you've never taken a compiler course, and never seen
      the clang AST before.<br>
    </p>
    <p>I am not surprised experts miss those nodes though, and the AsIs
      mode is essentially the "expert mode" while the default is
      intended to be "easy mode".<br>
    </p>
    <p><br>
    </p>
    <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">http://ce.steveire.com/z/9EF8x0</a> see my other posts on the
      topic for more)<br>
    </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.<br>
    </p>
    <p><br>
    </p>
    <p>Thanks,</p>
    <p>Stephen.<br>
    </p>
    <br>
  </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></div>