[cfe-dev] Need help designing a new diagnostic formatting command line option for use with x86_64-pc-win32

[Douglas Gregor]

On May 18, 2011, at 12:33 PM, Andrew Fish wrote:

> On May 18, 2011, at 11:24 AM, Ted Kremenek wrote:
> 
>> Hi Andrew, Doug,
>> 
>> I'm wondering if support for Microsoft-style diagnostics should be implemented as a new DiagnosticClient (e.g., MSVCTextDiagnosticPrinter).  I'm not certain what the plan was, but putting this all into TextDiagnosticPrinter seems a little gross to me.  With a separate DiagnosticClient, we might not need extra bits in DiagnosticOptions.
> 
> I'm working on a patch to implement -fdiagnostic-format=xxx per Doug's suggestion, and will post the patch to mailing list for review shortly. 
> 
> I implemented for msvc and vi variants, here are the changes in TextDiagnosticPrinter.cpp for TextDiagnosticPrinter::HandleDiagnostic()
> 
>          if (DiagOpts->Format == 1) 
>           // msvc parsable format
>           OS << PLoc.getFilename() << '(' << LineNo << ") : ";
>         else if (DiagOpts->Format == 2) 
>           // vi command line format
>           OS << PLoc.getFilename() << " +" << LineNo << ':';
>         else 
>           // clang format
>           OS << PLoc.getFilename() << ':' << LineNo << ':';
>         if (DiagOpts->ShowColumn)
>           if (unsigned ColNo = PLoc.getColumn())
>             OS << ColNo << ':';

I'd rather that DiagOpts->Format use an enumeration type, so this can be a switch() on the format. Otherwise, this looks fine. 

> Where the original code was:
> 
>         // Emit a Visual Studio compatible line number syntax.
>         if (LangOpts && LangOpts->Microsoft) {
>           OS << PLoc.getFilename() << '(' << LineNo << ')';
>           OS << " : ";
>         } else {
>           OS << PLoc.getFilename() << ':' << LineNo << ':';
>           if (DiagOpts->ShowColumn)
>             if (unsigned ColNo = PLoc.getColumn())
>               OS << ColNo << ':';
>         }
> 
> 
> Seems like a bit of overkill to implement a new DiagnosticClient for one line of code? Maybe some method for abstracting filename, line number, and column printing would be better? Then again my brain thinks in C and not C++.

I agree that it would be overkill to create a new DiagnosticClient. Since the variation between the formats isn't so large now, this doesn't need to be abstracted further unless the code is repeated elsewhere.

	- Doug

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110518/cc41ce2a/attachment.html>