<div dir="ltr">Thats ok, thats why my patch is up for review. I do not have commit access but my mentor for GSoC, <span id=":bc" class="">Sylvestre, does have commit access.<br></span><table class="" id=":8z" tabindex="0" cellpadding="0">
<tbody id=":ba" class=""><tr class=""><td><br></td><td id=":bd" class=""><span id=":bc" class=""><br></span></td></tr></tbody></table></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jul 15, 2014 at 4:33 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.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="">On Tue, Jul 15, 2014 at 1:33 PM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:<br>

</div><div class="">> ================<br>
> Comment at: include/clang/Basic/DiagnosticDriverKinds.td:122<br>
> @@ -121,1 +121,3 @@<br>
><br>
> +def warn_linker_args_take_Wl : Warning<"linker argument -z should be -Wl,-z">,<br>
> +  InGroup<InvalidCommandLineArgument>;<br>
> ----------------<br>
> 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.<br>
><br>
> 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.<br>

><br>
<br>
</div>I've gone ahead and documented it in gcc here:<br>
<br>
dzur:~/sources/gcc> svn ci<br>
Sending        gcc/ChangeLog<br>
Sending        gcc/doc/invoke.texi<br>
Transmitting file data ..<br>
Committed revision 212575.<br>
<br>
Please go ahead and remove the diagnostic. I apologize about the churn<br>
for you. Once that's settled do you have commit access or do you need<br>
someone to commit for you?<br>
<span class="HOEnZb"><font color="#888888"><br>
-eric<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> ================<br>
> Comment at: include/clang/Driver/Options.td:312<br>
> @@ -311,1 +311,3 @@<br>
>    Flags<[DriverOption, CoreOption]>;<br>
> +def z_Flag : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>,<br>
> +  HelpText<"Pass -z <arg> to the linker">, MetaVarName<"<arg>">;<br>
> ----------------<br>
> This can be `def z :`.  For -Z we use Z_Flag and Z_Joined because I think they have different semantics.<br>
><br>
> ================<br>
> Comment at: lib/Driver/Tools.cpp:207<br>
> @@ +206,3 @@<br>
> +      // Pass -z prefix for gcc linker compatibility.<br>
> +      if (A.getOption().getName().equals(z_Flag)) {<br>
> +        D.Diag(diag::warn_linker_args_take_Wl);<br>
> ----------------<br>
> Why do this check this way instead of .matches(options::OPT_z)?<br>
><br>
> ================<br>
> Comment at: lib/Driver/Tools.cpp:209<br>
> @@ +208,3 @@<br>
> +        D.Diag(diag::warn_linker_args_take_Wl);<br>
> +        CmdArgs.push_back("-z");<br>
> +      }<br>
> ----------------<br>
> Why not `A.claim(); A.render(Args, CmdArgs)`?  Then you can split this whole clause into it's own 'else if' bucket.<br>
><br>
> ================<br>
> Comment at: lib/Driver/Tools.cpp:7588<br>
> @@ +7587,3 @@<br>
> +      // Pass -z prefix for gcc linker compatibility.<br>
> +        if (Input.getInputArg().getOption().getName().equals(z_Flag)) {<br>
> +          D.Diag(diag::warn_linker_args_take_Wl);<br>
> ----------------<br>
> ditto, why not .matches?<br>
><br>
> <a href="http://reviews.llvm.org/D4393" target="_blank">http://reviews.llvm.org/D4393</a><br>
><br>
><br>
</div></div></blockquote></div><br></div>