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

Arthur Marble arthur at info9.net
Tue Jul 15 14:39:42 PDT 2014


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/c959fd2f/attachment.html>


More information about the cfe-commits mailing list