[cfe-commits] patch: add gcov-compatible coverage support to clang

Nick Lewycky nlewycky at google.com
Thu Apr 21 14:27:53 PDT 2011


On 20 April 2011 21:44, Chandler Carruth <chandlerc at google.com> wrote:

> On Wed, Apr 20, 2011 at 7:06 PM, Nick Lewycky <nlewycky at google.com> wrote:
>
>> This patch adds the -ftest-coverage flag, and wires it--along with
>> -coverage and -fprofile-arcs--up to llvm's recently added support for
>> emission of gcov data files. Please review!
>>
>
> Some comments:
>
> Index: include/clang/Driver/Options.td
> ===================================================================
> --- include/clang/Driver/Options.td (revision 129902)
> +++ include/clang/Driver/Options.td (working copy)
> @@ -280,6 +280,7 @@
>  def feliminate_unused_debug_symbols :
> Flag<"-feliminate-unused-debug-symbols">, Group<f_Group>;
>  def femit_all_decls : Flag<"-femit-all-decls">, Group<f_Group>;
>  def fencoding_EQ : Joined<"-fencoding=">, Group<f_Group>;
> +def ferror_limit_EQ : Joined<"-ferror-limit=">, Group<f_Group>;
>
> Was this intended to be part of the commit?
>

Sure why not. I saw that it wasn't sorted when I was adding a flag adjacent
to it so I fixed that in this patch.


> Index: lib/Frontend/CompilerInvocation.cpp
> ===================================================================
>  --- lib/Frontend/CompilerInvocation.cpp (revision 129902)
> +++ lib/Frontend/CompilerInvocation.cpp (working copy)
> @@ -123,6 +123,10 @@
>      Res.push_back("-dwarf-debug-flags");
>      Res.push_back(Opts.DwarfDebugFlags);
>    }
> +  if (Opts.EmitGcovArcs)
> +    Res.push_back("-emit-coverage-data");
> +  if (Opts.EmitGcovNotes)
> +    Res.push_back("-emit-coverage-notes");
>
> Er, where is the "f" in these flags?
>

There is none, these are cc1 flags. I don't really care what these are
named.


>
> @@ -81,7 +81,9 @@
>                             ABI.getMangleContext());
>
>    // If debug info generation is enabled, create the CGDebugInfo object.
> -  DebugInfo = CodeGenOpts.DebugInfo ? new CGDebugInfo(*this) : 0;
> +  if (CodeGenOpts.DebugInfo || CodeGenOpts.EmitGcovArcs ||
> +      CodeGenOpts.EmitGcovNotes)
> +    DebugInfo = new CGDebugInfo(*this);
>
> The comment for this code needs updating
>

Thanks! It now reads:

  // If debug info or coverage generation is enabled, create the CGDebugInfo
  // object.
  if (CodeGenOpts.DebugInfo || CodeGenOpts.EmitGcovArcs ||
      CodeGenOpts.EmitGcovNotes)
    DebugInfo = new CGDebugInfo(*this);


>      Block.GlobalUniqueCount = 0;
>
> @@ -590,7 +592,7 @@
>
>      GlobalDecl D = DeferredDeclsToEmit.back();
>      DeferredDeclsToEmit.pop_back();
> -
> +
>
> Extraneous whitespace change...
>

Fixed too. Thanks for the review!

As an aside, this patch only adds support for compiling with these flags.
They're also supposed to add the profile_rt library when asked to link, but
I haven't written that yet. It seems a cleanly divisible change to me.

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110421/148b7046/attachment.html>


More information about the cfe-commits mailing list