[PATCH] D112756: [lld-macho] -all_load and -ObjC should not affect LC_LINKER_OPTION flags

[Nico Weber via Phabricator via llvm-commits]

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

Nice!



================
Comment at: lld/MachO/Driver.cpp:271
       }
-    } else if (config->forceLoadObjC) {
+    } else if (config->forceLoadObjC && forceLoadArchive != ForceLoad::No) {
       for (const object::Archive::Symbol &sym : file->getArchive().symbols())
----------------
nit: I'd s/!= ForceLoad::No/s == ForceLoad::Default/ since then the reader doesn't have to read line 257 to realize that ::Yes never comes here and hence is uninteresting


================
Comment at: lld/test/MachO/lc-linker-option.ll:66
+
+;; Make sure -all_load and -ObjC have no effect on LC_LINKER_OPTION flags.
+; RUN: llc %t/load-library-foo.ll -o %t/load-library-foo.o -filetype=obj
----------------
nit: I'd find "have no effect on libraries loaded via LC_LINKER_OPTION flags" clearer


================
Comment at: lld/test/MachO/lc-linker-option.ll:68
+; RUN: llc %t/load-library-foo.ll -o %t/load-library-foo.o -filetype=obj
+; RUN: llvm-ar rcs %t/libfoo.dylib %t/foo.o
+; RUN: %lld -all_load -ObjC %t/load-framework-foo.o %t/load-library-foo.o \
----------------
nit: it's a bit weird to call a static library .dylib -- why not create a real dylib with lld? (up to you though)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112756