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

[Alexander Kornienko]

On Thu, Jan 30, 2014 at 7:27 PM, Jordan Rose <jordan_rose at apple.com> wrote:

>
> 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.
>

It should be easy enough for in-tree consumers. Plus it's more convenient
to track usages, when type information is available, than when we lose it
while round-tripping the data through a wire format.


> 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.
>

Yes, clang-tidy is going to be some sort of an insulation layer, but that
shouldn't be a problem due to the reasons stated above.


> I'll admit I'm having trouble determining is what "struct ClangTidyError"
> expects to do with path diagnostics and checker names.
>

Path diagnostics are converted to notes, and checker names are just stored.
Clang-tidy binary prints checker names the same way, as warning names in
clang diagnostics, which totally makes sense, as they can be used to
enable/disable checkers.


> 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.
>

The difference between a tool statically linked with clang and a standalone
tool consuming analyzer's output in some format is huge, and I don't think
sane people would choose the former, if they only need to consumer
analyzer's output in some form.


>
> 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/20140131/505ac025/attachment.html>