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
