[cfe-dev] Open sourcing a batch of checkers

Tue Dec 23 03:07:28 PST 2014

Hi Alex!

I have a few more questions.

I did not see any checkers in clang tidy that are not ASTMatcher or
PPCallbacks based. We have a checker for local variables that could be
declared in a more narrow scope. It was more natural to implement that
checker using RecursiveASTVisitors. Should we add a matcher for example to
match function decls and clas the visitor on each function body or should
we add direct support for matchers that are consists of only Visitors but
no matchers?

The report from the Static Analyzer are consumed by the
AnalyzerDiagnosticConsumer which has all the information we needs. The
DiagnosticBuilder however does not support a structured way to store the
path sensitive information. To add plist output support we should be able
to pass those information to DiagnosticBuilder and we would like to reuse
as much code from the current PlistDiagnostics. Unfortunately the
implementation of PlistDiagnostics is not part of the public API. Is it ok
to make it public? What is the convention?

I think we will start to port the checkers to clang tidy and implement the
plist reporting in january. Where do you expect the patches to be
submitted? This mailing list, cfe-commits or the phabricator?

Thanks,
Gábor Horváth

On 19 December 2014 at 17:48, Alexander Kornienko <alexfh at google.com> wrote:

> Hi Gábor,
>
> On Fri, Dec 19, 2014 at 11:19 AM, Gábor Horváth <xazax.hun at gmail.com>
> wrote:
>>
>> Hi!
>>
>> I started to investigate how Clang Tidy works, and I find it very
>> promising. We are likely to convert our ASTMatcher/PPCallback/ASTVisitor
>> checks to clang tidy checkers and submit patches. We will submit our Static
>> Analyzer checkers as well. Not all of our checkers are style based, but we
>> will add fix-its where appropriate when doing the conversions.
>>
>> I have some questions about the Clang Tidy though:
>> - Is it possible to report issues in plist output format? Right now we
>> are consuming the plist output of the Static Analyzer. Once we converted
>> our codebase to use Clang Tidy we would like to continue to parse the plist
>> output. If it is not possible now, we also would like to develop and
>> contribute the plist reporting to Clang Tidy (in case you find it a worthy
>> addition).
>>
>
> Clang-tidy checks use Clang's DiagnosticsEngine to report warnings which
> limits the structure of information passed from checks to what
> DiagnosticsEngine supports. This is also valid for the Static Analyzer:
> clang-tidy gets its results as Clang diagnostics which probably leads to
> some loss of information already. However, if you're fine with the
> information that is already passed through clang-tidy (see the
> ClangTidyError structure in ClangTidyDiagnosticConsumer.h)
>
>
>
, then adding another output format should be easy.
>
- As far as I can see, one can specify key value pairs as configuration
>> options for checkers. Can these values passed to Static Analyzer checkers
>> as well? If not, we also would like to add this capability and contribute
>> it if you find it useful enough.
>>
>
> Currently configuration options are not passed to the Static Analyzer. If
> it supports this kind of configuration, it should be easy to forward
> options there.
>
>
>> - Is there any documentation related to the specific checkers?
>>
>
> Only in the class comments for each check.
>
>
>> What we do right now: there is a markdown file next to each of our
>> checker implementation files and we have a build option to generate a HTML
>> documentation that contains the user documentation of each checker (when to
>> use, guidelines to fix, code examples, limitations, rationales etc). I
>> think it might be useful to have similar system in clang tidy. What do you
>> think?
>>
>
> That would be awesome. I'm happy to review patches (but expect long review
> times, as I'm on vacation until Jan 2).
>
> Regards,
> Alex
>
>
>>
>> Thanks,
>> Gábor
>>
>>
>> On 18 December 2014 at 10:25, Manuel Klimek <klimek at google.com> wrote:
>>>
>>> Yep, the general idea is:
>>> 1. if it's using ASTMatchers or PPCallbacks, make it a clang-tidy check
>>> 2. if it's using the CFG / being path sensitive, make it a Static
>>> Analyzer checker
>>>
>>> Note that static analyzer checkers are integrated in clang-tidy
>>> automatically - usually style rule stuff is much easier expressed in
>>> clang-tidy though (and I don't think static analyzer checks are integrated
>>> well with fix-its (but I'm not sure), and style violations usually have
>>> easy fix-its).
>>>
>>> On Thu Dec 18 2014 at 1:40:05 AM Sean Silva <chisophugis at gmail.com>
>>> wrote:
>>>
>>>> +Manuel
>>>>
>>>> clang-tidy seems like a natural place for things that find "design
>>>> rule violations".
>>>>
>>>> -- Sean Silva
>>>>
>>>> On Wed, Dec 17, 2014 at 4:50 AM, Gábor Horváth <xazax.hun at gmail.com>
>>>> wrote:
>>>>>
>>>>> Hello everyone,
>>>>>
>>>>> I am an intern at a company where we developed several checkers based
>>>>> on the clang infrastructure. We have about 30 checkers that are either C++
>>>>> core language or STL related. We would like to contribute some of these
>>>>> checkers (together with the documentation and the test cases). The ones
>>>>> that are well tested and have good false positive rates.
>>>>>
>>>>>
>>>>> Most of the checkers were originally developed find certain design
>>>>> rule violations. One example of a design rule at this company is: „The
>>>>> emptiness of a container should be checked using the empty method instead
>>>>> of the size method. It is not guaranteed that size is a constant-time
>>>>> function, and it is generally more efficient and also shows clearer intent
>>>>> to use empty. Furthermore some containers may implement the empty method
>>>>> but not implement the size method. Using empty whenever possible makes it
>>>>> easier to switch to another container in the future.” We realized that,
>>>>> maybe such checkers could be useful for the clang community as well.
>>>>>
>>>>>
>>>>> Our tool right now is a clang plugin that runs custom frontend actions
>>>>> on the analyzed source code. The reason is that, some of the checkers are
>>>>> only consist of ASTMatchers, some of them using RecursiveASTVisitors, some
>>>>> of them are clang Static Analyzer checkers. We are wondering what would be
>>>>> the desired way to contribute those checkers back. Should we focus on
>>>>> migrating them to clang-tidy? Should we focus on transforming them to
>>>>> Static Analyzer checkers?
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Gábor Horváth
>>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20141223/9b174a6c/attachment.html>