[PATCH] D78342: [lld] Add archive file support to Mach-O backend
Jez Ng via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu May 14 13:03:48 PDT 2020
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
More information about the llvm-commits
mailing list