[cfe-dev] [RFC PATCH] making the static analyzer a PluginASTAction

[Jordy Rose]

A few additions to what I said before (some of which probably still need to be changed eventually):

AnalysisAction.h:
+  void toArgs(std::vector<std::string> &args);

Had a thought: since this isn't hooked up right now in CompilerInvocation anyway, why not make it a virtual method on PluginASTAction? Eventually we'll probably want to be able to serialize any plugin invocation anyway.

Or, the flip side: CompilerInvocation's FrontendOptsToArgs already blindly serializes all plugin options, so maybe we can just kill this.


FrontendActions.h:
+extern "C" void clang_LinkInStaticAnalyzerPlugin(void);

Yuck...but as far as I see, unavoidable in some form. This is a lot less brittle than relying on compiler options to make sure libClangStaticAnalyzer doesn't get left out.


FrontendActions.cpp:
+  // If -analyzer-checker-help was given, this should be a no-op.
+  if (Opts.ShowCheckerHelp)
+    return new ASTConsumer();
+

This isn't really a no-op, since it means the compiler will still go through the whole parsing phase to build an AST. I *think* it's safe to return NULL here, but I haven't tried it.


+  // TODO: CreateASTConsumer() is far too late to add a macro, and
+  // ParseArgs() is far too early for getCompilerInstance() to work yet,
+  // so we have to switch to a non-const CompilerInstance argument here
+  // in order to remove the plugin-specific hack from InitPreprocessor.cpp.
+  //CI.getPreprocessorOpts().addMacroDef("__clang_analyzer__");

How about BeginInvocation? Or BeginSourceFileAction? FrontendAction has those hooks specifically for this sort of thing. I haven't /tried/ it, but it's before the Preprocessor is created, and after the CompilerInstance is initialized with a file. Yes, it means we add the option for every file, but it's not like InitPreprocessor isn't doing the same work now.

(Sorry, the fixme here is really bugging me.)


+static void AnalyzerOptsToArgs(const AnalyzerOptions &Opts,
+                               std::vector<std::string> &Res) {
+  if (Opts.ShowCheckerHelp)
+    Res.push_back("-analyzer-checker-help");

It's not so important (since this isn't ever called right now), but this function isn't correct anymore -- all of these options would need "-plugin-arg-analyzer" in front. But again, since plugin args are already blindly serialized by CompilerInvocation, fixing this function may not be worth it.

(Also, there's probably no reason anymore to have this outside of AnalysisAction::toArgs.)


Longer-term (can fix later): once all the analyzer plugin args go through -plugin-arg-analyzer, we can strip the 'analyzer' prefix off each option.

I'm really happy this is happening. It'll help dogfood better plugin support, I hope. Not to mention we (read: Ted) might be able to publish updates to the analyzer more frequently than Clang updates, at least in theory.

(In practice, the analyzer relies so much on libClangAST that this probably isn't feasible. Not yet, anyway.)

Thanks again, nobled!
Jordy


On Jan 24, 2012, at 4:08, nobled wrote:

> Attached an updated patch--it had to be rebased against recent changes
> in the Driver/Tools.cpp code.
> 
> Also available as broken-down discrete steps on this git branch:
> https://github.com/nobled/clang/tree/anal-plugin