<div dir="ltr">Thanks for the information. I will make those changes.<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jul 15, 2014 at 3:33 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">================<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>
Comment at: include/clang/Driver/Options.td:312<br>
@@ -311,1 +311,3 @@<br>
<div class="">   Flags<[DriverOption, CoreOption]>;<br>
+def z_Flag : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>,<br>
+  HelpText<"Pass -z <arg> to the linker">, MetaVarName<"<arg>">;<br>
</div>----------------<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>
<div class="">+      // Pass -z prefix for gcc linker compatibility.<br>
+      if (A.getOption().getName().equals(z_Flag)) {<br>
</div>+        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>
<div class="">+        CmdArgs.push_back("-z");<br>
+      }<br>
</div>----------------<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>
<div class="">+      // Pass -z prefix for gcc linker compatibility.<br>
+        if (Input.getInputArg().getOption().getName().equals(z_Flag)) {<br>
</div>+          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>
</blockquote></div><br></div>