[PATCH] D135690: [ASTMatchers] Add matcher for functions that are effectively inline

Fri Oct 14 07:53:14 PDT 2022

aaron.ballman added a comment.

In D135690#3856863 <https://reviews.llvm.org/D135690#3856863>, @Trass3r wrote:

> In D135690#3852362 <https://reviews.llvm.org/D135690#3852362>, @aaron.ballman wrote:
>
>> what's the need for adding this matcher? Do you plan to use it for some in-tree needs? We usually only add new matchers where there's an immediate need for them because of how expensive AST matchers are to compile (and each matcher adds a fair number of template instantiations to the final binary as well, so there's a bit of runtime cost too).
>
> Didn't realize it has a big cost. Looking inside the `AST_MATCHER` and `REGISTER_MATCHER` macros I can't see any unique instantiations, should be memoized?

IIRC, the cost may depend on the compiler. I know we had to enable `/bigobj` when building with MSVC because each template instantiation here was being added to its own section in the object file and we'd wind up with tens of thousands of sections.

> I created it a while ago for use in a clang-tidy check. Oddly I can't find that code right now.
> It might have been for finding inline ctors/dtors: https://github.com/llvm/llvm-project/issues/51577.
> https://chromium.googlesource.com/chromium/src/tools/clang/+/refs/heads/main/plugins/FindBadConstructsConsumer.cpp#495

I think it might make more sense to use a local matcher if you need it only for one clang-tidy check. If we find we need it in more checks or there's a wider need for it, we can hoist it up to ASTMatchers.h so it's exposed more generally at that time. WDYT?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135690