[PATCH] D76252: [lld-macho] Add basic support for linking against dylibs

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/MachO/Arch/X86_64.cpp:38
   default:
-    error("TODO: Unhandled relocation type");
+    error("TODO: Unhandled relocation type " + std::to_string(type));
     return 0;
----------------
This message enhancement should be moved to the initial patch.


================
Comment at: lld/test/MachO/Inputs/goodbye-dylib.yaml:1
+# This yaml file was originally generated from linking the following source
+# input with ld64:
----------------
smeenai wrote:
> Description for hello and goodbye seems swapped.
Use `## ` for comments and `# ` for RUN and CHECK lines.


================
Comment at: lld/test/MachO/Inputs/goodbye-dylib.yaml:108
+    tocoff:          0
+    ntoc:            0
+    modtaboff:       0
----------------
Are all these fields important? I think we probably should think what fields are optional and improve yaml2obj. Otherwise the verbosity can make tests a lot more complex.


================
Comment at: lld/test/MachO/dylink.s:3
+# RUN: mkdir -p %t
+# RUN: yaml2obj %p/Inputs/hello-dylib.yaml > %t/libhello.dylib
+# RUN: yaml2obj %p/Inputs/goodbye-dylib.yaml > %t/libgoodbye.dylib
----------------
`>` -> `-o`


================
Comment at: lld/test/MachO/dylink.s:7
+# RUN: lld -flavor darwinnew -o %t/dylink -Z -L%t -lhello -lgoodbye %t/dylink.o
+# RUN: (obj2yaml %t/dylink; llvm-objdump -d %t/dylink) | FileCheck %s
+
----------------
Should obj2yaml and llvm-objdump -d tests be separated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76252