[cfe-commits] r79288 - /cfe/trunk/lib/CodeGen/TargetABIInfo.cpp

[Daniel Dunbar]

On Tue, Aug 25, 2009 at 3:39 AM, David Chisnall<csdavec at swansea.ac.uk> wrote:
> On 25 Aug 2009, at 08:20, Daniel Dunbar wrote:
>
>> On Thu, Aug 20, 2009 at 5:23 AM, David Chisnall<csdavec at swansea.ac.uk>
>> wrote:
>>>
>>> On 20 Aug 2009, at 07:15, Chris Lattner wrote:
>>>
>>>> I admit that the original check for darwin was completely hideous here,
>>>> but this is making it a lot worse.  Can this be factored out into
>>>> targetInfo
>>>> somehow?  At the very least, can this use llvm::Triple to do this
>>>> decoding?
>>>
>>>
>>> Agreed - very ugly.  The attached patch makes it much less so.  Since we
>>> were already passing the Context, I've moved all of the
>>> architecture-specific tweaking inside the constructor and modified it to
>>> use
>>> llvm::Triple.
>>
>> Oops, I missed this patch. However, I since fixed this to basically
>> have the same effect, although its still outside the constructor.
>
> Would it make more sense to move it into the constructor?  It still seems
> messy having IA32-specific stuff in the generic code.

I personally prefer all the policy decisions to be grouped together,
and to not have constructors do real work or make descisions. For
example, if we had a command line argument for this I would consider
it wrong to need to pass LangOptions in to the constructor.

 - Daniel