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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 19 23:03:50 PDT 2019


MaskRay added inline comments.


================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:72
+
+  MutableELFSegment(Elf_Phdr Header) : Header(Header) {}
+
----------------
The ctor is not really necessary.


================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:211
 
+    NewType *getIfNew(uint64_t Index) {
+      assert(Index < Mappings.size() && "Out of bounds");
----------------
This should be added to Part I D64281.


================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:302
+  public:
+    SegmentRef(MutableELFObject<ELFT> *Object, uint64_t Index = 0)
+        : Object(Object), Index(Index) {}
----------------
You can use `uint64_t Index = 0; ` (did you mean `size_t Index = 0;`?) and delete the ctor.


================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:424
+  /// Add segment \param Seg.
+  void addSegment(MutableELFSegment<ELFT> &&Seg) {
+    Segments.add(std::move(Seg));
----------------
Delete `&&`. See my comment in Part 4 D66062.


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

https://reviews.llvm.org/D66070





More information about the llvm-commits mailing list