r274991 - [clang-cl] Add support for /Zd

David Majnemer via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 11 08:51:27 PDT 2016


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.
- 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 imagine that if they wanted to add support back for -gline-tables-only,
they'd name it /Zd.

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.
- 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/68b8cd1b/attachment.html>


More information about the cfe-commits mailing list