[PATCH] D64281: [Object] Create MutableELFObject Class for Doing Mutations on ELFObjectFiles [Part 1]

[Alex Brachet via Phabricator via llvm-commits]

abrachet added a comment.

(won't quote because of length but I'll tag you so you know I am responding) @jhenderson 
That does work, and was what I was thinking of before. Also I quite like keeping pointers so it just works with the methods in the base class. But I'm still not sure what type `T` should be. If we use sections as an example, when changing the name it isn't possible (or feasible, I suppose that `sh_name` could be a pointer (not into the string table but a pointer into the processes memory) for modified sections). Right now, the original type is just an `Elf_Shdr`, or rather the mapping type points to one, but for a modified section, it indexes into the vector where to find the `T`, which is a wrapper around an `Elf_Shdr` but for example for its name it has a `StringRef` so it can be easily changed. This is how llvm-objcopy does it, but it copies every single section whether or not they will ever be modified. I agree that it is ugly for the users of the class to ever have to deal with the MappingType, it isn't ideal, but I don't know how to do this unless we copy every single one like is done in llvm-objcopy, and at that point I would suggest the Section type just have an `Elf_Shdr *` to the original and just keep a `vector` of Sections.

Something I played around with before was having a template parameter to the original type and then having methods with return `OrigType &` and require that `T` have a conversion function to 'OrigType`. This would be cleaner for the user but also avoid copying every original type into the wrapper type `T`. This is all moot if you don't think that copying is that big of a bottle neck, @MaskRay   mentioned this. After all this is how its done for sections in llvm-objcopy. But I am still shooting for being able to use this class with symbols too so there may be more concern about this there.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64281/new/

https://reviews.llvm.org/D64281