[PATCH] D145263: [lld-macho] Remove duplicate minimum version info (NFC)

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/Driver.cpp:686-687
 // Has the side-effect of setting Config::platformInfo.
-static PlatformType parsePlatformVersions(const ArgList &args) {
+static std::pair<PlatformVersion, std::optional<PlatformVersion>>
+parsePlatformVersions(const ArgList &args) {
   std::map<PlatformType, PlatformVersion> platformVersions;
----------------
can we set both `platformInfo` and `secondaryPlatformInfo` in this function, and make it return void? It doesn't seem like return them as values helps readability + I would like to avoid `std::pair` as much as possible (again for readability).

remember to update the "side-effect" comment above as well :)


================
Comment at: lld/MachO/Driver.cpp:688
+parsePlatformVersions(const ArgList &args) {
   std::map<PlatformType, PlatformVersion> platformVersions;
   const PlatformVersion *lastVersionInfo = nullptr;
----------------
while we're here, could we change this to a `DenseMap` or `IndexedMap`?

Doesn't really matter for perf but we try to avoid `std::map` in general. And this might save some binary size


================
Comment at: lld/test/MachO/tapi-link.s:20
+# INCOMPATIBLE-NEXT: error: {{.*}}libc++.tbd(/usr/lib/libc++.dylib) is incompatible with x86_64h (macOS11.0)
+# INCOMPATIBLE-NEXT: error: {{.*}}CoreFoundation.tbd(/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation) is incompatible with x86_64h (macOS11.0)
 
----------------
I think we should not put `NFC` in the commit title. Minor change but a change nonetheless...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145263