[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for the history, I wasn't aware this was such a big deal for such a long time!

In D158872#4625555 <https://reviews.llvm.org/D158872#4625555>, @aaron.ballman wrote:

>> A few ideas:
>>
>> - could we move these (and possibly other rarely-used matchers) to ObscureMatchers.h or something? It's definitely ugly, but maybe not as bad as missing them entirely.
>> - seems like we should extract an ASTMatchersBase.h or so to separate "matcher framework" from "all the slow matchers" so headers that expose matcher-based APIs don't have to transitively expose everything, reducing the cost of such changes
>> - (long shot) maybe we can work out which code patterns are particularly terrible for parse times and change them
>
> The problem is the pile of templates that is used to generate the DSL and used for dynamic matchers by clang-query. IIRC, we looked into reducing the overhead and eventually punted on it, some background can be found here: https://reviews.llvm.org/D84038 when we finally decided to enable /bigobj everywhere.

Thanks, I'll take a look and see if C++17 gives us new tools, but seems likely it's just too complicated.
Clearly the answer here is to force everyone to use the dynamic matchers and string literals. (Only half kidding - if we didn't try to express all constraints at compile time this would all be so much easier...)

> I know we did a pass to split ASTMatchers.h up so we now have ASTMatchersInternal.h, so I think we might have done about as much extraction as we can

The thing I think is possible is to have a header that defines `DeclMatcher`, `BoundNodes`, `anything` etc without including all the concrete matchers. This is the header that `ASTMatchFinder.h` should include.

(At the moment we have some insane include chains like `Analysis/FlowSensitive/DataflowAnalysis.h` => `MatchSwitch.h` (for `TransferState) => `ASTMatchers.h` (for `anything` etc). Thus the main header of an ~unrelated library exports all matcher types).

> Ideally, I would love to table generate more of the AST matchers ... Clang itself will still need to include all of those headers (and thus will still hit all the compile time and object size problems).

That would be great. For compile times, I'd expect we could include fewer matchers into fewer TUs - very little of clang should be using most of the matchers, and the sum of TU compile times is usually somewhat more important than the max.
I have less understanding of the object size issues. My understanding is that the current matchers are written ~entirely as templates in headers, the API surface is that way to achieve the decidedly un-C++-like DSL, but the implementation just because it's convenient. Fundamentally it doesn't seem like basic node-type matchers should have to instantiate many templates!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158872/new/

https://reviews.llvm.org/D158872