[Sema]: ban @isolated(any) conversions to synchronous function types #80812
Conversation
|
@swift-ci please smoke test macos |
lib/Sema/CSSimplify.cpp
Outdated
| @@ -2985,6 +2985,10 @@ ConstraintSystem::matchFunctionIsolations(FunctionType *func1, | |||
| case FunctionTypeIsolation::Kind::Erased: | |||
| return matchIfConversion(); | |||
|
|
|||
| // TODO: should this be handled here? | |||
There was a problem hiding this comment.
Rejecting invalid conversions in the solver is generally better. For example if you have two overloads and one of the choices involves the invalid conversion, diagnosing the conversion after the solution is applied means the code will fail to type check with an ambiguity.
There was a problem hiding this comment.
thanks! and just to clarify as i'm not very familiar with this area of the code – this file is part of the (constraint?) 'solver', correct?
i assume i should remove the logic from the actor isolation checker in favor of this then, since presumably there's no reason to duplicate the check in both places – or is there?
There was a problem hiding this comment.
although... the comment up above does give me some pause (i don't fully grasp what it's trying to communicate):
// Erasing global-actor isolation to non-isolation can admit data
// races; such violations are diagnosed by the actor isolation checker.
// We deliberately do not allow actor isolation violations to influence
// overload resolution to preserve the property that an expression can
// be re-checked against a different isolation context for isolation
// violations.
//
// This also applies to @isolated(any) because we want to be able to
// decide that we contextually isolated to the function's dynamic
// isolation.
edit: in this instance, perhaps the concerns there don't apply, since this issue is more about a conversion that should presumably always be invalid?
jamieQ
force-pushed
the
invalid-isolated-any-conversion
branch
from
a45fb51 to
6711957
Compare
|
@swift-ci please smoke test |
| @@ -2943,6 +2943,11 @@ ConstraintSystem::matchFunctionIsolations(FunctionType *func1, | |||
| ConstraintLocatorBuilder locator) { | |||
| auto isolation1 = func1->getIsolation(), isolation2 = func2->getIsolation(); | |||
|
|
|||
| // An @isolated(any) function cannot be converted to a non-@isolated(any) | |||
| // synchronous function regardless of whether or not it is itself async. | |||
| if (isolation1.isErased() && !isolation2.isErased() && !func2->isAsync()) | |||
There was a problem hiding this comment.
the early exit here presumably means some of the 'score increase' logic that used to occur no longer will. does that seem generally okay?
additionally, what's the appropriate way to handle possible source breakage from these conversions no longer working?
jamieQ
changed the title
|
note: the proposal for |
|
@swift-ci please smoke test linux |
currently, function conversions from
@isolated(any)functions to synchronous, non-@isolated(any)functions are permitted, despite this being expressly stated as an invalid conversion in SE-0431. this change updates the constraint solver to disallow such conversions.resolves #80823
relevant forum posts: one & two