[PATCH] D119671: [ObjectYAML][MachO] Add basic chained fixups support

[Keith Smiley via Phabricator via llvm-commits]

keith added inline comments.


================
Comment at: llvm/lib/Object/MachOObjectFile.cpp:1384-1387
+      if ((Err = checkLinkeditDataCommand(
+               *this, Load, I, &DyldChainedFixupsLoadCmd,
+               "LC_DYLD_CHAINED_FIXUPS", Elements, "chained fixups data")))
+        return;
----------------
jhenderson wrote:
> I see this pattern is very common. I think it would be worth factoring out the repetivtive function call into a lambda, to save repeating most of the arguments (`*this`, `Load`, `I`, and `Elements` appear to be common to all `checkLinkeditDataCommand`). This might be worth doing as a prerequisite patch, and tidying up the `checkDyldInfoCommand` calls in the same manner. There might be other refactorings worth doing to reduce repetitiveness even further, but that's probably going a bit far.
Yea I can think about that as a follow up, it's a bit of a pain with how these string + constant representations are duplicated


================
Comment at: llvm/test/ObjectYAML/MachO/chained_fixups.yaml:183-184
+# RUN: obj2yaml %t | FileCheck %s
+# CHECK: ChainedFixups: [ 0x0, 0x0, 0x0, 0x0, 0x20, 0x0, 0x0, 0x0, 0x30, 0x0,
+# CHECK: DyldExportsTrie: [ 0x0, 0x1,
----------------
jhenderson wrote:
> These look incomplete to me?
Yea I was intentionally adding just enough to make sure something came through here, but I can verify 1:1 to be safe


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119671