NoQ added a comment.

In D138329#4027490 <https://reviews.llvm.org/D138329#4027490>, @thakis wrote:

> The `clang` binary should not depend on `ASTMatchers`.
>
> I see this isn't new in this patch, but `ASTMatchers` should only be used 
> from clang-tidy, not from the compiler itself.

ASTMatchers have been baked into clang binary for at least a few years already, 
actively used by the static analyzer (some backstory in D25429 
<https://reviews.llvm.org/D25429>). This warning is, however, probably the 
first use in clang proper, so they'll be there even if static analyzer support 
is turned off through cmake flags.

I'm open to a broader discussion. To me it's natural that there exist 
"analysis-based warnings" that take advantage of advanced analysis tools, and 
ASTMatchers is just one such tool, definitely not the most expensive one and 
not the most hazardous one (CFG and flow-sensitive analysis are arguably much 
scarier in both regards, and they're actively used in essential warnings such 
as `-Wuninitialized`).

With respect to this warning, we're also paying a lot of attention to make sure 
that performance is acceptable for the compiler. This patch is actually an 
example of our commitment to performance as it flattens the performance of this 
analysis down to linear time over AST size, something that wasn't previously 
possible with ASTMatchers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138329

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

Reply via email to