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

[Shoaib Meenai via Phabricator via llvm-commits]

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

LGTM



================
Comment at: lld/MachO/InputFiles.cpp:421
+      break;
+    }
+  }
----------------
int3 wrote:
> smeenai wrote:
> > compnerd wrote:
> > > smeenai wrote:
> > > > int3 wrote:
> > > > > compnerd wrote:
> > > > > > 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 `_`.
> > > > > > Remember that x86_64-apple-coff is a real target
> > > > > 
> > > > > oh what... is this actually used?
> > > > I believe it's used for UEFI. Presumably that's something for LLD for COFF to worry about and not LLD for Mach-O though, right? Also, I'd be surprised if Objective-C is being used for UEFI :)
> > > Sorry, that was a partial thought.  What I was getting at is that the user label prefix is important and in that case, you have MachO conversions that occur.  We should be more careful about the user label prefix rather than assume that it is guaranteed to be `_`.  That is a convention that is target dependent and not MachO dependent.
> > I agree with you in theory, but in practice, are there any targets we'll be linking with LLD for Mach-O where you wouldn't expect the underscore? I think we're pretty much assuming that Mach-O == an Apple Mach-O target; we're certainly not testing with anything else.
> > 
> > Out of curiosity, what does ld64 do?
> ld64 includes the underscore prefixes in their string literals
I think it's pretty justifiable to use the underscore directly as well here then.


================
Comment at: lld/test/MachO/Inputs/MacOSX.sdk/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation.tbd:13
+    objc-ivars:    [ NSConstantArray._count ]
+    objc-eh-types: [ NSException ]
----------------
int3 wrote:
> compnerd wrote:
> > Id really rather see a second library added (`Foundation.tbd`) rather than put this into `CoreFoundation`.  This makes it confusing for others IMO.
> these members are from CoreFoundation in the actual SDK, though...
Huh, TIL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85255