Page MenuHomePhabricator

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

Authored by thakis on Dec 20 2018, 6:19 PM.

Details

Summary

The intent is to add the build file for clang/lib/StaticAnalyzer/Frontend; everything else is pulled in by that.

It's close to 200 TUs; I might want to default to clang_enable_static_analyzer=false when I add the clang/lib/FrontendTool build file.

Diff Detail

Repository
rL LLVM

Event Timeline

thakis created this revision.Dec 20 2018, 6:19 PM

Looks reasonable, what about linking with Z3? Or is your goal just to get a minimally working functionality?

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.

Thanks for taking a look!

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.)
  1. 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 :-)

Looks reasonable, what about linking with Z3? Or is your goal just to get a minimally working functionality?

Sorry, forgot to reply to this. I don't use this, so I feel someone who does want to use Z3 + GN should probably add support for this.

phosek accepted this revision.Dec 21 2018, 2:46 PM

LGTM

This revision is now accepted and ready to land.Dec 21 2018, 2:46 PM
This revision was automatically updated to reflect the committed changes.