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

[nobled]

On Sat, Jan 28, 2012 at 12:22 AM, Jordy Rose <jediknil at belkadan.com> wrote:
> 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.
Yeah, I figured I would add that method after this series so the
functionality would become available again.

>
> 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.
Ah, you're right. I tested it out with my patch, and running this:

$ bin/clang -cc1 -plugin analyzer -plugin-arg-analyzer -analyzer-checker-help

will print the help, then hold, waiting for the input file on STDIN.

However, current behavior is broken in its own way already, as far as
I can tell:

$ clang --analyze -Xanalyzer -analyzer-checker-help
clang-3: error: no input files
$ clang -cc1 -analyze -analyzer-checker-help || echo returned nonzero!
<the normal -analyzer-checker-help spew>
returned nonzero!

...So if I wanted, I can just preserve the current behavior by
returning 'false' from ParseArgs, indicating failure, without actually
setting a diagnostic. Which is what the current
return-zero(false)-from-ExecuteCompilerInvocation code does,
effectively.
>
>
> +  // 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.)
Ah, of course. Done in BeginInvocation now. (I see that you mentioned
it before, but I got stuck on that not-per-file idea, doh.) There's
one small peculiarity with using addMacroDef() (the define gets listed
alongside all the driver/user-specified -D commandline options in the
preprocessed file instead of with the builtins), but it's not one that
should really matter.

>
>
> +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.
Yep, I had the same thought.
>
> 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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0016-StaticAnalyzer-factor-into-an-AST-plugin.patch
Type: text/x-patch
Size: 80479 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120130/2f7fa8cd/attachment.bin>