<div dir="ltr">Thanks. I was trying to be consistent with how the prefixes were used in this particular file, where the prefix was modeled after the case-sensitive flags being tested.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 29, 2016 at 8:49 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><div><div class="h5"><div dir="ltr">On Thu, Aug 25, 2016 at 11:32 AM Adrian McCarthy via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: amccarth<br>
Date: Thu Aug 25 13:24:35 2016<br>
New Revision: 279765<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=279765&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=279765&view=rev</a><br>
Log:<br>
Omit column info for CodeView by default<br>
<br>
Clang tracks only start columns, not start-end ranges. CodeView allows for that, but the VS debugger doesn't handle anything less than a complete range well--it either highlights the wrong part of a statement or truncates source lines in the assembly view. It's better to have no column information at all.<br>
<br>
So by default, we'll omit the column information for CodeView targeting Windows.<br>
<br>
Since the column info is still useful for sanitizers, I've promoted -gcolumn-info (and -gno-column-info) to a CoreOption and added a couple tests to make sure that works for clang-cl.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D23720" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D23720</a><br>
<br>
Modified:<br>
cfe/trunk/include/clang/<wbr>Driver/Options.td<br>
cfe/trunk/lib/Driver/Tools.cpp<br>
cfe/trunk/test/Driver/cl-<wbr>options.c<br>
<br>
Modified: cfe/trunk/include/clang/<wbr>Driver/Options.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=279765&r1=279764&r2=279765&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/Driver/Options.td?rev=<wbr>279765&r1=279764&r2=279765&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/<wbr>Driver/Options.td (original)<br>
+++ cfe/trunk/include/clang/<wbr>Driver/Options.td Thu Aug 25 13:24:35 2016<br>
@@ -1305,8 +1305,8 @@ def gno_record_gcc_switches : Flag<["-"]<br>
Group<g_flags_Group>;<br>
def gstrict_dwarf : Flag<["-"], "gstrict-dwarf">, Group<g_flags_Group>;<br>
def gno_strict_dwarf : Flag<["-"], "gno-strict-dwarf">, Group<g_flags_Group>;<br>
-def gcolumn_info : Flag<["-"], "gcolumn-info">, Group<g_flags_Group>;<br>
-def gno_column_info : Flag<["-"], "gno-column-info">, Group<g_flags_Group>;<br>
+def gcolumn_info : Flag<["-"], "gcolumn-info">, Group<g_flags_Group>, Flags<[CoreOption]>;<br>
+def gno_column_info : Flag<["-"], "gno-column-info">, Group<g_flags_Group>, Flags<[CoreOption]>;<br>
def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group<g_flags_Group>;<br>
def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group<g_flags_Group>;<br>
def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group<g_flags_Group>;<br>
<br>
Modified: cfe/trunk/lib/Driver/Tools.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=279765&r1=279764&r2=279765&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Driver/<wbr>Tools.cpp?rev=279765&r1=<wbr>279764&r2=279765&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Driver/Tools.cpp (original)<br>
+++ cfe/trunk/lib/Driver/Tools.cpp Thu Aug 25 13:24:35 2016<br>
@@ -2574,7 +2574,7 @@ static void getTargetFeatures(const Tool<br>
case llvm::Triple::wasm32:<br>
case llvm::Triple::wasm64:<br>
getWebAssemblyTargetFeatures(<wbr>Args, Features);<br>
- break;<br>
+ break;<br>
case llvm::Triple::sparc:<br>
case llvm::Triple::sparcel:<br>
case llvm::Triple::sparcv9:<br>
@@ -4656,9 +4656,13 @@ void Clang::ConstructJob(<wbr>Compilation &C,<br>
// We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now.<br>
Args.ClaimAllArgs(options::<wbr>OPT_g_flags_Group);<br>
<br>
- // PS4 defaults to no column info<br>
+ // Column info is included by default for everything except PS4 and CodeView.<br>
+ // Clang doesn't track end columns, just starting columns, which, in theory,<br>
+ // is fine for CodeView (and PDB). In practice, however, the Microsoft<br>
+ // debuggers don't handle missing end columns well, so it's better not to<br>
+ // include any column info.<br>
if (Args.hasFlag(options::OPT_<wbr>gcolumn_info, options::OPT_gno_column_info,<br>
- /*Default=*/ !IsPS4CPU))<br>
+ /*Default=*/ !IsPS4CPU && !(IsWindowsMSVC && EmitCodeView)))<br>
CmdArgs.push_back("-dwarf-<wbr>column-info");<br>
<br>
// FIXME: Move backend command line options to the module.<br>
@@ -6712,7 +6716,7 @@ void ClangAs::ConstructJob(<wbr>Compilation &<br>
case llvm::Triple::mips64el:<br>
AddMIPSTargetArgs(Args, CmdArgs);<br>
break;<br>
-<br>
+<br>
case llvm::Triple::x86:<br>
case llvm::Triple::x86_64:<br>
AddX86TargetArgs(Args, CmdArgs);<br>
<br>
Modified: cfe/trunk/test/Driver/cl-<wbr>options.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=279765&r1=279764&r2=279765&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/Driver/<wbr>cl-options.c?rev=279765&r1=<wbr>279764&r2=279765&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Driver/cl-<wbr>options.c (original)<br>
+++ cfe/trunk/test/Driver/cl-<wbr>options.c Thu Aug 25 13:24:35 2016<br>
@@ -50,6 +50,15 @@<br>
// fpstrict-NOT: -menable-unsafe-fp-math<br>
// fpstrict-NOT: -ffast-math<br>
<br>
+// RUN: %clang_cl /Z7 -gcolumn-info -### -- %s 2>&1 | FileCheck -check-prefix=gcolumn %s<br>
+// gcolumn: -dwarf-column-info<br>
+<br>
+// RUN: %clang_cl /Z7 -gno-column-info -### -- %s 2>&1 | FileCheck -check-prefix=gnocolumn %s<br></blockquote></div></div><div><br>We pretty consistently use uppercase names for the filecheck prefixes - and you can use something shorter if you like, since these are used in such short scopes (NOCOL, COL, etc)<br> </div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+// gnocolumn-NOT: -dwarf-column-info<br>
+<br>
+// RUN: %clang_cl /Z7 -### -- %s 2>&1 | FileCheck -check-prefix=gdefcolumn %s<br>
+// gdefcolumn-NOT: -dwarf-column-info<br>
+<br>
// RUN: %clang_cl /GA -### -- %s 2>&1 | FileCheck -check-prefix=GA %s<br>
// GA: -ftls-model=local-exec<br>
<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></span></div></div>
</blockquote></div><br></div>