[PATCH] D124895: [Object] Fix updating darwin archives

Thu May 5 23:05:58 PDT 2022

jhenderson added inline comments.

================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:117

+bool NewArchiveMember::isMacho() const {
+  auto Magic = identify_magic(Buf->getBuffer());
----------------
keith wrote:
> jhenderson wrote:
> > I feel like this function really belongs as a free function in either the BinaryFormat or Object libraries. If I'm not mistaken, the fact that the object is an archive member is irrelevant to this function?
> Yes any buffer could be used for this function. I added it here thinking that if there wasn't already a use case for this logic out there it might make sense to keep it localized until it's needed elsewhere, also now since it's called from 2 external users instead of only here. I'm happy to move it to a free function in the Object library if you think I should!
I think I'm okay with it being a function contained within this file, if it's not useful outside the scope of archive writing, but I think it makes more sense sitting somewhere alongside Mach-O code rather than archive code.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124895