[PATCH] GCC compatibility: pass -z linker options to the linker

[Eric Christopher]

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 <arthur at info9.net> 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 <echristo at gmail.com>
> wrote:
>
>> On Tue, Jul 15, 2014 at 1:33 PM, Reid Kleckner <rnk at google.com> 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
>> >
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140715/3a432085/attachment.html>