r246473.
On 31 August 2015 at 13:30, Xan López <x...@igalia.com> wrote: > Oops, missed this. Here is the updated patch. > > And yes please, commit the patch for me. > > Cheers, > > Xan > > On Mon, Aug 31, 2015 at 01:07:35PM -0400, Rafael Espíndola wrote: >> Do you have a version with the last suggestions? I lgtmed it conditional on >> it. Do you need someone to commit it for you? >> On Aug 23, 2015 7:07 PM, "Rafael Espíndola" <rafael.espind...@gmail.com> >> wrote: >> >> > 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 <x...@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 <x...@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-comm...@cs.uiuc.edu >> > >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits >> > >> > >> > >> > _______________________________________________ >> > >> > llvm-commits mailing list >> > >> > llvm-comm...@cs.uiuc.edu >> > >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits >> > >> > >> > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
