[PATCH] D63843: [Object][XCOFF] Add support for 64-bit file header and section header dumping.

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:49
 
+static uintptr_t getWithOffset(uintptr_t Base, ptrdiff_t Offset) {
+  return reinterpret_cast<uintptr_t>(reinterpret_cast<const char *>(Base) +
----------------
MaskRay wrote:
> `getWithOffset` does not seem useful: it just does a plus operation.
The function helps the code be more self-documenting about the intended semantics and is much more greppable than raw arithmetic. This allows pointer arithmetic to be properly performed in pointer space. Although not all such arithmetic is done in pointer space currently, that does not mean that we shouldn't keep the instances where they are.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:83
+#endif
+  return viewAs<XCOFFSectionHeader32>(Ref.p);
+}
----------------
MaskRay wrote:
> I think inline reinterpret_cast may be clearer, then the `viewAs` helper can be deleted.
The `viewAs` helper is not new to this patch. Additionally, it saves on verbosity and noise that is distracting. Familiarity with `viewAs` is not difficult to attain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63843