[cfe-dev] -fdiagnostics-format= design review : added enum to patch

[Douglas Gregor]

On May 19, 2011, at 8:55 AM, Andrew Fish wrote:

>>  
>> I think you intended this:
>> +    Opts.Format = 1;
>> +    Opts.ShowColumn = DiagnosticOptions::Msvc;
>>  
>> To be:
>> +    Opts.Format = DiagnosticOptions::Msvc;
>> +    Opts.ShowColumn = false;
>>  
> 
> Yes, sorry about that. 
> 
> In DiagnosticOptions.h why is it
> 
> ShowColumn = 1;
> 
> and not
> 
> ShowColumn = true;
> 
> I was just trying to match existing style by using 0 and 1.

I have no idea :)

I would have used true/false, but it doesn't really matter.

>> At least in Visual Studio 2010 columns are supported with the following format:
>> t.c:(3,10)
>> I not entirely sure but I think you need ColNo-1 on MSVC
>>  
> 
> I can add -fshow-column support for msvc to the patch, but I don't have a copy of Visual Studio 2010 to test with. Can you test it with Visual Studio 2010 when I add it?

That'd be great; someone with Visual Studio 2010 can confirm the format or submit a patch to tweak it if needed.

>> Also, shouldn't there be tests for this?
>>  
> 
> Can you point me in the direction of how to write the tests, or even where the current tests are? 


The tests for diagnostic formatting are in test/Misc. A good example there is test/Misc/caret-diags-macros.c, which uses FileCheck to verify the output:

	http://llvm.org/cmds/FileCheck.html

	- Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110519/4007ca55/attachment.html>