[PATCH] D66070: [Object] Create MutableELFObject Class for Doing Mutations on ELFObjectFiles [Part 6]

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Posting this here since I've not seen a patch for Part 7 yet. I may have said this elsewhere already, but I don't think it's a good idea to have sections as members of segments. I think you'll find the layout algorithm easier to work with when using a parent relationship, rather than a child one. Note that this is how llvm-objcopy is already written. That should allow you to handle multiple levels of nesting/overlapping etc without too much difficulty. That would also, I believe, make the Segment and Section implementations much closer to each other.



================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:302
+  public:
+    SegmentRef(MutableELFObject<ELFT> *Object, uint64_t Index = 0)
+        : Object(Object), Index(Index) {}
----------------
abrachet wrote:
> MaskRay wrote:
> > You can use `uint64_t Index = 0; ` (did you mean `size_t Index = 0;`?) and delete the ctor.
> Hmm am I missing something on how to construct objects? C++ doesn't have the nice C syntax like `SegmentRef S = {.Object = nullptr, .Index = 0};` Is there a way to leave out these boiler plate constructors that I don't know of?
Do you ever need the default value of Index? It seems to me to be unnecessary.


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

https://reviews.llvm.org/D66070