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

[Alex Brachet via Phabricator via llvm-commits]

abrachet marked 14 inline comments as done.
abrachet added a comment.

Ahh sorry about all those unrelated changes! Not sure what happened there probably a bad rebase like you said.



================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:331
+
+    bool operator==(const SegmentRef &Other) const {
+      return std::tie(Object, Index) == std::tie(Other.Object, Other.Index);
----------------
jhenderson wrote:
> What is the point of this and the less-than operator?
They are needed by `content_iterator` same with `moveNext`


================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:347-353
+    bool hasSection(SectionRef Sec) const {
+      return Object->segmentHasSection(Index, Sec.getRawDataRefImpl().p);
+    }
+
+    void addSection(SectionRef Sec) {
+      return Object->segmentAddSection(Index, Sec.getRawDataRefImpl().p);
+    }
----------------
jhenderson wrote:
> I'm not convinced this patch should attempt to add sections to segments. The reason for this is because they're not an intrinsic part of segments, but rather an artefact of the layout of an ELF object. Relatedly, this doesn't handle segments within other segments, so feels incomplete. I'd defer it to a patch that handles laying out an ELF.
Ok, I'll move it out to a part 7.


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

https://reviews.llvm.org/D66070