On Wed, Apr 25, 2012 at 11:50 AM, James Molloy <[email protected]> wrote: > Hi Evgeniy, > > That looks much better, thanks! My last comment is that now you've merged > addAsanRT* into addAsanRTLinux, it should be renamed to just "addAsanRT" or > "addAsanRTUnix", I think.
Android is Linux (we use arm-linux-androideabi triple), so addAsanRTLinux seems appropriate. > > Apart from that, LGTM. > > Cheers, > > James > > -----Original Message----- > From: Evgeniy Stepanov [mailto:[email protected]] > Sent: 25 April 2012 08:19 > To: James Molloy > Cc: [email protected] > Subject: Re: [cfe-commits] [PATCH] Android support in Clang driver > > Hi, > > thanks for looking at the patch! > I've merged addAsanRT* into one function and wrote some tests. Please > take another look. > > On Tue, Apr 24, 2012 at 6:50 PM, James Molloy <[email protected]> wrote: >> Hi Evgeniy, >> >> You're right, it is ugly, but that unfortunately is par for the course for a >> driver patch :( >> >> You change several functions to take a Triple argument and switch on that >> inside, but for addAsanRT you add a brand new function. It'd be cleaner to >> do the same and add a Triple argument, modifying the behaviour of addAsanRT >> depending on Linux or Android. >> >> Also, testcases...! >> >> Cheers, >> >> James >> >> -----Original Message----- >> From: [email protected] >> [mailto:[email protected]] On Behalf Of Evgeniy Stepanov >> Sent: 24 April 2012 15:15 >> To: [email protected] >> Subject: [cfe-commits] [PATCH] Android support in Clang driver >> >> Hi, >> >> this patch add Android support to linuxtools::Link. This allows using >> Clang as a drop-in GCC replacement in Android NDK. >> >> It is kind of ugly, sorry about that. There is simply too many small >> differencies in how things are done between android and linux. >> >> >> > > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
