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

Alexander Shaposhnikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 14 12:24:36 PDT 2020


alexshap marked an inline comment as done.
alexshap added inline comments.


================
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;
----------------
smeenai wrote:
> 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.
thank you for review!
relocation_info appears to break the pattern used across the codebase
when we populate a struct and then if necessary call  MachO::swapStruct (there is no "swapStruct" for relocation_info), it seems to me it would make things inconsistent / a tiny bit harder to reason about.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77844





More information about the llvm-commits mailing list