Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR-953] Implement Bundle(for: class): Expose SPI to get type descriptor. #16964

Merged
merged 2 commits into from
Jun 12, 2018

Conversation

millenomi
Copy link
Contributor

@millenomi millenomi commented Jun 2, 2018

Per discussion with @jckarter — we would like to have a way, given a class, to get a symbol that originates in the same image (library/shared object/executable etc.) so we can implement Bundle(for class: AnyClass) in terms of dladdr(). The type descriptor apparently satisfies this condition for all classes that were compiled AOT.

This exposes a C SPI for swift-corelibs-foundation use to obtain the type descriptor given a type.

Resolves SR-953 alongside the patch at swiftlang/swift-corelibs-foundation#1573.

@millenomi
Copy link
Contributor Author

@swift-ci please test

@millenomi millenomi requested a review from jckarter June 2, 2018 16:38
#if !SWIFT_OBJC_INTEROP

SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_SPI // SPI for swift-corelibs-foundation
ConstTargetMetadataPointer<swift::InProcess, TargetTypeContextDescriptor>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should just return a void *, since (at least for now) the callers treat it as an opaque pointer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have it use const TypeContextDescriptor * instead of the Target template, since this will only ever be used in-process. I don't think there's any harm in having it return the real type here. I also don't think the leading underscore is necessary; IMO this makes sense as a general runtime entry point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll make those changes.

@millenomi
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

@jckarter jckarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@millenomi
Copy link
Contributor Author

@swift-ci please test and merge

@swift-ci swift-ci merged commit f65000a into swiftlang:master Jun 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants