[PATCH] D103746: [lld][MachO] Add support for $ld$install_name symbols

[Jez Ng via Phabricator via llvm-commits]

int3 accepted this revision.
int3 added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: lld/MachO/InputFiles.cpp:987
 
-  StringRef installName;
-  StringRef compatVersion;
-  StringRef platformStr;
-  StringRef startVersion;
-  StringRef endVersion;
-  StringRef symbolName;
-  StringRef rest;
-
-  std::tie(installName, name) = name.split('$');
-  std::tie(compatVersion, name) = name.split('$');
-  std::tie(platformStr, name) = name.split('$');
-  std::tie(startVersion, name) = name.split('$');
-  std::tie(endVersion, symbolName) = name.split('$');
-  std::tie(symbolName, rest) = symbolName.split('$');
-  // TODO: ld64 contains some logic for non-empty symbolName as well.
-  if (!symbolName.empty())
-    return true;
-  unsigned platform;
-  if (platformStr.getAsInteger(10, platform) ||
-      platform != static_cast<unsigned>(config->platform()))
-    return true;
+  if (action == "previous") {
+    StringRef installName;
----------------
how about splitting out each action handler into its own function?


================
Comment at: lld/MachO/InputFiles.cpp:1040
+    std::tie(condition, installName) = name.split('$');
+    llvm::VersionTuple version;
+    if (!condition.startswith("os") ||
----------------
no need for llvm::


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103746