[PATCH] D79668: [lld-macho] Support -order_file

Jez Ng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 18 16:17:32 PDT 2020


int3 marked 2 inline comments as done.
int3 added inline comments.


================
Comment at: lld/MachO/Driver.cpp:131
 
+static std::vector<StringRef> archNames{"arm",    "arm64", "i386",
+                                        "x86_64", "ppc",   "ppc64"};
----------------
smeenai wrote:
> I *think* this'll end up creating a global constructor, which is unfortunate. Would a `std::array` suffice, either globally or as a function `static`? I think it'd let us avoid that. Or even an `llvm::ArrayRef`, perhaps? (Haven't thought about that too much.)
> 
> Also, I'm assuming the DenseSet can't just be directly constructed with the names :(
DenseSet can be directly constructed; this is defined outside `isArchName` so that the error message can list out all the valid archs


================
Comment at: lld/MachO/Driver.cpp:182
+    case 2:
+      objectFile = fields[0];
+      symbol = fields[1];
----------------
smeenai wrote:
> This could either be the object file or the architecture. We'll need some additional checking here (I think assuming it's an architecture if it doesn't end with `.o` will just do the right thing, cos then the later check will bail out).
oh yeah, good catch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79668





More information about the llvm-commits mailing list