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.
On Mon, Aug 29, 2016 at 8:49 AM, David Blaikie <dblai...@gmail.com> wrote: > > > On Thu, Aug 25, 2016 at 11:32 AM Adrian McCarthy via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: amccarth >> Date: Thu Aug 25 13:24:35 2016 >> New Revision: 279765 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=279765&view=rev >> Log: >> Omit column info for CodeView by default >> >> 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. >> >> So by default, we'll omit the column information for CodeView targeting >> Windows. >> >> 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. >> >> Differential Revision: https://reviews.llvm.org/D23720 >> >> Modified: >> 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/Options.td >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/ >> clang/Driver/Options.td?rev=279765&r1=279764&r2=279765&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/include/clang/Driver/Options.td (original) >> +++ cfe/trunk/include/clang/Driver/Options.td Thu Aug 25 13:24:35 2016 >> @@ -1305,8 +1305,8 @@ def gno_record_gcc_switches : Flag<["-"] >> Group<g_flags_Group>; >> def gstrict_dwarf : Flag<["-"], "gstrict-dwarf">, Group<g_flags_Group>; >> def gno_strict_dwarf : Flag<["-"], "gno-strict-dwarf">, >> Group<g_flags_Group>; >> -def gcolumn_info : Flag<["-"], "gcolumn-info">, Group<g_flags_Group>; >> -def gno_column_info : Flag<["-"], "gno-column-info">, >> Group<g_flags_Group>; >> +def gcolumn_info : Flag<["-"], "gcolumn-info">, Group<g_flags_Group>, >> Flags<[CoreOption]>; >> +def gno_column_info : Flag<["-"], "gno-column-info">, >> Group<g_flags_Group>, Flags<[CoreOption]>; >> def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group<g_flags_Group>; >> def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group<g_flags_Group>; >> def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group<g_flags_Group>; >> >> Modified: cfe/trunk/lib/Driver/Tools.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ >> Tools.cpp?rev=279765&r1=279764&r2=279765&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/lib/Driver/Tools.cpp (original) >> +++ cfe/trunk/lib/Driver/Tools.cpp Thu Aug 25 13:24:35 2016 >> @@ -2574,7 +2574,7 @@ static void getTargetFeatures(const Tool >> case llvm::Triple::wasm32: >> case llvm::Triple::wasm64: >> getWebAssemblyTargetFeatures(Args, Features); >> - break; >> + break; >> case llvm::Triple::sparc: >> case llvm::Triple::sparcel: >> case llvm::Triple::sparcv9: >> @@ -4656,9 +4656,13 @@ void Clang::ConstructJob(Compilation &C, >> // We ignore flags -gstrict-dwarf and -grecord-gcc-switches for now. >> Args.ClaimAllArgs(options::OPT_g_flags_Group); >> >> - // PS4 defaults to no column info >> + // Column info is included by default for everything except PS4 and >> CodeView. >> + // Clang doesn't track end columns, just starting columns, which, in >> theory, >> + // is fine for CodeView (and PDB). In practice, however, the Microsoft >> + // debuggers don't handle missing end columns well, so it's better not >> to >> + // include any column info. >> if (Args.hasFlag(options::OPT_gcolumn_info, >> options::OPT_gno_column_info, >> - /*Default=*/ !IsPS4CPU)) >> + /*Default=*/ !IsPS4CPU && !(IsWindowsMSVC && >> EmitCodeView))) >> CmdArgs.push_back("-dwarf-column-info"); >> >> // FIXME: Move backend command line options to the module. >> @@ -6712,7 +6716,7 @@ void ClangAs::ConstructJob(Compilation & >> case llvm::Triple::mips64el: >> AddMIPSTargetArgs(Args, CmdArgs); >> break; >> - >> + >> case llvm::Triple::x86: >> case llvm::Triple::x86_64: >> AddX86TargetArgs(Args, CmdArgs); >> >> Modified: cfe/trunk/test/Driver/cl-options.c >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/ >> cl-options.c?rev=279765&r1=279764&r2=279765&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/test/Driver/cl-options.c (original) >> +++ cfe/trunk/test/Driver/cl-options.c Thu Aug 25 13:24:35 2016 >> @@ -50,6 +50,15 @@ >> // fpstrict-NOT: -menable-unsafe-fp-math >> // fpstrict-NOT: -ffast-math >> >> +// RUN: %clang_cl /Z7 -gcolumn-info -### -- %s 2>&1 | FileCheck >> -check-prefix=gcolumn %s >> +// gcolumn: -dwarf-column-info >> + >> +// RUN: %clang_cl /Z7 -gno-column-info -### -- %s 2>&1 | FileCheck >> -check-prefix=gnocolumn %s >> > > 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) > > >> +// gnocolumn-NOT: -dwarf-column-info >> + >> +// RUN: %clang_cl /Z7 -### -- %s 2>&1 | FileCheck >> -check-prefix=gdefcolumn %s >> +// gdefcolumn-NOT: -dwarf-column-info >> + >> // RUN: %clang_cl /GA -### -- %s 2>&1 | FileCheck -check-prefix=GA %s >> // GA: -ftls-model=local-exec >> >> >> >> _______________________________________________ >> 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