r279765 - Omit column info for CodeView by default

[David Blaikie via cfe-commits]

On Thu, Aug 25, 2016 at 11:32 AM Adrian McCarthy via cfe-commits <
cfe-commits at 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 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/20160829/078d8679/attachment.html>