This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Support larger dylib symbol ordinals in bindings
ClosedPublic

Authored by int3 on Aug 6 2020, 12:32 PM.

Details

Summary

Do folks care if we don't have a test for this? Creating 16
dylibs to trigger this straightforward code path seems a little tedious

Diff Detail

Event Timeline

int3 created this revision.Aug 6 2020, 12:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2020, 12:33 PM
int3 requested review of this revision.Aug 6 2020, 12:33 PM
smeenai added a subscriber: smeenai.Aug 6 2020, 7:34 PM

I'm not super opposed to not having a test. It doesn't seem too terrible to write one though, just repetitive: we could produce 15 dylibs from the same source file and just have the 16th be different (to confirm the binding), right?

int3 added a comment.Aug 7 2020, 10:13 AM

we could produce 15 dylibs from the same source file and just have the 16th be different (to confirm the binding), right?

yeah pretty much. It's just annoying that FileCheck doesn't provide a better way of testing things like this

smeenai accepted this revision.Aug 12 2020, 5:22 PM

we could produce 15 dylibs from the same source file and just have the 16th be different (to confirm the binding), right?

yeah pretty much. It's just annoying that FileCheck doesn't provide a better way of testing things like this

Agreed.

I'd prefer to still add the test, but LGTM either way.

This revision is now accepted and ready to land.Aug 12 2020, 5:22 PM
This revision was landed with ongoing or failed builds.Aug 12 2020, 7:51 PM
This revision was automatically updated to reflect the committed changes.