[PATCH] D97867: [lld-macho] Filter TAPI re-exports by target

[Jez Ng via Phabricator via llvm-commits]

int3 marked an inline comment as done.
int3 added inline comments.


================
Comment at: lld/MachO/Config.h:87
+
+  llvm::MachO::Target target() const {
+    return llvm::MachO::Target(arch, platform.kind);
----------------
oontvoo wrote:
> nit: could this be called `makeTarget`? `target()` alone sounds like an accessor, which it technically isn't.
> 
> Otherwise (keeping the name), can it be cached/shared? How many time does it need to create a target? 
> 
> (It seems from the usages below, we only need it for comparison, so it seems safe to do that to me)
Making it sound like an accessor was intentional ;) `Target`'s constructor is basically a no-op, so `target()` was just constructing a pair of two enums, which is super cheap. But I get that it looks misleading / surprising at first glance, so I've cleaned it up.


================
Comment at: lld/MachO/Driver.cpp:586
+// Has the side-effect of setting Config::target.
+static TargetInfo *createTargetInfo(opt::InputArgList &args) {
+  StringRef archName = args.getLastArgValue(OPT_arch);
----------------
I moved the definition of this function down since it now invokes `parsePlatformVersion`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97867