[PATCH] D76908: [lld-macho] Add support for emitting dylibs with a single symbol

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added a comment.

Nice!



================
Comment at: lld/MachO/SyntheticSections.cpp:142
+  for (const Symbol *sym : symtab->getSymbols())
+    if (auto *defined = dyn_cast<Defined>(sym))
+      exported.push_back(defined);
----------------
This can be done in a follow-up, but we should be checking if the symbol has hidden visibility before exporting it, right?


================
Comment at: lld/MachO/SyntheticSections.h:26
 constexpr const char *pageZero = "__pagezero";
+constexpr const char *export_ = "__export";
 constexpr const char *stringPool = "__string_pool";
----------------
Nit: why the trailing underscore?


================
Comment at: lld/include/lld/Common/Sort.h:1
+//===- Sort.h ---------------------------------------------------*- C++ -*-===//
+//
----------------
Was this meant to be part of a previous diff?


================
Comment at: lld/test/MachO/dylib.s:6
+# RUN:   %t.o -o %t.dylib
+# CHECK:     cmd: LC_ID_DYLIB
+# CHECK-NOT: cmd:
----------------
Where's the corresponding FileCheck for these?


================
Comment at: lld/test/MachO/dylib.s:10
+
+## Check that -e is a no-op when -dylib is also passed.
+# RUN: lld -flavor darwinnew -dylib %t.o -o %t.dylib -e missing_entry
----------------
Which part of the test is checking this?


================
Comment at: lld/test/MachO/dylib.s:12
+# RUN: lld -flavor darwinnew -dylib %t.o -o %t.dylib -e missing_entry
+# RUN: obj2yaml %t.dylib | FileCheck %s -DOUTPUT=%t.dylib --check-prefix=DEFAULT-INSTALL-NAME
+# DEFAULT-INSTALL-NAME:     cmd: LC_ID_DYLIB
----------------
Is it possible to test these via objdump or readobj instead of obj2yaml? That's more conventional in LLD tests, and their output is probably a bit more stable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76908