On Mon, Jul 11, 2016 at 8:42 AM, Nico Weber <tha...@chromium.org> wrote:
> On Mon, Jul 11, 2016 at 11:36 AM, David Majnemer <david.majne...@gmail.com > > wrote: > >> >> >> On Mon, Jul 11, 2016 at 7:18 AM, Nico Weber <tha...@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 <tha...@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@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@lists.llvm.org >>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>>> >>>> >>>> >>> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
