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

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/Driver.cpp:116
+
+    inputFiles.push_back(make<ArchiveFile>(file.release()));
+    break;
----------------
Ktwu wrote:
> smeenai wrote:
> > 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?
> I originally had it so that ArchiveFile owned the input archive, but @int3 mentioned passing around unique_ptr is weird, so I made this change and figured leaking was acceptable given the globals in the linker :(
> 
> 
I said passing around unique_ptr is weird if the callee isn't changing ownership... which it wasn't :) (I think the first iteration of the diff was actually allowing the input archive to be freed prematurely... didn't notice that the first time around)

But yeah, we should follow what LLD-ELF is doing here -- take the Archive by rvalue reference to unique_ptr, and store it in a unique_ptr member in ArchiveFile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78342