[cfe-dev] PList output for clang Tidy

Fri Mar 27 04:50:03 PDT 2015

Hi,

I had some free time this week and I started to implement a prototype for
clang tidy plist output (patches are attached to this mail). In this first
attempt I tried to keep clang changes minimal. To achieve this, running
clang tidy generates two separate plist output: one for the reports that
are generated by the clang static analyzer and one for the reports
generated by tidy checkers. I try to make these two kinds of plists as
similar as possible. I did not add note and fixit information to the plists
yet and the plists which contains reports from tidy checkers do not contain
a file list. I choose this separate plist design because the
PathDiagnosticConsumers and DiagnosticConsumers have different life times.
Supporting one output would add too much extra complexity in my opinion
(unless the clang diagnostic engine learns to handle path diagnostics).

What is your opinion about the design? Should I continue the implementation
or should I change the design?

Regards,
Gábor

On 12 January 2015 at 16:10, Alexander Kornienko <alexfh at google.com> wrote:

> Hi Gábor,
>
> On Mon, Jan 5, 2015 at 11:21 AM, Gábor Horváth <xazax.hun at gmail.com>
> wrote:
>
>> On 5 January 2015 at 11:02, Manuel Klimek <klimek at google.com> wrote:
>>
>>> +chandler and daniel; I think that are all the chefs we need for this
>>> particular porridge
>>>
>>> On Mon Jan 05 2015 at 10:05:02 AM Gábor Horváth <xazax.hun at gmail.com>
>>> wrote:
>>>
>>>> Hello everyone,
>>>>
>>>> The Clang Static analyzer can output the diagnostics in plist format.
>>>> It is a useful feature, because it is easy to parse that format with 3rd
>>>> party tools, hence integrating clang tools with others.
>>>>
>>>> Unfortunately the plist reporting format is not supported by Clang
>>>> Tidy. We would like to add plist support to it. This involves a lot of
>>>> changes both to the format and the public API, so I want your opinion, how
>>>> to do it.
>>>>
>>>> In my opinion we need to extend the plist format to:
>>>> * Support notes that are not events
>>>> * Support fixits
>>>>
>>>> What do you think, what would be the bast way to extend the format with
>>>> those informations?
>>>>
>>>
> Ted or Anna can say whether it's possible to extend the plist Static
> Analyzer output format and what's the best way to do this. Also, as you
> noted, most of PlistDiagnostics should be factored out and exposed in a
> form of some API to be reusable. This can be done completely independent of
> the ClangTidy part of the changes. Ted, Anna and Jordan should be the right
> people to send relevant patches to.
>
>
>> The plist reporting related functionality is not part of the Clang public
>>>> API at the moment. The best would be, if the Static Analyzer and regular
>>>> diagnostics could be reported to the same plist output file. To achieve
>>>> this, the diagnostic consumer that outputs to the plist should support both
>>>> PathDiagnostics and regular Diagnostics. It would be redundant to
>>>> reimplement the whole functionality in Clang Tidy. To reduce the
>>>> redundancy, we would like to refactor several plist related helper
>>>> functions out from PlistDiagnostics and make it available in public
>>>> headers. We would also like to make PlistDiagnostics class available
>>>> so we can inherit from it. What do you think, what would be the best way to
>>>> organize these changes?
>>>>
>>>
>>> If I remember correctly, Chandler has long ago proposed to merge the
>>> different diagnostic types we have into one central clang diagnostic type
>>> that supports all our use cases.
>>>
>>
> Chandler, do you have any thoughts on this?
>
>
>> I personally would need to see a CL to judge whether it makes sense, but
>>> generally, I think what you say sounds like it's the right direction (if
>>> you agree with the sentence above: make clang's basic diagnostic system
>>> powerful enough to support the analyzer's use cases, and then switch the
>>> analyzer and clang-tidy to use it).
>>>
>>>
>> In my proposal it would be possible to create a Diag Consumer that can
>> consume both PathDiagnostics and regular Diagnostics. It would be possible,
>> to make the same object to consume both Clang and Static Analyzer diags.
>>
>
> The problem is not consuming messages from the Static Analyzer in a
> different format (ClangTidy already subscribes to PathDiagnostics), but the
> fact that ClangTidy uses Clang DiagnosticsEngine internally to handle
> diagnostics from the Static Analyzer, from all checks, and from the
> compiler. It means that to handle all information from PathDiagnostic we'd
> need to either extend Clang's diagnostic subsystem or rewrite a significant
> part of ClangTidy to introduce an alternative way to handle PathDiagnostics
> inside ClangTidy. It seems to me that the former is a much superior
> solution.
>
> In any case we'd also need to extend the ClangTidyError structure (used to
> return ClangTidy results to clients) to accommodate additional information
> from PathDiagnostic.
>
> It may not be as clean as merging the two kinds of diagnostics, but I
>> suspect this approach is faster to implement. In the long term I do agree
>> that, it would be desirable to use the same mechanisms for both Static
>> Analyzer and Clang. But I am not sure that, I have enough time for a
>> refactoring like that.
>>
>>
>>>
>>> I'd vote for renaming PList to something non-horrible in the process, (I
>>> still have no idea what the "P" stands for), but that's bikeshedding.
>>>
>>
>> I don't know if we could rename that. PList is not Clang specific, Apple
>> uses this format in other projects as well.
>>
>
>>
>>>
>>> Cheers,
>>> /Manuel
>>>
>>>
>>
>> Thanks,
>> Gábor
>>
>
>
> Regards,
> Alex
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150327/d8fddc07/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: TidyPlist_ClangPart.patch
Type: text/x-patch
Size: 3294 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150327/d8fddc07/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: TidyPlist.patch
Type: text/x-patch
Size: 16079 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150327/d8fddc07/attachment-0001.bin>