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

[Digger via Phabricator via llvm-commits]

DiggerLin marked 4 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:151
+
+  // Packed field, see XR_* masks for details of packing.
+  uint8_t Info;
----------------
hubert.reinterpretcast wrote:
> 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.
changed a suggestion.


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


================
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.
----------------
hubert.reinterpretcast wrote:
> Remove the comment and the blank line before it once the mask constants are defined in the class.
changed as suggestion


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


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67008