[PATCH] D67008: [llvm-readobj][XCOFF]implement parsing relocation information for 32-bit xcoff objectfile

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added a comment.

Thanks @DiggerLin. I think this is almost ready.



================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:151
+
+  // Packed field, see XR_* masks for details of packing.
+  uint8_t Info;
----------------
Move the masks to the start of this class. Separate the nested types/constants from the fields using an access specifier label (e.g., `public`) or a comment.


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:156
+
+  bool isRelocationSigned() const;
+
----------------
Separate the fields from the methods using an access specified label or a comment.


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:158
+
+  // If the Fixup bit is set, it indicates that the linker has modified
+  // the instruction the relocation refers to.
----------------
Remove the comment and the blank line before it once the mask constants are defined in the class.


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:313
+  getLogicalNumberOfRelocationEntries(const XCOFFSectionHeader32 &Sec) const;
+  Expected<ArrayRef<XCOFFRelocation32>>
+  relocations(const XCOFFSectionHeader32 &) const;
----------------
I would prefer a blank line between multi-line function declarations.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67008