[PATCH] D78342: [lld] Add archive file support to Mach-O backend

[Kellie Medlin via Phabricator via llvm-commits]

Ktwu marked 7 inline comments as done.
Ktwu added inline comments.


================
Comment at: lld/MachO/Driver.cpp:113
+    if (!file->isEmpty() && !file->hasSymbolTable()) {
+      error(mbref.getBufferIdentifier() +
+            ": archive has no index; run ranlib to add one");
----------------
int3 wrote:
> s/mbref.getBufferIdentifier()/file->getName()/
> 
> nit: drop `if` curly braces
I copied this from the wasm driver :o

If we want to follow the ELF driver, I should use path instead....


================
Comment at: lld/MachO/InputFiles.h:79
+private:
+  llvm::DenseSet<uint64_t> seen;
+};
----------------
int3 wrote:
> would be nice to have a comment here about what the uint64_t represents
ELF's ArchiveFile doesn't list a comment either, but I'll add a comment; these are address offsets into the archive, which are used to map whether we've loaded a child of the archive yet. 


================
Comment at: lld/test/MachO/symbol-order.s:2-5
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %S/Inputs/g.s -o %tg.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %S/Inputs/f1.s -o %tf1.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %S/Inputs/f2.s -o %tf2.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %S/Inputs/f2g.s -o %tf2g.o
----------------
int3 wrote:
> nit: the `Inputs` folder is shared by all tests, so it'll be cleaner if we namespace them. I.e. `symbol-order-g.s` instead of `g.s`
> 
> also personally I like to run tests with multiple outputs in their own directory (i.e. `mkdir -p %t` would be the first run command and all files would be of the form %t/foo.o) but either way is fine
Both ideas make sense; I didn't like the weird concatenation here at all...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78342