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

[nobled]

...ping?

On Mon, Jan 30, 2012 at 4:31 PM, nobled <nobled at dreamwidth.org> wrote:
> (And the tests/ patch again...)
>
>> 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
>>>