[PATCH] D76252: [lld-macho] Add basic support for linking against dylibs

[Jez Ng via Phabricator via llvm-commits]

int3 marked 2 inline comments as done.
int3 added inline comments.


================
Comment at: lld/MachO/Arch/X86_64.cpp:38
   default:
-    error("TODO: Unhandled relocation type");
+    error("TODO: Unhandled relocation type " + std::to_string(type));
     return 0;
----------------
MaskRay wrote:
> ruiu wrote:
> > MaskRay wrote:
> > > This message enhancement should be moved to the initial patch.
> > Yeah, but I'm not worried about the exact order and the contents of initial patch series, as this is going to be submitted as soon as the first patch is submitted, and no one will be using the feature only with the first patch, so I think I'm fine with this.
> I would argue that when sending patches as a series, we should make extra efforts keeping each commit clean and avoiding unneeded code move if possible...
> 
> I think the this patch is good in its current status but will hope Jez can make some final cleanups to avoid subsequent formatting/cleanup changes.
no worries, wasn't hard to split out


================
Comment at: lld/MachO/Target.h:18
 enum {
+  WordSize = 8,
   PageSize = 4096,
----------------
ruiu wrote:
> Are you going to make it work only for 64-bit platforms? IIRC, both macOS and iOS are dropping 32-bit app support, so it is probably a good choice to not support 32-bit, but just to confirm.
I'm not sure we're 100% not going to support it, but we're definitely prioritizing 64-bit for now


================
Comment at: lld/MachO/Writer.cpp:297
+    for (Reloc &r : sect->relocs) {
+      if (auto *s = r.target.dyn_cast<Symbol *>()) {
+        if (auto dylibSymbol = dyn_cast<DylibSymbol>(s))
----------------
ruiu wrote:
> I wonder if you directly dyn_cast to `DylibSymbol *` as `r.target.dyn_cast<DylibSymbol *>()`.
just tried, doesn't work sadly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76252