<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 11, 2016 at 8:42 AM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</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"><div class="gmail_extra"><div class="gmail_quote"><span class="">On Mon, Jul 11, 2016 at 11:36 AM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@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><div class="gmail_extra"><br><div class="gmail_quote"><span>On Mon, Jul 11, 2016 at 7:18 AM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>VS2013's cl.exe doesn't understand /Zd, 2015's doesn't either. This means people who want to ask clang-cl for line tables only will have to add this flag in some if(is_clang) block in their build file anyways. What's the advantage of giving this flag a spelling that's different from both cl and clang? With -gline-tables-only, an if(is_clang) works on Linux, Mac, Windows.<br></div><div><br></div><div>(Even if there's a good case for /Zd, I don't think we should remove user-exposed flags without a strong reason, so even if we keep /Zd I think we should also keep exposing -gline-tables-only.)</div></div></blockquote><div><br></div></span><div>Existing users of -gline-tables-only? I'd imagine any responsible users of -gline-tables-only would probably use their build system to verify that the flag exists. We have never released an official LLVM which supported it (LLVM 3.8 came out in early March and -gline-tables-only was exposed via clang-cl in mid March).<br></div></div></div></div></blockquote><div><br></div></span><div>Ok, but why is /Zd better than -gline-tables-only?</div></div></div></div></blockquote><div><br></div><div>I see a few reasons:</div><div>- It is less surprising for a debug flag in the cl world to be called /Zsomething instead of -gsomething.</div><div>- I think that avoiding invasion of their namespace, when possible, is a good thing. It makes it less likely that we will conflict with a flag they want to add in the future.</div><div>- I imagine that if they wanted to add support back for -gline-tables-only, they'd name it /Zd.</div><div><br></div><div>I'm sympathetic to the argument that "-gline-tables-only" is more familiar to Linux/Mac OS X folks and that /Zd won't work across all platforms.</div><div><br></div><div>Here why I think that's ok:</div><div>- This is not really a new problem. If you want to select c++1z you get to spell it /std:c++latest for clang-cl and -std=c++14.</div><div>- People who want harmony between Mac OS X, Linux and Windows on the driver side can just use clang++.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div><br></div><div>I'd consider it in poor form for users to take a hard dependency on a flag which only exists in a compiler which has never been released.</div><div><br></div><div>I'd agree with you if -gline-tables-only had made its way to an actual release.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 11, 2016 at 10:08 AM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">This breaks existing users of -gline-tables-only. What's the motivation for this change?<span style="color:rgb(34,34,34)"> </span></div></blockquote></div></div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div><div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Jul 9, 2016 at 5:49 PM, David Majnemer via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Author: majnemer<br>
Date: Sat Jul 9 16:49:16 2016<br>
New Revision: 274991<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=274991&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=274991&view=rev</a><br>
Log:<br>
[clang-cl] Add support for /Zd<br>
<br>
MASM (ML.exe and ML64.exe) and older versions of MSVC (CL.exe) support a<br>
flag called /Zd which is more-or-less -gline-tables-only.<br>
<br>
It seems nicer to support this flag instead of exposing<br>
-gline-tables-only.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Driver/CLCompatOptions.td<br>
cfe/trunk/include/clang/Driver/Options.td<br>
cfe/trunk/lib/Driver/Tools.cpp<br>
cfe/trunk/test/Driver/cl-options.c<br>
<br>
Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=274991&r1=274990&r2=274991&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=274991&r1=274990&r2=274991&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)<br>
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Sat Jul 9 16:49:16 2016<br>
@@ -166,6 +166,8 @@ def _SLASH_Zc_trigraphs_off : CLFlag<"Zc<br>
HelpText<"Disable trigraphs (default)">, Alias<fno_trigraphs>;<br>
def _SLASH_Z7 : CLFlag<"Z7">,<br>
HelpText<"Enable CodeView debug information in object files">;<br>
+def _SLASH_Zd : CLFlag<"Zd">,<br>
+ HelpText<"Emit debug line number tables only">;<br>
def _SLASH_Zi : CLFlag<"Zi">, Alias<_SLASH_Z7>,<br>
HelpText<"Alias for /Z7. Does not produce PDBs.">;<br>
def _SLASH_Zp : CLJoined<"Zp">,<br>
<br>
Modified: cfe/trunk/include/clang/Driver/Options.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=274991&r1=274990&r2=274991&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=274991&r1=274990&r2=274991&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Driver/Options.td (original)<br>
+++ cfe/trunk/include/clang/Driver/Options.td Sat Jul 9 16:49:16 2016<br>
@@ -1229,7 +1229,7 @@ def fdebug_prefix_map_EQ<br>
def g_Flag : Flag<["-"], "g">, Group<g_Group>,<br>
HelpText<"Generate source-level debug information">;<br>
def gline_tables_only : Flag<["-"], "gline-tables-only">, Group<gN_Group>,<br>
- Flags<[CoreOption]>, HelpText<"Emit debug line number tables only">;<br>
+ HelpText<"Emit debug line number tables only">;<br>
def gmlt : Flag<["-"], "gmlt">, Alias<gline_tables_only>;<br>
def g0 : Flag<["-"], "g0">, Group<gN_Group>;<br>
def g1 : Flag<["-"], "g1">, Group<gN_Group>, Alias<gline_tables_only>;<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=274991&r1=274990&r2=274991&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=274991&r1=274990&r2=274991&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Driver/Tools.cpp (original)<br>
+++ cfe/trunk/lib/Driver/Tools.cpp Sat Jul 9 16:49:16 2016<br>
@@ -6269,12 +6269,18 @@ void Clang::AddClangCLArgs(const ArgList<br>
CmdArgs.push_back(Args.MakeArgString(Twine(LangOptions::SSPStrong)));<br>
}<br>
<br>
- // Emit CodeView if -Z7 is present.<br>
- *EmitCodeView = Args.hasArg(options::OPT__SLASH_Z7);<br>
- if (*EmitCodeView)<br>
- *DebugInfoKind = codegenoptions::LimitedDebugInfo;<br>
- if (*EmitCodeView)<br>
+ // Emit CodeView if -Z7 or -Zd are present.<br>
+ if (Arg *DebugInfoArg =<br>
+ Args.getLastArg(options::OPT__SLASH_Z7, options::OPT__SLASH_Zd)) {<br>
+ *EmitCodeView = true;<br>
+ if (DebugInfoArg->getOption().matches(options::OPT__SLASH_Z7))<br>
+ *DebugInfoKind = codegenoptions::LimitedDebugInfo;<br>
+ else<br>
+ *DebugInfoKind = codegenoptions::DebugLineTablesOnly;<br>
CmdArgs.push_back("-gcodeview");<br>
+ } else {<br>
+ *EmitCodeView = false;<br>
+ }<br>
<br>
const Driver &D = getToolChain().getDriver();<br>
EHFlags EH = parseClangCLEHFlags(D, Args);<br>
@@ -9964,7 +9970,8 @@ void visualstudio::Linker::ConstructJob(<br>
<br>
CmdArgs.push_back("-nologo");<br>
<br>
- if (Args.hasArg(options::OPT_g_Group, options::OPT__SLASH_Z7))<br>
+ if (Args.hasArg(options::OPT_g_Group, options::OPT__SLASH_Z7,<br>
+ options::OPT__SLASH_Zd))<br>
CmdArgs.push_back("-debug");<br>
<br>
bool DLL = Args.hasArg(options::OPT__SLASH_LD, options::OPT__SLASH_LDd,<br>
<br>
Modified: cfe/trunk/test/Driver/cl-options.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=274991&r1=274990&r2=274991&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=274991&r1=274990&r2=274991&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Driver/cl-options.c (original)<br>
+++ cfe/trunk/test/Driver/cl-options.c Sat Jul 9 16:49:16 2016<br>
@@ -420,7 +420,7 @@<br>
// Z7: "-gcodeview"<br>
// Z7: "-debug-info-kind=limited"<br>
<br>
-// RUN: %clang_cl -gline-tables-only /Z7 /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7GMLT %s<br>
+// RUN: %clang_cl /Zd /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7GMLT %s<br>
// Z7GMLT: "-gcodeview"<br>
// Z7GMLT: "-debug-info-kind=line-tables-only"<br>
<br>
<br>
<br>
_______________________________________________<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/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>