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

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 29 11:58:50 PDT 2023


sammccall added a comment.

I did some testing on my machine (host compiler clang-14), and I'm not sure the specifics justify the level of caution here:

ASTMatchers.h parses in 6.5s (with `clang-check`)
If I comment out the code + internal ASTMatcher includes, then just the deps (mostly AST headers) parses in 4.9s.
1.6s for matchers impl is slow but not build-breakingly so. I don't think we should be scared of the marginal parse time increase from this change. (ASTMatchers is not in all that many TUs anyhow - no "core" headers re-export it).

The bug referenced is about object size. The connection with matchers seems to be that matcher tests had their section limit increased, before this applied to the whole project.
However this is specifically for tests that test most of the matchers, you only pay for what you use.
I verified that a cpp file that includes `ASTMatchers.h` and does nothing else generates a trivial-sized object file (<1kb). So again, I don't think we need to fear this change: if it's used in many places, it's justified, and if it's not used, it's not expensive.

(If compilation of files like `NodeMatchersTest.cpp` are significant build bottlenecks, we'd need to address that, but *those* would be fine to split up in unprincipled ways, so I'm not too concerned)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158872



More information about the cfe-commits mailing list