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

[Digger via Phabricator via llvm-commits]

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


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:168
+
+bool isRelocationSigned(XCOFFRelocation32 &Reloc);
+
----------------
hubert.reinterpretcast wrote:
> sfertile wrote:
> > hubert.reinterpretcast wrote:
> > > DiggerLin wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > Do these need to be declared in the header? Are they called only in one `.cpp` file? If so, they can be made `static` in the `.cpp` file. Otherwise, it seems odd that these aren't `const` member functions of `XCOFFRelocation32`.
> > > > the llvm-readobj is using those function and obj2yaml will use them too.
> > > It is still odd to me that these aren't `const` non-static member functions of `XCOFFRelocation32`.
> > I think were these originally templated to work with both 32-bit and 64-bit relocations, which explains why they aren't member functions.
> Would using CRTP with a base class template work for that case?
as Sean's comment, for we only implement 32 bits relocation, we do not use any template for the relocation implement this moment. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67008