[embedded] Initial Swift Concurrency for embedded Swift #68928
Conversation
kubamracek
requested review from
mikeash,
DougGregor,
rokhinip,
al45tair,
zoecarver and
eeckstein
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
| @@ -141,6 +141,19 @@ class AsyncLetWithBufferTaskOptionRecord : public TaskOptionRecord { | |||
| } | |||
| }; | |||
|
|
|||
| class ResultTypeInfoTaskOptionRecord : public TaskOptionRecord { | |||
There was a problem hiding this comment.
Should this be #if to only be for embedded, or is it useful outside of that context? Also, should this store a ResultTypeInfo rather than repeating its fields.
There was a problem hiding this comment.
I was fretting about this in the definition of ResultTypeInfo. Should we have ResultTypeInfo 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.
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.
Made the class ifdef'd to embedded only, for now, and I'll explore the idea of using ResultTypeInfo in desktop Swift separately.
kubamracek
force-pushed
the
embedded-concurrency
branch
from
1a0ff97 to
b7415fb
Compare
|
@swift-ci please test |
There was a problem hiding this comment.
Overall looks good but incredibly unfortunate to have yet another #if in concurrency library... it's already messy with all the various #ifs there...
Will the embedded configuration be tested in normal smoke tests, so we don't have to hunt for fixing it afterwards?
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
| // Explicitly allowed in embedded Swift because we generate value witnesses | ||
| // (but not witness tables) for Swift Concurrency usage. |
There was a problem hiding this comment.
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.
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?
|
@swift-ci please test Windows platform |
kubamracek
force-pushed
the
embedded-concurrency
branch
from
b111c2e to
df58797
Compare
|
@swift-ci please test |
Yes. It will build as part of regular builds, tests will run as part of regular PR testing too. |
|
@swift-ci please test |
|
@swift-ci please test Windows platform |
PR starts building a (simplified, cut-down) embedded version of the Swift Concurrency runtime library, implements a modified contract between the compiler and swift_task_create() that doesn't rely on metadata, and adds an executable test that exercises a simple task creation showing the setup working (!).
Note that there is definitely a lot of Swift Concurrency functionality still missing, this doesn't achieve full Swift Concurrency parity.
Major parts involved: