On 20 April 2011 21:44, Chandler Carruth <[email protected]> wrote:
> On Wed, Apr 20, 2011 at 7:06 PM, Nick Lewycky <[email protected]> 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
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
