[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: Re: [PATCH] D11737: Add -linker (and -linker=) alias for -fuse-ld=
From: Richard Smith via cfe-commits <cfe-commits () lists ! llvm ! org>
Date: 2015-08-25 21:57:33
Message-ID: CAOfiQqnqcd4SVXoOQ65JZKiSB5Q9T0q_Kn4X2cprMDWJx8KDnA () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Tue, Aug 25, 2015 at 1:39 PM, Filipe Cabecinhas <
filcab+llvm.phabricator@gmail.com> wrote:
> Hi Richard,
>
> On Tue, Aug 25, 2015 at 11:01 AM, Richard Smith <richard@metafoo.co.uk>
> wrote:
>
>> On Aug 25, 2015 10:26 AM, "Filipe Cabecinhas" <
>> filcab+llvm.phabricator@gmail.com> wrote:
>> >
>> > Hi Richard,
>> >
>> > We wouldn't be able to link with libs matching
>> "libinker=*.{dylib,so,...}", I guess. If that is a big problem and you'd
>> prefer that we keep this as a private patch, let me know.
>>
>> I don't think it's a big problem, more just a "try to pick better flag
>> names in future" comment :) It sounds like you guys have existing systems
>> that depend on this name, so while I'm not really overjoyed about this,
>> accepting it for compatibility seems OK.
>>
> Thanks.
>
>> Can we produce an accompanying "deprecated" warning suggesting use of the
>> other name?
>>
> Not on our side. If you still want the warning, I'd prefer to just keep
> the flag private than to have the option+warning upstream (basically just
> for PS4 toolchain use) and have a private patch to remove the warning
> anyway.
>
> Let me know what you think,
>
What do you see as the future of this flag? If this is something you will
need to keep around essentially forever, then we should just take this
change into upstream clang. In that case, the patch LGTM.
If this is something you need for short-term compatibility until the
projects using it switch to -fuse-ld= or become obsolescent, then it makes
less sense to take it as an upstream change (if we accept it upstream
without a deprecated warning, it's likely that some other projects will
start to rely on it, which makes it harder to eventually remove).
Filipe
>
> > Thank you,
>> >
>> > Filipe
>> >
>> >
>> > On Mon, Aug 24, 2015 at 6:45 PM, Richard Smith <richard@metafoo.co.uk>
>> wrote:
>> >>
>> >> On Mon, Aug 24, 2015 at 5:50 PM, Eric Christopher via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>> >>>
>> >>> echristo added inline comments.
>> >>>
>> >>> ================
>> >>> Comment at: include/clang/Driver/Options.td:1853
>> >>> @@ -1853,1 +1852,3 @@
>> >>> +def fuse_ld_EQ : Joined<["-"], "fuse-ld=">, HelpText<"Use linker
>> <name>">, Group<f_Group>;
>> >>> +def linker_EQ : Joined<["-"], "linker=">, Alias<fuse_ld_EQ>,
>> MetaVarName<"<name>">;
>> >>>
>> >>> ----------------
>> >>> thakis wrote:
>> >>> > Any reason to have another alias for this at all? Does gcc
>> understand this spelling? If not, could you limit this flag to PS4 targets?
>> (I'm guessing you need it for PS4 targets for some reason.)
>> >>> Any reason? (And I'm not sure how to limit it btw on target).
>> >>
>> >>
>> >> -l already has a meaning; adding a different flag starting with -l is
>> a bad idea.
>> >
>> >
>>
>
>
[Attachment #5 (text/html)]
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 25, 2015 \
at 1:39 PM, Filipe Cabecinhas <span dir="ltr"><<a \
href="mailto:filcab+llvm.phabricator@gmail.com" \
target="_blank">filcab+llvm.phabricator@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 dir="ltr">Hi Richard,<br><div \
class="gmail_extra"><br><div class="gmail_quote"><span class="">On Tue, Aug 25, 2015 \
at 11:01 AM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" \
target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><span><p dir="ltr">On Aug 25, 2015 10:26 AM, "Filipe \
Cabecinhas" <<a href="mailto:filcab%2Bllvm.phabricator@gmail.com" \
target="_blank">filcab+llvm.phabricator@gmail.com</a>> wrote:<br> ><br>
> Hi Richard,<br>
><br>
> We wouldn't be able to link with libs matching \
"libinker=*.{dylib,so,...}", I guess. If that is a big problem and \
you'd prefer that we keep this as a private patch, let me know.</p> </span><p \
dir="ltr">I don't think it's a big problem, more just a "try to pick \
better flag names in future" comment :) It sounds like you guys have existing \
systems that depend on this name, so while I'm not really overjoyed about this, \
accepting it for compatibility seems OK.</p></blockquote></span><div>Thanks. \
<br></div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"> <p dir="ltr">Can we produce an \
accompanying "deprecated" warning suggesting use of the other \
name?</p></blockquote></span><div>Not on our side. If you still want the warning, \
I'd prefer to just keep the flag private than to have the option+warning upstream \
(basically just for PS4 toolchain use) and have a private patch to remove the warning \
anyway.</div><div><br></div><div>Let me know what you \
think,</div></div></div></div></blockquote><div><br></div><div>What do you see as the \
future of this flag? If this is something you will need to keep around essentially \
forever, then we should just take this change into upstream clang. In that case, the \
patch LGTM.</div><div><br></div><div>If this is something you need for short-term \
compatibility until the projects using it switch to -fuse-ld= or become obsolescent, \
then it makes less sense to take it as an upstream change (if we accept it upstream \
without a deprecated warning, it's likely that some other projects will start to \
rely on it, which makes it harder to eventually \
remove).</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div \
class="gmail_extra"><div class="gmail_quote"><div> Filipe</div><span \
class=""><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div> <p dir="ltr">> Thank \
you,<br> ><br>
> Filipe<br>
><br>
><br>
> On Mon, Aug 24, 2015 at 6:45 PM, Richard Smith <<a \
href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> \
wrote:<br> >><br>
>> On Mon, Aug 24, 2015 at 5:50 PM, Eric Christopher via cfe-commits <<a \
href="mailto:cfe-commits@lists.llvm.org" \
target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br> >>><br>
>>> echristo added inline comments.<br>
>>><br>
>>> ================<br>
>>> Comment at: include/clang/Driver/Options.td:1853<br>
>>> @@ -1853,1 +1852,3 @@<br>
>>> +def fuse_ld_EQ : Joined<["-"], "fuse-ld=">, \
HelpText<"Use linker <name>">, Group<f_Group>;<br> \
>>> +def linker_EQ : Joined<["-"], "linker=">, \
Alias<fuse_ld_EQ>, MetaVarName<"<name>">;<br> \
>>><br> >>> ----------------<br>
>>> thakis wrote:<br>
>>> > Any reason to have another alias for this at all? Does gcc \
understand this spelling? If not, could you limit this flag to PS4 targets? (I'm \
guessing you need it for PS4 targets for some reason.)<br> >>> Any reason? \
(And I'm not sure how to limit it btw on target).<br> >><br>
>><br>
>> -l already has a meaning; adding a different flag starting with -l is a bad \
idea.<br> ><br>
><br>
</p>
</div></div></blockquote></span></div><br></div></div>
</blockquote></div><br></div></div>
[Attachment #6 (text/plain)]
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic