r274991 - [clang-cl] Add support for /Zd
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 11 09:03:32 PDT 2016
On Mon, Jul 11, 2016 at 11:51 AM, David Majnemer <david.majnemer at gmail.com>
wrote:
>
>
> On Mon, Jul 11, 2016 at 8:42 AM, Nico Weber <thakis at chromium.org> wrote:
>
>> On Mon, Jul 11, 2016 at 11:36 AM, David Majnemer <
>> david.majnemer at gmail.com> wrote:
>>
>>>
>>>
>>> On Mon, Jul 11, 2016 at 7:18 AM, Nico Weber <thakis at chromium.org> wrote:
>>>
>>>> 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.
>>>>
>>>> (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.)
>>>>
>>>
>>> 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).
>>>
>>
>> Ok, but why is /Zd better than -gline-tables-only?
>>
>
> I see a few reasons:
> - It is less surprising for a debug flag in the cl world to be called
> /Zsomething instead of -gsomething.
>
Eh, you'll have to look it up either way to find the flag. And when seeing
the flag, the -g flag is more self-explanatory. Also, it is surprising that
a clang-cl /-style flag doesn't work with cl, so it just moves the surprise
around a bit.
> - 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.
>
I don't understand this point. Doesn't invading their namespace make
collisions _more_ likely?
> - I imagine that if they wanted to add support back for
> -gline-tables-only, they'd name it /Zd.
>
Given they had this flag and removed it, they probably didn't like it very
much :-)
> 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.
>
> Here why I think that's ok:
> - 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.
>
Sure, but these are flags that clang/gcc and cl interpret. So if you have a
gcc build and a visual studio build, it's fairly easy to get each going
with clang / clang-cl.
With /Zd, we're needlessly inventing a new spelling for an existing clang
flag. As far as I can see, the new name creates a (small) problem without
solving one.
So far, when we wanted to add a flag that cl doesn't have and that clang
has, we've always gone for the clang spelling of that flag. That seems like
a good guideline to me.
> - People who want harmony between Mac OS X, Linux and Windows on the
> driver side can just use clang++.
>
>
>>
>>
>>>
>>> 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.
>>>
>>> I'd agree with you if -gline-tables-only had made its way to an actual
>>> release.
>>>
>>>
>>>>
>>>> On Mon, Jul 11, 2016 at 10:08 AM, Nico Weber <thakis at chromium.org>
>>>> wrote:
>>>>
>>>>> This breaks existing users of -gline-tables-only. What's the
>>>>> motivation for this change?
>>>>>
>>>>
>>>>> On Sat, Jul 9, 2016 at 5:49 PM, David Majnemer via cfe-commits <
>>>>> cfe-commits at lists.llvm.org> wrote:
>>>>>
>>>>>> Author: majnemer
>>>>>> Date: Sat Jul 9 16:49:16 2016
>>>>>> New Revision: 274991
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=274991&view=rev
>>>>>> Log:
>>>>>> [clang-cl] Add support for /Zd
>>>>>>
>>>>>> MASM (ML.exe and ML64.exe) and older versions of MSVC (CL.exe)
>>>>>> support a
>>>>>> flag called /Zd which is more-or-less -gline-tables-only.
>>>>>>
>>>>>> It seems nicer to support this flag instead of exposing
>>>>>> -gline-tables-only.
>>>>>>
>>>>>> Modified:
>>>>>> cfe/trunk/include/clang/Driver/CLCompatOptions.td
>>>>>> cfe/trunk/include/clang/Driver/Options.td
>>>>>> cfe/trunk/lib/Driver/Tools.cpp
>>>>>> cfe/trunk/test/Driver/cl-options.c
>>>>>>
>>>>>> Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=274991&r1=274990&r2=274991&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
>>>>>> +++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Sat Jul 9
>>>>>> 16:49:16 2016
>>>>>> @@ -166,6 +166,8 @@ def _SLASH_Zc_trigraphs_off : CLFlag<"Zc
>>>>>> HelpText<"Disable trigraphs (default)">, Alias<fno_trigraphs>;
>>>>>> def _SLASH_Z7 : CLFlag<"Z7">,
>>>>>> HelpText<"Enable CodeView debug information in object files">;
>>>>>> +def _SLASH_Zd : CLFlag<"Zd">,
>>>>>> + HelpText<"Emit debug line number tables only">;
>>>>>> def _SLASH_Zi : CLFlag<"Zi">, Alias<_SLASH_Z7>,
>>>>>> HelpText<"Alias for /Z7. Does not produce PDBs.">;
>>>>>> def _SLASH_Zp : CLJoined<"Zp">,
>>>>>>
>>>>>> Modified: cfe/trunk/include/clang/Driver/Options.td
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=274991&r1=274990&r2=274991&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/include/clang/Driver/Options.td (original)
>>>>>> +++ cfe/trunk/include/clang/Driver/Options.td Sat Jul 9 16:49:16 2016
>>>>>> @@ -1229,7 +1229,7 @@ def fdebug_prefix_map_EQ
>>>>>> def g_Flag : Flag<["-"], "g">, Group<g_Group>,
>>>>>> HelpText<"Generate source-level debug information">;
>>>>>> def gline_tables_only : Flag<["-"], "gline-tables-only">,
>>>>>> Group<gN_Group>,
>>>>>> - Flags<[CoreOption]>, HelpText<"Emit debug line number tables
>>>>>> only">;
>>>>>> + HelpText<"Emit debug line number tables only">;
>>>>>> def gmlt : Flag<["-"], "gmlt">, Alias<gline_tables_only>;
>>>>>> def g0 : Flag<["-"], "g0">, Group<gN_Group>;
>>>>>> def g1 : Flag<["-"], "g1">, Group<gN_Group>,
>>>>>> Alias<gline_tables_only>;
>>>>>>
>>>>>> Modified: cfe/trunk/lib/Driver/Tools.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=274991&r1=274990&r2=274991&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/lib/Driver/Tools.cpp (original)
>>>>>> +++ cfe/trunk/lib/Driver/Tools.cpp Sat Jul 9 16:49:16 2016
>>>>>> @@ -6269,12 +6269,18 @@ void Clang::AddClangCLArgs(const ArgList
>>>>>>
>>>>>> CmdArgs.push_back(Args.MakeArgString(Twine(LangOptions::SSPStrong)));
>>>>>> }
>>>>>>
>>>>>> - // Emit CodeView if -Z7 is present.
>>>>>> - *EmitCodeView = Args.hasArg(options::OPT__SLASH_Z7);
>>>>>> - if (*EmitCodeView)
>>>>>> - *DebugInfoKind = codegenoptions::LimitedDebugInfo;
>>>>>> - if (*EmitCodeView)
>>>>>> + // Emit CodeView if -Z7 or -Zd are present.
>>>>>> + if (Arg *DebugInfoArg =
>>>>>> + Args.getLastArg(options::OPT__SLASH_Z7,
>>>>>> options::OPT__SLASH_Zd)) {
>>>>>> + *EmitCodeView = true;
>>>>>> + if (DebugInfoArg->getOption().matches(options::OPT__SLASH_Z7))
>>>>>> + *DebugInfoKind = codegenoptions::LimitedDebugInfo;
>>>>>> + else
>>>>>> + *DebugInfoKind = codegenoptions::DebugLineTablesOnly;
>>>>>> CmdArgs.push_back("-gcodeview");
>>>>>> + } else {
>>>>>> + *EmitCodeView = false;
>>>>>> + }
>>>>>>
>>>>>> const Driver &D = getToolChain().getDriver();
>>>>>> EHFlags EH = parseClangCLEHFlags(D, Args);
>>>>>> @@ -9964,7 +9970,8 @@ void visualstudio::Linker::ConstructJob(
>>>>>>
>>>>>> CmdArgs.push_back("-nologo");
>>>>>>
>>>>>> - if (Args.hasArg(options::OPT_g_Group, options::OPT__SLASH_Z7))
>>>>>> + if (Args.hasArg(options::OPT_g_Group, options::OPT__SLASH_Z7,
>>>>>> + options::OPT__SLASH_Zd))
>>>>>> CmdArgs.push_back("-debug");
>>>>>>
>>>>>> bool DLL = Args.hasArg(options::OPT__SLASH_LD,
>>>>>> options::OPT__SLASH_LDd,
>>>>>>
>>>>>> Modified: cfe/trunk/test/Driver/cl-options.c
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=274991&r1=274990&r2=274991&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/test/Driver/cl-options.c (original)
>>>>>> +++ cfe/trunk/test/Driver/cl-options.c Sat Jul 9 16:49:16 2016
>>>>>> @@ -420,7 +420,7 @@
>>>>>> // Z7: "-gcodeview"
>>>>>> // Z7: "-debug-info-kind=limited"
>>>>>>
>>>>>> -// RUN: %clang_cl -gline-tables-only /Z7 /c -### -- %s 2>&1 |
>>>>>> FileCheck -check-prefix=Z7GMLT %s
>>>>>> +// RUN: %clang_cl /Zd /c -### -- %s 2>&1 | FileCheck
>>>>>> -check-prefix=Z7GMLT %s
>>>>>> // Z7GMLT: "-gcodeview"
>>>>>> // Z7GMLT: "-debug-info-kind=line-tables-only"
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> cfe-commits mailing list
>>>>>> cfe-commits at lists.llvm.org
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160711/4ffda1ef/attachment-0001.html>
More information about the cfe-commits
mailing list