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

[Stephen Kelly via cfe-dev]

On 16/06/2020 05:16, Richard Smith wrote:
> On Mon, 25 May 2020 at 14:56, Stephen Kelly via cfe-dev 
> <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
>
>     On 20/12/2019 21:01, Stephen Kelly via cfe-dev wrote:
>     >
>     > Hi,
>     >
>     > (Apologies if you receive this twice. GMail classified the first
>     one as
>     > spam)
>     >
>     > Aaron Ballman and I met by chance in Belfast and we discussed a way
>     > forward towards making AST Matchers easier to use, particularly
>     for C++
>     > developers who are not familiar with the details of the Clang AST.
>     >
>     > For those unaware, I expanded on this in the EuroLLVM conference
>     this
>     > year, and then expanded on it at ACCU:
>     >
>     > https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching
>     >
>     > One step in the process of getting there is changing the default
>     > behavior of AST Matchers to ignore invisible nodes while
>     matching using
>     > the C++ API, and while matching and dumping AST nodes in
>     clang-query.
>     >
>     > I think this is the most important change in the entire proposal
>     as it
>     > sets out the intention of making the AST Matchers easier to use
>     for C++
>     > developers who are not already familiar with Clang APIs.
>     >
>     > To that end, I've written an AST to motivate the change:
>     >
>     >
>     >
>     https://docs.google.com/document/d/17Z6gAwwc3HoRXvsy0OdwU0X5MFQEuiGeSu3i6ICOB90
>
>     >
>     >
>     > We're looking for feedback before pressing forward with the
>     change. I
>     > already have some patches written to port clang-tidy and unit
>     tests to
>     > account for the change of default.
>
>
>     This change is now in master.
>
>
> Is this change responsible for 
> https://bugs.llvm.org/show_bug.cgi?id=46287 ?


Yes, I added a message on the bug and pointed out the

set traversal AsIs

command in clang-query: https://godbolt.org/z/tIS622


> 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.


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.

Consider

https://godbolt.org/z/TvUTgy

and

https://godbolt.org/z/me3EQu

and imagine you've never taken a compiler course, and never seen the 
clang AST before.

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".


> 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?


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.

(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: 
http://ce.steveire.com/z/9EF8x0 see my other posts on the topic for more)

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.


Thanks,

Stephen.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200617/95cf5a2b/attachment.html>