[PATCH] D85255: [lld-macho] Generate ObjC symbols from .tbd files

Saleem Abdulrasool via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 5 08:47:10 PDT 2020


compnerd added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:394
   dylibName = saver.save(interface->getInstallName());
+  auto addSymbol = [&](const Twine &name) -> void {
+    symbols.push_back(symtab->addDylib(saver.save(name), umbrella,
----------------
Does this really need the full context?


================
Comment at: lld/MachO/InputFiles.cpp:421
+      break;
+    }
+  }
----------------
Same as last time - would be nice to actually compose with the user label prefix.

Remember that `x86_64-apple-coff` is a real target - and the user label prefix is empty there.  This breaks because you are assuming that the user label prefix is `_`.


================
Comment at: lld/test/MachO/Inputs/MacOSX.sdk/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation.tbd:13
+    objc-ivars:    [ NSConstantArray._count ]
+    objc-eh-types: [ NSException ]
----------------
Id really rather see a second library added (`Foundation.tbd`) rather than put this into `CoreFoundation`.  This makes it confusing for others IMO.


================
Comment at: lld/test/MachO/stub-link.s:6
 # RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %s -o %t/test.o
-# RUN: lld -flavor darwinnew -o %t/test -Z -L%S/Inputs/MacOSX.sdk/usr/lib -lSystem %t/test.o
+# RUN: lld -flavor darwinnew -o %t/test -syslibroot %S/Inputs/MacOSX.sdk -lSystem -framework CoreFoundation %t/test.o
 #
----------------
I think that we should go through and actually improve all the tests to use `-syslibroot`.  This really looks far better (and matches the real world usage).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85255



More information about the llvm-commits mailing list