[PATCH] D116913: [lld-macho] Add --start-lib --end-lib

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added a comment.

In D116913#3246384 <https://reviews.llvm.org/D116913#3246384>, @MaskRay wrote:

> - Support -ObjC
> - remove `inLib`
>
> I keep my viewpoint that --start-lib is more suitable because of (b) (as stated in the description).
> The library boundary is useful and -objlib would lose the information.
>
> --whole-archive (-force_load), while similar, keeps the library boundary information,
> so it's not the best analogy.

Ok, I think i'm convinced --start-lib/--end-lib is better in this case :)



================
Comment at: lld/MachO/Driver.cpp:350
   if (newFile && !isa<DylibFile>(newFile)) {
+    if ((magic == file_magic::macho_object || magic == file_magic::bitcode) &&
+        newFile->lazy && config->forceLoadObjC) {
----------------
int3 wrote:
> int3 wrote:
> > nit: how about `isa<ObjFile>` and `isa<BitcodeFile>` since the line above is also using `isa<>`?
> IIUC, we are evaluating lazy object files for ObjC members after doing regular symbol resolution. Archive loading (intentionally) does this in the opposite order: {D108781}. Would probably be good to follow suit
is anything wrong with switching the previous predicate to checking the magic? (checking for the underlying type always seemed kind of iffy to me ... )


================
Comment at: lld/MachO/InputFiles.cpp:1619
+  symbols.resize(obj->symbols().size());
+  for (auto it : llvm::enumerate(obj->symbols())) {
+    const lto::InputFile::Symbol &objSym = it.value();
----------------
int3 wrote:
> no need for `llvm::`
Sorry, I'm missing something here but why not simply ` for (const lto::InputFile::Symbol &objSym : obj->symbols())` ?
IOWs, why is `llvm::enumerate` needed?


================
Comment at: lld/test/MachO/objc.s:34
 # OBJC-NEXT:  g     F __TEXT,__text _main
-# OBJC-NEXT:  g     F __TEXT,__text _OBJC_CLASS_$_MyObject
+# OBJC:       g     F __TEXT,__text _OBJC_CLASS_$_MyObject
 
----------------
`OBJC-DAG` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116913