[PATCH][Solaris] Clang/Driver, stop hardcoding GCC paths in crt/ld.so lookup

Rafael Espíndola via cfe-commits cfe-commits at lists.llvm.org
Sun Aug 23 16:07:51 PDT 2015


SolarisScanLibDirForGCCTriple should start with a lower case. Starting
it with "scan" would probably also be more in line with the code
style.

LGTM

On 11 August 2015 at 16:33, Xan López <xan at igalia.com> wrote:
> Hi,
>
> thanks for the review, I was not even aware that this could be
> tested. Adding a test helped to fix me a couple extra issues (plus the
> one you already mentioned). New patch attached.
>
> Xan
>
> On Wed, Aug 05, 2015 at 09:14:30AM -0400, Rafael Espíndola wrote:
>> Please git-clang-format this patch.
>>
>> +  // /usr/gcc/<major>.<minor>/lib/gcc/<major>.<minor>.<patch>/,
>>
>> The code appends a triple after the "/lib/gcc". Is the comment missing it?
>>
>> The inner loop has no version comparison. Are you depending on the
>> directory iteration order?
>>
>> Can you add a testcase?
>>
>>
>> On 28 July 2015 at 12:35, Xan López <xan at igalia.com> wrote:
>> > Here it is.
>> >
>> > On Tue, Jul 28, 2015 at 01:21:06PM +0200, Xan López wrote:
>> >> On Tue, Jul 28, 2015 at 01:55:23PM +0300, Yaron Keren wrote:
>> >> > +cfe-commits
>> >> >
>> >> > This is a very large Solaris special case in ScanLibDirForGCCTriple which
>> >> > shares almost no code with the function.
>> >> > How about splitting it out to a helper function or
>> >> > making ScanLibDirForGCCTriple virtual and overriding on Solaris?
>> >>
>> >> Yep, at least a helper function makes sense, you are right. I'll send
>> >> another patch with either of those suggestions later today.
>> >>
>> >>
>> >> Xan
>> >> _______________________________________________
>> >> llvm-commits mailing list
>> >> llvm-commits at cs.uiuc.edu
>> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> >


More information about the cfe-commits mailing list