[PATCH] D81295: lld: initial pass at supporting TBD

[Kellie Medlin via Phabricator via llvm-commits]

Ktwu added inline comments.


================
Comment at: lld/test/MachO/iPhoneSimulator.sdk/usr/lib/libSystem.tbd:1
+--- !tapi-tbd-v3
+archs:            [ i386, x86_64 ]
----------------
compnerd wrote:
> Ktwu wrote:
> > It's nice to know where these files came from, but there's not a need to create a full copy that even preserves the path, right? If anything, you should be able to combine the two stub files into one manually and use them both for your test.
> > 
> > 
> > For example, you don't use __crashreporeter_info__, so get rid of it.
> Keeping them separate is important IMO, and far more useful.  This is going to grow over time, and get used for future tests as the functionality improves.  The paths here are not indicative of where they originate from - the UUIDs are pretty obviously sequential - as these are fabricated.  The versions happen to be cause I had a tab open to some revision of opensource.apple.com.
> 
> The single symbol there is there because its useful for testing symbol origins from the parent umbrella vs re-exported dylibs.
> This is going to grow over time, and get used for future tests as the functionality improves.

Hmmm, I was under the impression that it's better to be intentional in testing for supported features (don't add red herrings in tests that could give a reader a false impression of which scenarios are tested). 

> The single symbol there is there because its useful for testing symbol origins from the parent umbrella vs re-exported dylibs.

But this isn't actively tested :(

> The versions happen to be cause I had a tab open to some revision of opensource.apple.com

Ok, since the paths aren't really important, can you flatten them and stick them into lld/test/Macho/Inputs?


================
Comment at: lld/test/MachO/stub-link-2.s:1
+# REQUIRES: x86
+
----------------
compnerd wrote:
> Ktwu wrote:
> > stub-link-2 isn't descriptive. Move this into /test/MachO/invalid and give it a better name like "undefined-stub-symbol"?
> Its not really an invalid test, its a negative test case.
"invalid" is where negative / error-based tests live for lld (or ought to live, there are probably a few exceptions from before thinking such a place was a good idea).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81295