OK, feel free to commit after or send me something that applies and I'll do it for you.
Thanks! On Tue, Jul 15, 2014 at 2:39 PM, Arthur Marble <[email protected]> wrote: > Thats ok, thats why my patch is up for review. I do not have commit access > but my mentor for GSoC, Sylvestre, does have commit access. > > > > > On Tue, Jul 15, 2014 at 4:33 PM, Eric Christopher <[email protected]> > wrote: > >> On Tue, Jul 15, 2014 at 1:33 PM, Reid Kleckner <[email protected]> wrote: >> > ================ >> > Comment at: include/clang/Basic/DiagnosticDriverKinds.td:122 >> > @@ -121,1 +121,3 @@ >> > >> > +def warn_linker_args_take_Wl : Warning<"linker argument -z should be >> -Wl,-z">, >> > + InGroup<InvalidCommandLineArgument>; >> > ---------------- >> > Maybe "linker argument %0 should be escaped with -Wl, and commas"? >> Specifically, I think many users won't realize that they need to escape >> the second space after -z. >> > >> > Alternatively, I would be happy if we removed this diagnostic, but I >> know Joerg won't. My preferred resolution is, again, that we ask GCC to >> document -z as a linker option and then we support it without equivocation. >> > >> >> I've gone ahead and documented it in gcc here: >> >> dzur:~/sources/gcc> svn ci >> Sending gcc/ChangeLog >> Sending gcc/doc/invoke.texi >> Transmitting file data .. >> Committed revision 212575. >> >> Please go ahead and remove the diagnostic. I apologize about the churn >> for you. Once that's settled do you have commit access or do you need >> someone to commit for you? >> >> -eric >> >> > ================ >> > Comment at: include/clang/Driver/Options.td:312 >> > @@ -311,1 +311,3 @@ >> > Flags<[DriverOption, CoreOption]>; >> > +def z_Flag : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>, >> > + HelpText<"Pass -z <arg> to the linker">, MetaVarName<"<arg>">; >> > ---------------- >> > This can be `def z :`. For -Z we use Z_Flag and Z_Joined because I >> think they have different semantics. >> > >> > ================ >> > Comment at: lib/Driver/Tools.cpp:207 >> > @@ +206,3 @@ >> > + // Pass -z prefix for gcc linker compatibility. >> > + if (A.getOption().getName().equals(z_Flag)) { >> > + D.Diag(diag::warn_linker_args_take_Wl); >> > ---------------- >> > Why do this check this way instead of .matches(options::OPT_z)? >> > >> > ================ >> > Comment at: lib/Driver/Tools.cpp:209 >> > @@ +208,3 @@ >> > + D.Diag(diag::warn_linker_args_take_Wl); >> > + CmdArgs.push_back("-z"); >> > + } >> > ---------------- >> > Why not `A.claim(); A.render(Args, CmdArgs)`? Then you can split this >> whole clause into it's own 'else if' bucket. >> > >> > ================ >> > Comment at: lib/Driver/Tools.cpp:7588 >> > @@ +7587,3 @@ >> > + // Pass -z prefix for gcc linker compatibility. >> > + if (Input.getInputArg().getOption().getName().equals(z_Flag)) { >> > + D.Diag(diag::warn_linker_args_take_Wl); >> > ---------------- >> > ditto, why not .matches? >> > >> > http://reviews.llvm.org/D4393 >> > >> > >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
