[PATCH] D55978: [gn build] Add build files for clang/lib/{ASTMatchers, CrossTU}, clang/lib/StaticAnalyzer/{Checkers, Core, Frontend}

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks for taking a look!

In D55978#1338654 <https://reviews.llvm.org/D55978#1338654>, @george.karpenkov wrote:

> > I might want to default to clang_enable_static_analyzer=false when I add the clang/lib/FrontendTool build file.
>
> I don't think that quite makes sense, since by default clang does have the analyzer built in.


Two points about this:

1. The GN build's main point is developer productivity (see llvm/utils/gn/README.rst). My guess is that most people don't work on the static analyzer, so if turning it off might make sense for that reason. (If the GN build gets adopted by people working on the static analyzer and less so by other people, this wouldn't make sense of course.)

2. I believe there's general agreement that if clang-tidy had existed when the static analyzer was first written, it would've been in clang-tidy, not in clang. Because of that, I think it might make sense to eventually move to a world where by default the analyzer is in clang-tidy but not in clang (but still have toggles to change that), and where the checker tests run through clang-tidy. (I don't mean to propose this at the moment, and it has some dependencies: ARCMigrate depends on the static analyzer and I think ARCMigrate isn't hooked up in clang-tidy at all at the moment. But at some point in the future I think this would make sense) The case for this is is imho stronger after r284112 which added ASTMatchers to the Checkers deps; historically we've tried to keep ASTMatchers out of the clang binary.

In any case, at the moment clang_enable_static_analyzer does default to true in the GN build. If I'm getting serious about changing that default, I'll cc you on that patch :-)


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

https://reviews.llvm.org/D55978