[cfe-dev] Open sourcing a batch of checkers

Tue Dec 23 04:29:48 PST 2014

On Tue Dec 23 2014 at 12:07:28 PM Gábor Horváth <xazax.hun at gmail.com> wrote:

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

There is already a matcher 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?
>

Generally, I'd be curious what exactly you're trying to match that is more
naturally expressed by writing down a RAV yourself.

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

That is indeed an interesting question - I'd start a new top-level thread
discussing only this part; I know that some people had interest in seeing
plist based diagnostics available more generally, so I think there will be
support, but the solution must be carefully designed.

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

cfe-commits is the mailing list to submit patches to, and phabricator is a
way to submit patches to the mailing list :)

>
> 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/5b482112/attachment.html>