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

James Y Knight via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 11 07:57:13 PDT 2022


jyknight added a comment.

Couple more comments, but looks pretty good.



================
Comment at: llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp:487-490
+      auto Kind = (*ArOrErr)->kind();
+      if (Kind == object::Archive::K_BSD && !NewArchiveMembersOrErr->empty() &&
+          isMacho(*NewArchiveMembersOrErr->front().Buf))
+        Kind = object::Archive::K_DARWIN;
----------------
This ought to specify K_DARWIN unconditionally. An archive inside a Mach-O universal binary wrapper has no reason to be in other formats.


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:973-980
+  if (Kind == object::Archive::K_BSD) {
+    if (NewMembersP && !NewMembersP->empty() &&
+        llvm::object::isMacho(*NewMembersP->front().Buf))
+      Kind = object::Archive::K_DARWIN;
+    else if (!NewMembers.empty() &&
+             llvm::object::isMacho(*NewMembers.front().Buf))
+      Kind = object::Archive::K_DARWIN;
----------------
The special-casing only needs to apply in the "else if (OldArchive)" under "case Default" above.

1. If "--format=bsd" was explicitly specified on the command-line, we shouldn't be overruling with a detected format.
2. In the other cases in the "if" chain within that, we've computed Kind from getKindFromMember, which already has logic that does something very similar (possibly even the same?

Actually, not quite sure, but maybe getKindFromMember's detection logic could be unified with the detection logic for isMachO?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124895



More information about the llvm-commits mailing list