[cfe-commits] [PATCH] Tests for formatter diagnostics + custom DiagnosticConsumer in API.

[Alexander Kornienko]

I've made a very simple FileCheck test, see r172407.


On Mon, Jan 14, 2013 at 2:02 PM, Manuel Klimek <klimek at google.com> wrote:

> On Mon, Jan 14, 2013 at 1:31 PM, Alexander Kornienko <alexfh at google.com>wrote:
>
>> On Mon, Jan 14, 2013 at 12:48 PM, Manuel Klimek <klimek at google.com>wrote:
>>
>>> Can't we put those tests into clang-tools-extra? After all, the
>>> diagnostic approach seems to be bound much more to the tool than the
>>> library...
>>>
>>
>> Yes, I can, if you think this is the most reasonable way to do it. But
>> the only diagnostic we have to date is implemented in the library, and is
>> tool-independent. So I'd better put tests closer to the library, rather
>> than the tool.
>>
>
> Let me explain my reasoning a little more:
> To me, clang-format is a tool that tries to format as much as possible. If
> we find an error in a place where we cannot format, our errors are going to
> be useless compared to errors a real compiler would output. Thus, I don't
> really care that much about our errors. To me they're more a help in
> debugging, and will basically completely lose their value once we don't
> fail parsing valid code any more.
> I also think that over time we'll take out more and more errors, as we're
> finding better heuristics on how to format broken code.
>
> So, in conclusion, I don't find our diagnostics important, so if we have a
> tests that verifies that diagnostics get output at all (which would be a
> test that needs to live with with the tool, because otherwise we can always
> regress the tool), and tests for all the cases we want to actually format
> (thus knowing they don't have errors), I think that I honestly don't care
> enough about the rest of the cases.
>
> So, from my side, select either approach and check it in. We can always
> revisit this later. I'd personally prefer a very simple FileCheck in the
> extra tools repo.
>
> Cheers,
> /Manuel
>
>
>>
>> On Mon, Jan 14, 2013 at 12:41 PM, Alexander Kornienko <alexfh at google.com>wrote:
>>>
>>>> On Mon, Jan 14, 2013 at 12:13 PM, Alexander Kornienko <
>>>> alexfh at google.com> wrote:
>>>>
>>>>> In this case I'd prefer to use VerifyDiagnosticConsumer, which is an
>>>>> implementation of clang's "-verify" option. Any concerns?
>>>>>
>>>>
>>>> I found a little problem with both FileCheck and "-verify" approaches:
>>>> they both require a stand-alone binary, which it resides in
>>>> clang-tools-extra.
>>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>> On Mon, Jan 14, 2013 at 11:26 AM, Daniel Jasper <djasper at google.com>wrote:
>>>>>
>>>>>> +1, seems like the better approach.
>>>>>>
>>>>>>
>>>>>> On Mon, Jan 14, 2013 at 11:18 AM, Manuel Klimek <klimek at google.com>wrote:
>>>>>>
>>>>>>>
>>>>>>>   This still seems like an awful lot of code for checking 2(!)
>>>>>>> diagnostics. I'd actually vote for using FileCheck.
>>>>>>>
>>>>>>> http://llvm-reviews.chandlerc.com/D290
>>>>>>>
>>>>>>> BRANCH
>>>>>>>   svn
>>>>>>>
>>>>>>> ARCANIST PROJECT
>>>>>>>   clang
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Alexander Kornienko | Software Engineer | alexfh at google.com | +49 151
>>>>> 221 77 957
>>>>> Google Germany GmbH | Dienerstr. 12 | 80331 München
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Alexander Kornienko | Software Engineer | alexfh at google.com | +49 151
>>>> 221 77 957
>>>> Google Germany GmbH | Dienerstr. 12 | 80331 München
>>>>
>>>
>>>
>>
>>
>> --
>> Alexander Kornienko | Software Engineer | alexfh at google.com | +49 151
>> 221 77 957
>> Google Germany GmbH | Dienerstr. 12 | 80331 München
>>
>
>


-- 
Alexander Kornienko | Software Engineer | alexfh at google.com | +49 151 221
77 957
Google Germany GmbH | Dienerstr. 12 | 80331 München
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130114/a76242e4/attachment.html>