-
Notifications
You must be signed in to change notification settings - Fork 10.6k
Preliminary support for existential in embedded Swift #85551
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
Preliminary support for existential in embedded Swift #85551
Conversation
Allow storing struct, enum, and tuple types in an any.
We need to add a version of swift_allocBox to the embedded runtime implementation for outline storage
Code using the outline heap storage path will crash and burn because support is incomplete. But at least inline storage existential inhabitants should compile and run.
... or so I believe
|
@swift-ci test |
|
We will also need: #85426 |
lib/IRGen/GenDecl.cpp
Outdated
| concreteType, TypeMetadataAddress::FullMetadata)); | ||
|
|
||
| if (Context.LangOpts.hasFeature(Feature::Embedded)) { | ||
| if (Context.LangOpts.hasFeature(Feature::Embedded) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary insertion of ' '
include/swift/IRGen/Linking.h
Outdated
|
|
||
| inline bool isEmbeddedWithoutEmbeddedExitentials(CanType t) { | ||
| return t->getASTContext().LangOpts.hasFeature(Feature::Embedded) && | ||
| !t->getASTContext().LangOpts.hasFeature(Feature::EmbeddedExistentials); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's best to pull out the ASTContext only once
| // reasons to continue using "generic" value witness table functions i.e the | ||
| // same once used for runtime instantiated generic metadata. | ||
| if (!IGM.Context.LangOpts.hasFeature(Feature::EmbeddedExistentials)) { | ||
| if (auto boundGenericType = dyn_cast<BoundGenericType>(abstractType)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this code used for? It's already wrong if you have a non-generic type nested in a generic one, like
struct G<T> {
struct Inner {}
}
Something like G<Int>.Inner is not a BGT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pre-existing code.
It is "safe" to fail the check on G<Int>.Inner or any type for that matter:
We can generate value witness tables for generic metadata patterns or for specialized generic metadata.
When we added the ability to generate specialized metadata initially the code would generate specialized value witnesses (i.e a value witness that knows self is e.g G<Int>).
That lead to undesired code bloat. So when we see a G<Int> the code behind that check makes it such that we lower the value witness under the assumption that self is G<T> (except for the the non-function witnesses such size, flags, stride).
When we fail that check we will use "specialized value witness functions" i.e. functions that "know" that the type is G<Int>.Inner rather than the ones we would use for the generic case (G<T>.Inner).
That comes at the cost of code size but is not incorrect.
This code path is only exercised if you compile with specialized generic metadata on (we only do this behind a flag / in the standard library).
Do you have to walk the parent chain to detect a bound generic type in the ancestry or is there an API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TypeBase::isSpecialized() returns true if the type itself or any parent is a BGT. Do you mind fixing this and adding a test case? Subsequent PR is totally fine of course.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take a look
lib/IRGen/MetadataRequest.cpp
Outdated
| IRGenFunction::emitTypeMetadataRef(CanType type, | ||
| DynamicMetadataRequest request) { | ||
| if (type->getASTContext().LangOpts.hasFeature(Feature::Embedded) && | ||
| !type->getASTContext().LangOpts.hasFeature(Feature::EmbeddedExistentials) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to pull the ASTContext out only once into a local variable. Also there's a third usage below you could simplify
|
@swift-ci test |
|
@swift-ci test linux |
Still missing is casting from existential to concrete type (the only we will initially support) and a bigger set of types (e.g foreign types).