[clang-tools-extra] r194707 - Make clang's static analyzer checks available through clang-tidy.

[Alexander Kornienko]

On Tue, Jan 14, 2014 at 6:37 PM, Jordan Rose <jordan_rose at apple.com> wrote:

>
> On Jan 13, 2014, at 10:48 , Alexander Kornienko <alexfh at google.com> wrote:
>
> It seems that consuming either plist or HTML format is not a good option
> for us. I'd prefer to implement a custom PathDiagnosticConsumer, so that we
> have raw data in a form easily usable from the code (unlike plist/HTML in a
> file). Analyzer currently doesn't provide an interface to pass a custom
> PathDiagnosticConsumer, but I can implement this, if you have no
> objections. My idea was to make AnalysisConsumer, that is returned from
> clang::ento::CreateAnalysisConsumer(...), implement a new public interface,
> that provides a way to add PathConsumers.
>
>
> I...guess this could be okay. What we'd like to avoid is Yet Another
> Serialization Format—if you're going to be dealing with the output as data,
> we'd much prefer you just augment the plist format and use and improve the
> APIs in utils/analyzer/CmpRuns.py (a bit of a misnomer). But another
> consumer for immediate processing seems fair...so, what are you going to do
> with your new PathDiagnosticConsumer?
>

The first thing I'd like to use this for is retrieving the name of the
checker from the diagnostic message. In a longer term perspective we could
want to experiment with the HTML output format, and it would probably
require making the current HTML diagnostic consumer more flexible or
extracting some reusable part of it.


> There's one more thing, that is not directly supported by the current
> diagnostic system in the Static Analyzer: in clang-tidy we'd like to know
> the name of the checker producing each diagnostic message. PathDiagnostic
> has BugType and Category fields, which are both arbitrary human-readable
> strings, but we need to know the exact name of the checker in the form that
> can be used in the CheckersControlList option to enable/disable the
> specific checker. I have a sketch of a patch, that does this. The idea of
> the patch is to add a CheckerName field to the CheckerBase class, and set
> it in the CheckerManager::registerChecker() method, which will get them
> from the CheckerRegistry class through register.*Checker() functions.
> Before I polish the patch and sending for actual review, I wanted to check
> with you, if there are any concerns regarding this.
>
>
> We have deliberately resisted this, because there are two different names
> associated with checkers, and it's not the name of the class that matters.
> The class called MallocChecker, for example, implements unix.Malloc,
> unix.MismatchedDeallocator, alpha.unix.MallocWithAnnotations,
> cplusplus.NewDelete, and alpha.cplusplus.NewDeleteLeaks. If we want to
> start including those names in output, it has to be on a per-diagnostic
> basis...and the strings, at least, should be generated nicely from
> TableGen, so that we're not dealing with raw strings.
>

The idea was to pass names of checkers, under which they are registered.
These names come from the tablegen'd Checkers.inc file, and there's no need
for any raw strings. Anyway, I'll finish the patch soon and send it to you
for a review.


>
> I'd have to check with Ted but exposing just the name of the user-visible
> checker setting seems like a reasonable thing to do.
>

I hope he has no concerns regarding this idea.


>
> Jordan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140115/14cedde3/attachment.html>