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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to