[PATCH] D123277: [lld-macho][nfc] De-templatize UnwindInfoSection

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added inline comments.


================
Comment at: lld/MachO/UnwindInfoSection.cpp:106-123
+#define SET_OFFSET(Ptr, x) this->x = offsetof(Layout<Ptr>, x)
+  CompactUnwindOffsets(size_t wordSize) {
+    if (wordSize == 8) {
+      SET_OFFSET(uint64_t, functionAddress);
+      SET_OFFSET(uint64_t, functionLength);
+      SET_OFFSET(uint64_t, encoding);
+      SET_OFFSET(uint64_t, personality);
----------------
nit: I'm not a big fan of macros and all these lines are quite repetitive,  so i'm wondering if maybe we could shorten them to a helper function like this ^
(technically shorter :) )






================
Comment at: lld/MachO/UnwindInfoSection.cpp:107
+#define SET_OFFSET(Ptr, x) this->x = offsetof(Layout<Ptr>, x)
+  CompactUnwindOffsets(size_t wordSize) {
+    if (wordSize == 8) {
----------------
👍  nicer than the previous struct (i.e hide the template stuff)


================
Comment at: lld/MachO/UnwindInfoSection.cpp:171
+  uint64_t unwindInfoSize = 0;
+  std::vector<decltype(symbols)::value_type> symbolsVec;
+  CompactUnwindOffsets cuOffsets;
----------------
Why do you need another symbolsVec? (shouldh't this be inheritted from UnwindInfoSection?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123277