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