[PATCH] Allow specifying a custom PathDiagnosticConsumer for use with the static analyzer.

[Jordan Rose]

On Jan 29, 2014, at 9:15 , Alexander Kornienko <alexfh at google.com> wrote:

>> Then again, why aren't we improving the existing data formats and the tools meant to consume them? (cf.
>> the now-inappropriately-named CmpRuns.py)
>> 
>> I'm consciously giving you a hard time because I've watched the plist output leave the HTML output in the
>> dust. scan-build on its own is much worse at reporting errors, to the point where we don't even show paths
>> that cross files because no one has put in the time to make that work in a nice way. I don't want to see
>> clang-tidy get all these nice features that don't translate into the plists or Xcode, and I don't want clang-tidy
>> to have to spend a ton of effort to get the existing features of the plists and Xcode. As much as possible, I
>> want to have one code path.
> 
>  I feel that we're talking about different things. I certainly support your ideas of feature parity in output formats and reusing the existing path diagnostic consumers. But clang-tidy is not only a tool, but also a library, and its "output format" is `struct ClangTidyError` (tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.h).
> 
>  We can try to extend clang DiagnosticEngine to pass additional information with each diagnostic (first, checker names, then maybe something else), but it fits bad in its architecture (which is mostly targeted at tablegen'd diagnostic IDs).
> 
>  Plist and HTML formatters are bad options, as we'd need to round-trip information through text format, which doesn't seem to be a good idea. We could reuse certain logic from other diagnostic consumers, if there are reasons to do this, but I don't see how we can end up having "one code path".
> 
>  If you have a good alternative idea on how we could achieve, for example, what I try to do in this patch + D2620, can you outline it?

Re: YAML: pretty much equivalent to the plists. I agree that it's a nicer wire format, but there's no extra power there, except that it's easier to parse.

I think I actually do like the idea of a tool getting to actually write a PathDiagnosticConsumer; the downside is that that consumer may have to be updated for changes in the analyzer. With the HTML format, the insulation is that it's meant to be consumed by humans; for the plist output, it's that it's easier to remain backwards-compatible, plus we have the CmpRuns tool (sort of). It definitely makes sense for a ClangTidy client to be able to get analyzer results without having to go to disk, but then ClangTidy itself is going to have to be that insulation layer. I guess that's what you were trying to tell me.

I'll admit I'm having trouble determining is what "struct ClangTidyError" expects to do with path diagnostics and checker names. Are you going to put the checker name in where the warning flag would have gone? That's kind of correct, but they're not actually the same. Do the path diagnostics become notes? That's kind of correct, but loses some semantic information, which is why the text and HTML outputs use the "minimal" paths and the plist-for-Xcode output uses "extensive" paths. (Also, if we ever add true notes to analyzer results, mixing the two might not make sense.)

But maybe I'm just worried about people doing this in general, and shouldn't worry about the specific case of clang-tidy, which will probably get these answers right and be able to evolve with the analyzer core. Still, once Pandora's Box is opened, it can't easily be closed again, and we'd really like external consumers of path diagnostics to build tools that consume a standard output format rather than needing a custom build of Clang to do it.

I'd like Ted to weigh in on this one too; unfortunately he's been quite busy lately...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140130/127bfcea/attachment.html>