[cfe-commits] [PATCH] Android support in Clang driver

[James Molloy]

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.

Apart from that, LGTM.

Cheers,

James

-----Original Message-----
From: Evgeniy Stepanov [mailto:eugeni.stepanov at gmail.com] 
Sent: 25 April 2012 08:19
To: James Molloy
Cc: cfe-commits at cs.uiuc.edu
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 <james.molloy at arm.com> 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: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of Evgeniy Stepanov
> Sent: 24 April 2012 15:15
> To: cfe-commits at cs.uiuc.edu
> 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.
>
>
>