<div class="gmail_quote">On 20 April 2011 21:44, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

<div class="gmail_quote"><div class="im">On Wed, Apr 20, 2011 at 7:06 PM, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<div>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!</div></blockquote><div><br></div>


</div><div>Some comments:</div><div><br></div><div>Index: include/clang/Driver/Options.td</div><div>===================================================================</div><div>--- include/clang/Driver/Options.td<span style="white-space:pre-wrap">      </span>(revision 129902)</div>


<div>+++ include/clang/Driver/Options.td<span style="white-space:pre-wrap">       </span>(working copy)</div><div>@@ -280,6 +280,7 @@</div><div> def feliminate_unused_debug_symbols : Flag<"-feliminate-unused-debug-symbols">, Group<f_Group>;</div>


<div> def femit_all_decls : Flag<"-femit-all-decls">, Group<f_Group>;</div><div> def fencoding_EQ : Joined<"-fencoding=">, Group<f_Group>;</div><div>+def ferror_limit_EQ : Joined<"-ferror-limit=">, Group<f_Group>;</div>


<div><br></div><div>Was this intended to be part of the commit?</div></div></blockquote><div><br></div><div>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.</div>

<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="gmail_quote"><div>Index: lib/Frontend/CompilerInvocation.cpp</div><div><div>===================================================================</div>

<div>
--- lib/Frontend/CompilerInvocation.cpp<span style="white-space:pre-wrap">      </span>(revision 129902)</div><div>+++ lib/Frontend/CompilerInvocation.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div>
<div>@@ -123,6 +123,10 @@</div><div>     Res.push_back("-dwarf-debug-flags");</div><div>     Res.push_back(Opts.DwarfDebugFlags);</div><div>   }</div><div>+  if (Opts.EmitGcovArcs)</div><div>+    Res.push_back("-emit-coverage-data");</div>


<div>+  if (Opts.EmitGcovNotes)</div><div>+    Res.push_back("-emit-coverage-notes");</div></div><div><br></div><div>Er, where is the "f" in these flags?</div></div></blockquote><div><br></div><div>There is none, these are cc1 flags. I don't really care what these are named.</div>

<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="gmail_quote"><div><br></div><div><div>@@ -81,7 +81,9 @@</div>
<div>                            ABI.getMangleContext());</div><div> </div><div>   // If debug info generation is enabled, create the CGDebugInfo object.</div><div>-  DebugInfo = CodeGenOpts.DebugInfo ? new CGDebugInfo(*this) : 0;</div>


<div>+  if (CodeGenOpts.DebugInfo || CodeGenOpts.EmitGcovArcs ||</div><div>+      CodeGenOpts.EmitGcovNotes)</div><div>+    DebugInfo = new CGDebugInfo(*this);</div><div><br></div><div>The comment for this code needs updating</div>

</div></div></blockquote><div><br></div><div>Thanks! It now reads:</div><div><br></div><div><div>  // If debug info or coverage generation is enabled, create the CGDebugInfo</div><div>  // object.</div><div>  if (CodeGenOpts.DebugInfo || CodeGenOpts.EmitGcovArcs ||</div>

<div>      CodeGenOpts.EmitGcovNotes)</div><div>    DebugInfo = new CGDebugInfo(*this);</div></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="gmail_quote">

<div>
<div>    Block.GlobalUniqueCount = 0;</div><div> </div><div>@@ -590,7 +592,7 @@</div><div> </div><div>     GlobalDecl D = DeferredDeclsToEmit.back();</div><div>     DeferredDeclsToEmit.pop_back();</div><div>-</div>
<div>+   </div><div><br></div><div>Extraneous whitespace change...</div></div></div>
</blockquote></div><br><div>Fixed too. Thanks for the review!</div><div><br></div><div>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.</div>

<div><br></div><div>Nick</div>