mcrosier added inline comments. ================ Comment at: lib/Driver/Tools.cpp:3279 @@ -3278,1 +3278,3 @@ + + Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr); } ---------------- silvas wrote: > xur wrote: > > mcrosier wrote: > > > I don't think AddAllArgs is what you really want. What if the user > > > specifies the option twice? Do we really want to pass the flag from the > > > driver to the front-end twice? Also, should we warn if the option is > > > passed twice? > > > > > > I also think some of the logic in CompilerInvocation should land here... > > > see comments below. > > Thanks for pointing thi out. What about add a guard, like: > > - Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr); > > + if (Args.hasArg(options::OPT_fprofile_ir_instr)) > > + Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr); > > > > But looking at it again, I think i need to remove this stmt in the most > > recently patch as OPT_profile_ir_instr is now a CC1 option. it will not be > > seen here, right? > > > > > > But looking at it again, I think i need to remove this stmt in the most > > recently patch as OPT_profile_ir_instr is now a CC1 option. it will not be > > seen here, right? > > Yes. The patch should make no changes to lib/Driver and should require no > tests in test/Driver Ah, okay. I still think this type of complexity should land in the driver, but if this is a special case then just ignore my comments.
http://reviews.llvm.org/D15829 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
