[PATCH] D77844: [ObjectYAML][MachO] Add support for relocations

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added a comment.

The Mach-O bits look good to me. I'd be happy if someone more familiar with the YAML bits could take a look at those as well, but they seem straightforward enough.



================
Comment at: llvm/include/llvm/ObjectYAML/MachOYAML.h:31
+struct Relocation {
+  // Offset in the section to what is being  relocated.
+  llvm::yaml::Hex32 address;
----------------
Nit: change two spaces to one between "being" and "relocated"


================
Comment at: llvm/lib/ObjectYAML/MachOEmitter.cpp:266
+  assert(!R.is_scattered && "non-scattered relocation expected");
+  MachO::any_relocation_info MRE;
+  MRE.r_word0 = R.address;
----------------
Any reason to prefer `any_relocation_info` to `relocation_info`? Assigning to the individual fields in it would be much clearer than all the shifting and bit arithmetic below IMO.


================
Comment at: llvm/lib/ObjectYAML/MachOEmitter.cpp:282
+  assert(R.is_scattered && "scattered relocation expected");
+  MachO::any_relocation_info MRE;
+  const unsigned FixupOffset = R.address;
----------------
Same here, but with `scattered_relocation_info`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77844