[PATCH] D97438: [lld-macho] Change loadReexport to handle the case where a TAPI re-exports to reference documents nested within other TBD.

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo marked 13 inline comments as done.
oontvoo added inline comments.


================
Comment at: lld/test/MachO/reexport-nested-lib.s:5
+# RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %s -o %t/test.o
+# RUN: %lld -o %t/test  -syslibroot %S/Inputs/iPhoneSimulator.sdk -lSystem %t/test.o
+
----------------
int3 wrote:
> int3 wrote:
> > int3 wrote:
> > > I like to 
> > lt doesn't seem like libReexportSystem is actually being used in this test
> > 
> > Also, there doesn't seem to be a need to use the iPhoneSimulator SDK over the macOS SDK here -- `%lld` defaults to include the macOS SDK as syslibroot, so it's a bit more convenient to use that.
> > 
> > Another nit: I would rather libReexportSystem to not be placed under `/usr/lib` -- but I acknowledge there's no strong rationale here, I just like our fake `/usr/lib` to look as much like a realistic `/usr/lib` as possible. Personally I would place that tbd directly under `%S/Inputs`.
> > 
> > Finally, I think it's nice to test that the binary contains the right symbol binding via `llvm-objdump --macho --bind`, rather than just assuming it from a successful LLD exit code.
> > I like to 
> 
> sorry, stray comment, ignore it
> lt doesn't seem like libReexportSystem is actually being used in this test
> 
> Also, there doesn't seem to be a need to use the iPhoneSimulator SDK over the macOS SDK here -- `%lld` defaults to include the macOS SDK as syslibroot, so it's a bit more convenient to use that.
> 
> Another nit: I would rather libReexportSystem to not be placed under `/usr/lib` -- but I acknowledge there's no strong rationale here, I just like our fake `/usr/lib` to look as much like a realistic `/usr/lib` as possible. Personally I would place that tbd directly under `%S/Inputs`.
> 
> Finally, I think it's nice to test that the binary contains the right symbol binding via `llvm-objdump --macho --bind`, rather than just assuming it from a successful LLD exit code.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97438