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

[Manuel Klimek via cfe-dev]

On Wed, Jun 17, 2020 at 11:21 PM Stephen Kelly via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

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

Given that we need to write matches with traverse(...) inside, I think we
need to get traverse(...) into clang-query.

>
>
> 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.
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200618/15f9726e/attachment-0001.html>