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

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.

Looks great with the leak fixed!



================
Comment at: lld/MachO/Driver.cpp:116
+
+    inputFiles.push_back(make<ArchiveFile>(file.release()));
+    break;
----------------
So we're just leaking the memory for the Archive? That's not ideal.

LLD ELF has the `unique_ptr<object::Archive>` be a member of `ArchiveFile` and moves the pointer here into the constructor. Can we do the same?


================
Comment at: lld/test/MachO/archive.s:18
+
+## Linking with the archive first in the command line shouldn't change anything
+# RUN: lld -flavor darwinnew %t/test.a %t/main.o -o %t/test.out
----------------
int3 wrote:
> You could reuse the same CHECK commands instead of having a different prefix with the same checks -- that's what I did in my order file diff. But this is fine too
Yeah, what I was thinking was that you create two separate output files, one for the archive second case and one for the archive first case, and then run the same FileCheck commands for both (as in with the same check prefix (or lack thereof)). That way it's immediately obvious that both of them have the same output, and the VISIBLE-NOT tests cover both cases as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78342