[PATCH] D87929: [lld-macho] Support -weak_lx, -weak_library, -weak_framework

[Greg McGary via Phabricator via llvm-commits]

gkm added inline comments.


================
Comment at: lld/MachO/Driver.cpp:603-608
     case OPT_INPUT:
+    case OPT_weak_library:
       addFile(arg->getValue());
+      if (opt.getID() == OPT_weak_library)
+        weakImportPaths.insert(arg->getValue());
       break;
----------------
IMO, there is no benefit in combining these cases, and a deficit in clarity.
By contrast, the other two `OPT_weak_l` and `OPT_weak_framework` do benefit from combining, since they have surrounding context that need not be duplicated.


================
Comment at: lld/MachO/Driver.cpp:672-676
+  for (InputFile *file : inputFiles)
+    if (auto *dylibFile = dyn_cast<DylibFile>(file))
+      if (weakImportPaths.contains(file->getName()))
+        dylibFile->forceWeakImport = true;
+
----------------
How about setting the `forceWeakImport` flag immediately after construction? Perhaps create `void addWeakFile(StringRef path)` and/or `void addFile(StringRef path, bool isWeak)` APIs around `void addFile(StringRef path)` that sets the flag when `addFlag()` returns a `DylibFile`. That way you don't need `weakImportPaths`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87929