[cxx-interop] Runtime support for foreign reference types. #59439
Conversation
zoecarver
force-pushed
the
frt-metatdata-runtime
branch
from
595f4ef to
f972f66
Compare
| @@ -53,6 +53,8 @@ NOMINALTYPEMETADATAKIND(Optional, 2 | MetadataKindIsNonHeap) | |||
| /// A foreign class, such as a Core Foundation class. | |||
| METADATAKIND(ForeignClass, 3 | MetadataKindIsNonHeap) | |||
|
|
|||
| METADATAKIND(ForeignReferenceType, 4 | MetadataKindIsNonHeap) | |||
There was a problem hiding this comment.
@rjmccall I have no idea what this value should be.
| @@ -1688,7 +1688,7 @@ namespace { | |||
| VTable(IGM.getSILModule().lookUpVTable(getType())), | |||
| Resilient(IGM.hasResilientMetadata(Type, ResilienceExpansion::Minimal)) { | |||
|
|
|||
| if (getType()->isForeign() || Type->isForeignReferenceType()) | |||
There was a problem hiding this comment.
(This was just a redundant check.)
lib/IRGen/GenMeta.cpp
Outdated
| @@ -1788,7 +1787,8 @@ namespace { | |||
| // is a foreign class. | |||
| if ((IGM.IRGen.Opts.ReflectionMetadata != | |||
| ReflectionMetadataMode::Runtime) || | |||
| getType()->isForeign()) { | |||
| getType()->isForeign() || | |||
| getType()->isForeignReferenceType()) { | |||
There was a problem hiding this comment.
I should remove this (see above).
|
@rjmccall can you verify that this won't affect ABI at all for existing metadata? (Specifically with the added METADATAKIND.) |
|
@swift-ci please test. |
lib/IRGen/GenMeta.cpp
Outdated
| // Always leave the superclass pointer unfilled. We'll have to | ||
| // unique it during initialization anyway, so we might as well spare | ||
| // ourselves the load-time work. | ||
| B.addNullPointer(IGM.TypeMetadataPtrTy); |
There was a problem hiding this comment.
I don't see this superclass field in the metadata class.
In general, if there isn't any commonality of layout in the metadata classes, I'm not sure we should be using related metadata builder classes; it just ends up obscuring how the code in GenMeta matches the structs in Metadata.h.
There was a problem hiding this comment.
This is using ForeignMetadataBuilderBase which seemed generic enough. I'll just make this into a no-op + assert. Let me know if you want me to add another builder, though.
There was a problem hiding this comment.
Looking at this again, I see that I am actually using ForeignClassMetadataBuilderBase. I added my own builder for FRTs and updated it to use that. Thanks for pointing this out.
|
@swift-ci please test. |
| @@ -1,7 +1,7 @@ | |||
| // RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop -Xfrontend -validate-tbd-against-ir=none -Xfrontend -disable-llvm-verify -g) | |||
| // | |||
| // REQUIRES: executable_test | |||
| // REQUIRES: foreign-reference-type-metadata-support | |||
| // XFAIL: OS=windows-msvc | |||
There was a problem hiding this comment.
This matches all existing executable tests for foreign reference types.
…r foreign reference types.
|
@swift-ci please test. |
|
@swift-ci please test macOS |
|
I'm going to go ahead and merge this. If there are any more comments, I'm happy to address them post-commit (or if necessary, revert). |
| @@ -31,6 +31,7 @@ swift::_swift_isObjCTypeNameSerializable(Class theClass) { | |||
| switch (type->getKind()) { | |||
| case MetadataKind::ObjCClassWrapper: | |||
| case MetadataKind::ForeignClass: | |||
| case MetadataKind::ForeignReferenceType: | |||
There was a problem hiding this comment.
Is this right?
- Zoe wrote this from the future
|
The pieces in the runtime directory will need to be back deployed. |
This is the first of a few PRs to add a value witness table, metadata, and runtime support for foreign reference types.