[PATCH] D88400: [llvm-objcopy][MachO] Add support for universal binaries
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 29 00:40:27 PDT 2020
jhenderson added a comment.
Just a few minor comments from me, but I don't really follow the logic as always, so this will need others to comment on it.
================
Comment at: llvm/test/tools/llvm-objcopy/MachO/universal-object.test:31
+## Case 4: try to copy a universal object file contaning a bitcode slice.
+# RUN: echo 'target triple = "arm64-apple-ios8.0.0"' >%t.arm64.ll
+# RUN: llvm-as %t.arm64.ll -o %t.bitcode
----------------
You could also just inline this text directly in the file and feed `%s` as the input to `llvm-as`.
================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:423
+ }
+ consumeError(ArOrErr.takeError());
+
----------------
Here and the other `consumeError` site probably deserve comments explaining why it's okay to throw away the error reported.
================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:429
+ return createStringError(std::errc::invalid_argument,
+ "Universal Mach-O object '%s' contains a slice "
+ "which is not a Mach-O object or an archive",
----------------
Preferred error message style starts with a lower-case letter.
Is there anything that allows us to uniquely identify which slice is the problematic one (e.g. a member name or similar)? If so, it might be helpful to report it as part of the message.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88400/new/
https://reviews.llvm.org/D88400
More information about the llvm-commits
mailing list