[llvm] r179051 - Template the MachO types over the word size.

[Kaylor, Andrew]

If I may interject an opinion, I found the templating in ELF objects to be a bit of a nuisance when we were fleshing out the RuntimeDyldELF implementation.  I'm sure there were some things that were easier because of it in ways that I didn't even notice, but there were also places where it felt burdensome.

-Andy

-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Eric Christopher
Sent: Tuesday, April 16, 2013 3:37 PM
To: Rafael Espíndola
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm] r179051 - Template the MachO types over the word size.

On Tue, Apr 16, 2013 at 1:12 PM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
> On 16 April 2013 15:53, Jim Grosbach <grosbach at apple.com> wrote:
>> Ping.
>>
>> I reiterate that I have serious problems with this patch. Having not 
>> heard any explanation for why it's an improvement, I want it 
>> reverted. Sooner rather than later, please.
>
> Sorry, I completely missed the original reply and only now noticed this thread.
>
> The objective of templating first on 32/64 bits and then on endianness 
> is to allow generic access to macho objects in the same way we have 
> access to elf objects. I do realize the in between states are a bit 
> messy, so let me explain the overall design.
>
> From an user perspective (say llvm-readobj), they can just template 
> code that needs macho specific type. Inside the template the different 
> data structures are easily available.
>
> For code that is not templated, typedefs are now provided for an 
> easier access. For example, you can say MachOObjectFileLE64::Section 
> to access the section of a 64 bit little endian object.
>
> The main win of having the templates is the ability to access little 
> and big endian objects without having to put an if at every memory 
> access.
>
> The code is structured with a base class that is common to all 4 
> variants, a middle class that is common to 64 and 32 bits and the 4 
> base classes.
>
> We are now able to correctly process all 4 types of objects (with 
> tests!), so I am now more comfortable refactoring this a bit. On the 
> list are
>
> * Any missing features.
> * Sharing code when possible. The relocation name logic is big on the list.
> * Making more methods private.
> * Moving the individual templates to a detail namespace to make it 
> clear that they should not be used directly.
>
> I hope this explains what is going on.
>

I'm having problems coming up with a solution that doesn't involve reading anything in as a batch operating, parsing it, filling out structures and _then_ having it for access rather than lib/Object having pointers to everything.

Another thought is to clean and layer the code a little better so that an individual class is abstracted out a bit both up and down the layers. Rafael, any ideas here?

-eric

_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits