[PATCH] D75131: [llvm-objdump][XCOFF][AIX] Implement -r option

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:252
+  static constexpr uint64_t InvalidRelocOffset =
+      std::numeric_limits<uint64_t>::max();
+
----------------
MaskRay wrote:
> hubert.reinterpretcast wrote:
> > `0xffffffff'ffffffffULL`
> Will UINT64_C(-1) be clearer?
You mean `-UINT64_C(1)`? I don't think so. My experience is that people are not familiar with these macros.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:16
 #include <cstring>
+#include <limits>
+
----------------
@jasonliu: Is the `<limits>` inclusion still needed?


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:22
+enum : uint16_t { NO_REL_MASK = 0x0001 };
+} // namespace
 
----------------
jasonliu wrote:
> MaskRay wrote:
> > Is the anonymous namespace useful?
> I think it's good for internal linkage, and make it unreachable from other translation units.
Please refer to
https://github.com/cplusplus/draft/commit/d942a391d8faa999cb643d98757b15348fe88bb4



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

https://reviews.llvm.org/D75131