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

Reply via email to