Align isolated conformances with the current proposal #79983
Conversation
Instead of using the `isolated P` syntax, switch to specifying the global actor type directly, e.g., class MyClass: @mainactor MyProto { ... } No functionality change at this point
Update diagnostic and Fix-It to suggest global actor isolation on a conformance (e.g, `@MainActor P`) rather than `isolated`.
…e type's With the move to explicitly specifying the global actor for an isolated conformance, we can now have conformances whose isolation differs from that of the type, including having actors with global-actor-isolated conformances. Introduce this generalization to match the proposal, and update/add tests accordingly.
It doesn't mean anything yet, but parse it and make sure it reproduces in the module interface.
The NormalProtocolConformance APIs for checking for an explicitly-written isolation on a conformance were easy to get to, and the real semantic API was buried in the type checker, leading to some unprincipled checking. Instead, create a central ProtocolConformance::getIsolation() to get the (semantic) actor isolation, and let that be the only place that will access the explicitly-written global actor isolation for a conformance. Update all call sites appropriately.
…eansMainActorIsolated When code in the current module defaults to main actor (under SE-0466), also infer main-actor isolation for protocol conformances of main-actor isolated types.
DougGregor
requested review from
xymus,
rjmccall,
ktoso,
AnthonyLatsis,
hborla,
slavapestov,
xedin,
ahoppen,
bnbarham,
CodaFi,
hamishknight,
rintaro and
tshortli
as code owners
|
@swift-ci please smoke test |
|
@swift-ci please build toolchain |
|
@swift-ci please smoke test |
|
@swift-ci please build toolchain Windows |
|
@swift-ci please smoke test Windows |
| SourceLoc globalActorAtLoc; | ||
|
|
||
| /// The global actor type to which this conformance is isolated. | ||
| TypeExpr *globalActorType = nullptr; |
There was a problem hiding this comment.
Maybe this ought to be a TypeLoc?
There was a problem hiding this comment.
It could. We get a TypeExpr out of the CustomAttr itself, so we can either stick that into a TypeLoc or keep it as-is. This way it's only a single pointer in ConformanceAttributes vs. the two-pointer TypeLoc, so I didn't do it.
| bool ConformanceIsolationRequest::isCached() const { | ||
| // We only want to cache for global-actor-isolated conformances. For | ||
| // everything else, which is nearly every conformance, this request quickly | ||
| // returns "nonisolated" so there is no point in caching it. |
There was a problem hiding this comment.
This is what split caching is for
There was a problem hiding this comment.
Ahh yes, so it is! Thanks for the reminder---I don't think I've used this feature before.
| @@ -546,6 +555,9 @@ class NormalProtocolConformance : public RootProtocolConformance, | |||
| /// NominalTypeDecl that declared the conformance. | |||
| DeclContext *Context; | |||
|
|
|||
| /// The global actor isolation for this conformance, if there is one. | |||
| TypeExpr *globalActorIsolation = nullptr; | |||
There was a problem hiding this comment.
Is this null most of the time? You can use a single bit to record the presence of this attribute, while storing the actual value in the request evaluator cache only when present.
There was a problem hiding this comment.
It's NULL most of the time. We could have a put and put it an ASTContext-managed side table, since it needs to be there at the point when the NormalProtocolConformance is created.
DougGregor
force-pushed
the
isolated-conformances-alignment
branch
from
836fcbe to
a1132ca
Compare
|
Nnnnoooooooooo!!!! I force-pushed the wrong branch. Now it is time for me to go cry |
Switch over to split caching for the conformance isolation request, which optimizes for the common case where the conformance is nonisolated. Also put the explicit global actor TypeExpr* in an ASTContext side table, so we don't take a pointer's worth of storage in every conformance. For that side table, introduce a new "ASTContext::GlobalCache" that's there only for side tables, so we don't have to go add get/set operations to ASTContext and recompile the world every time we want to add a side table like this. Thanks, Slava!
DougGregor
force-pushed
the
isolated-conformances-alignment
branch
from
a1132ca to
c7fd48a
Compare
|
@swift-ci please smoke test |
|
@swift-ci please smoke test |
Make several changes to bring the isolated conformances implementation into alignment with the latest proposal:
@MainActor P, rather thanisolated P@MainActoron conformances when SE-0466 is enabled, which can be suppressed bynonisolated