[PATCH] Change -fdiagnostics-format=msvc column number to match VS2012 and VS2013

[Reid Kleckner]

Can we key this off of -fmsc-version >= 1700 instead of adding a new flag?
 Either that, or remove the old behavior entirely.  If users have
off-by-one locations in their diagnostics, it seems unlikely to me that
they will realize they need to pass an extra special flag to clang.


On Wed, Mar 5, 2014 at 10:59 AM, Yunzhong Gao <
Yunzhong_Gao at playstation.sony.com> wrote:

> ygao added you to the CC list for the revision "Change
> -fdiagnostics-format=msvc column number to match VS2012 and VS2013".
>
> Hi,
> The Visual Studio IDE changed behavior in VS2012. It used to be the case
> that the clang diagnostic
> has to report a column number one less than the correct value in order for
> the IDE to move the cursor
> to the expected location. This behavior is changed in VS2012 and VS2013 so
> that the IDE is now
> expecting the column number to match the actual source location.
>
> Before:    source(line, column-1): type: message
> After:    source(line, column): type: message
>
> This patch changes -fdiagnostics-format=msvc to match the new VS2012 and
> VS2013, and also
> introduces a new -fdiagnostics-format=msvc2010 in case anyone needs the
> old behavior.
>
> Can someone take a look whether this patch is reasonable?
> - Gao
>
> http://llvm-reviews.chandlerc.com/D2949
>
> Files:
>   include/clang/Basic/DiagnosticOptions.h
>   lib/Frontend/CompilerInvocation.cpp
>   lib/Frontend/TextDiagnostic.cpp
>   test/Misc/diag-format.c
>
> Index: include/clang/Basic/DiagnosticOptions.h
> ===================================================================
> --- include/clang/Basic/DiagnosticOptions.h
> +++ include/clang/Basic/DiagnosticOptions.h
> @@ -27,7 +27,7 @@
>  /// \brief Options for controlling the compiler diagnostics engine.
>  class DiagnosticOptions : public RefCountedBase<DiagnosticOptions>{
>  public:
> -  enum TextDiagnosticFormat { Clang, Msvc, Vi };
> +  enum TextDiagnosticFormat { Clang, Msvc, Msvc2010, Vi };
>
>    // Default values.
>    enum { DefaultTabStop = 8, MaxTabStop = 100,
> Index: lib/Frontend/CompilerInvocation.cpp
> ===================================================================
> --- lib/Frontend/CompilerInvocation.cpp
> +++ lib/Frontend/CompilerInvocation.cpp
> @@ -602,6 +602,8 @@
>      Opts.setFormat(DiagnosticOptions::Clang);
>    else if (Format == "msvc")
>      Opts.setFormat(DiagnosticOptions::Msvc);
> +  else if (Format == "msvc2010")
> +    Opts.setFormat(DiagnosticOptions::Msvc2010);
>    else if (Format == "msvc-fallback") {
>      Opts.setFormat(DiagnosticOptions::Msvc);
>      Opts.CLFallbackMode = true;
> Index: lib/Frontend/TextDiagnostic.cpp
> ===================================================================
> --- lib/Frontend/TextDiagnostic.cpp
> +++ lib/Frontend/TextDiagnostic.cpp
> @@ -803,6 +803,7 @@
>    OS << PLoc.getFilename();
>    switch (DiagOpts->getFormat()) {
>    case DiagnosticOptions::Clang: OS << ':'  << LineNo; break;
> +  case DiagnosticOptions::Msvc2010:
>    case DiagnosticOptions::Msvc:  OS << '('  << LineNo; break;
>    case DiagnosticOptions::Vi:    OS << " +" << LineNo; break;
>    }
> @@ -810,16 +811,19 @@
>    if (DiagOpts->ShowColumn)
>      // Compute the column number.
>      if (unsigned ColNo = PLoc.getColumn()) {
> -      if (DiagOpts->getFormat() == DiagnosticOptions::Msvc) {
> +      if (DiagOpts->getFormat() == DiagnosticOptions::Msvc2010) {
>          OS << ',';
>          ColNo--;
> -      } else
> +      } else if (DiagOpts->getFormat() == DiagnosticOptions::Msvc)
> +        OS << ',';
> +      else
>          OS << ':';
>        OS << ColNo;
>      }
>    switch (DiagOpts->getFormat()) {
>    case DiagnosticOptions::Clang:
>    case DiagnosticOptions::Vi:    OS << ':';    break;
> +  case DiagnosticOptions::Msvc2010:
>    case DiagnosticOptions::Msvc:  OS << ") : "; break;
>    }
>
> Index: test/Misc/diag-format.c
> ===================================================================
> --- test/Misc/diag-format.c
> +++ test/Misc/diag-format.c
> @@ -2,12 +2,16 @@
>  // RUN: %clang -fsyntax-only -fdiagnostics-format=clang %s 2>&1 |
> FileCheck %s -check-prefix=DEFAULT
>  // RUN: %clang -fsyntax-only -fdiagnostics-format=clang -target
> x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
>  //
> +// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc2010  %s 2>&1 |
> FileCheck %s -check-prefix=MSVC2010
> +// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc2010 -target
> x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
> +// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc2010 -target
> x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
>  // RUN: %clang -fsyntax-only -fdiagnostics-format=msvc  %s 2>&1 |
> FileCheck %s -check-prefix=MSVC
>  // RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target
> x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC
>  // RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target
> x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s -check-prefix=MSVC
>  //
>  // RUN: %clang -fsyntax-only -fdiagnostics-format=vi    %s 2>&1 |
> FileCheck %s -check-prefix=VI
>  //
> +// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc2010
> -fno-show-column %s 2>&1 | FileCheck %s -check-prefix=MSVC_ORIG
>  // RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fno-show-column
> %s 2>&1 | FileCheck %s -check-prefix=MSVC_ORIG
>  //
>  // RUN: %clang -fsyntax-only -fno-show-column %s 2>&1 | FileCheck %s
> -check-prefix=NO_COLUMN
> @@ -26,10 +30,11 @@
>
>  #ifdef foo
>  #endif bad // extension!
> -// DEFAULT: {{.*}}:28:8: warning: extra tokens at end of #endif directive
> [-Wextra-tokens]
> -// MSVC: {{.*}}(28,7) : warning: extra tokens at end of #endif directive
> [-Wextra-tokens]
> -// VI: {{.*}} +28:8: warning: extra tokens at end of #endif directive
> [-Wextra-tokens]
> -// MSVC_ORIG: {{.*}}(28) : warning: extra tokens at end of #endif
> directive [-Wextra-tokens]
> -// NO_COLUMN: {{.*}}:28: warning: extra tokens at end of #endif directive
> [-Wextra-tokens]
> -// MSVC-FALLBACK: {{.*}}(28,7) : error(clang): extra tokens at end of
> #endif directive
> +// DEFAULT: {{.*}}:32:8: warning: extra tokens at end of #endif directive
> [-Wextra-tokens]
> +// MSVC2010: {{.*}}(32,7) : warning: extra tokens at end of #endif
> directive [-Wextra-tokens]
> +// MSVC: {{.*}}(32,8) : warning: extra tokens at end of #endif directive
> [-Wextra-tokens]
> +// VI: {{.*}} +32:8: warning: extra tokens at end of #endif directive
> [-Wextra-tokens]
> +// MSVC_ORIG: {{.*}}(32) : warning: extra tokens at end of #endif
> directive [-Wextra-tokens]
> +// NO_COLUMN: {{.*}}:32: warning: extra tokens at end of #endif directive
> [-Wextra-tokens]
> +// MSVC-FALLBACK: {{.*}}(32,8) : error(clang): extra tokens at end of
> #endif directive
>  int x;
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140305/77059239/attachment.html>