-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[embedded] Initial Swift Concurrency for embedded Swift #68928
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
Changes from all commits
5d8c55e
e68faff
a1f0782
b9fe308
744f558
4926485
afa1427
df58797
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1012,7 +1012,8 @@ class LinkEntity { | |
} | ||
|
||
static LinkEntity forValueWitness(CanType concreteType, ValueWitness witness) { | ||
assert(!isEmbedded(concreteType)); | ||
// Explicitly allowed in embedded Swift because we generate value witnesses | ||
// (but not witness tables) for Swift Concurrency usage. | ||
Comment on lines
+1015
to
+1016
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we guard this on Swift concurrency being enabled? Maybe I'm being overly paranoid, but I could see some unrelated thing asking for value witness tables, and clients that don't enable concurrency shouldn't have to pay for anything (even vwt). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the right tradeoff is to catch emitting witness tables (for which the assert is still active, a few lines below), but allow individual value witness methods (this code). Even if we somehow had a bug where unexpected value witness methods are emitted, they will be dead-stripped if there isn't a table holding them. WDYT? |
||
LinkEntity entity; | ||
entity.Pointer = concreteType.getPointer(); | ||
entity.Data = LINKENTITY_SET_FIELD(Kind, unsigned(Kind::ValueWitness)) | ||
|
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.
Should this be
#if
to only be for embedded, or is it useful outside of that context? Also, should this store aResultTypeInfo
rather than repeating its fields.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 was fretting about this in the definition of
ResultTypeInfo
. Should we haveResultTypeInfo
always have the five fields, so we have consistent layout between "embedded" and non-embedded? That makes our development easier, and maybe possibly allows some level of interoperability between embedded-ish and non-embedded code, but it wastes a couple of pointers worth of storage per task.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'm not sure there's any prospect of such interoperability. In any case, the embedded side doesn't have to be ABI stable so we could fix this up later if need be.
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.
Made the class ifdef'd to embedded only, for now, and I'll explore the idea of using ResultTypeInfo in desktop Swift separately.