Skip to content

Commit be253ab

Browse files
committed
[sending] Improve the sending mismatch errors and make them warnings when not in swift 6.
This will ensure that we do not break anyone who has adopted APIs like CheckedContinuation.resume that now have sending parameters. An example of where this can come up is shown by the ProcessType in SwiftToolsCore: ```swift @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) @discardableResult public func waitUntilExit() async throws -> ProcessResult { try await withCheckedThrowingContinuation { continuation in DispatchQueue.processConcurrent.async { self.waitUntilExit(continuation.resume(with:)) } } } ``` This fails to compile since self.waitUntilExit doesn't expect a function that takes a sending parameter. We want to give people time to fix such issues. (cherry picked from commit 16d0194)
1 parent fb187bf commit be253ab

File tree

7 files changed

+157
-8
lines changed

7 files changed

+157
-8
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7952,6 +7952,19 @@ ERROR(sending_only_on_parameters_and_results, none,
79527952
"'sending' may only be used on parameters and results", ())
79537953
ERROR(sending_cannot_be_applied_to_tuple_elt, none,
79547954
"'sending' cannot be applied to tuple elements", ())
7955+
ERROR(sending_function_wrong_sending,none,
7956+
"converting a value of type %0 to type %1 risks causing data races",
7957+
(Type, Type))
7958+
NOTE(sending_function_param_with_sending_param_note, none,
7959+
"converting a function typed value with a sending parameter to one "
7960+
"without risks allowing actor-isolated values to escape their isolation "
7961+
"domain as an argument to an invocation of value",
7962+
())
7963+
NOTE(sending_function_result_with_sending_param_note, none,
7964+
"converting a function typed value without a sending result as one with "
7965+
"risks allowing actor-isolated values to escape their "
7966+
"isolation domain through a result of an invocation of value",
7967+
())
79557968

79567969
#define UNDEFINE_DIAGNOSTIC_MACROS
79577970
#include "DefineDiagnosticMacros.h"

include/swift/Sema/CSFix.h

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,17 @@ enum class FixKind : uint8_t {
471471
/// Ignore situations when key path subscript index gets passed an invalid
472472
/// type as an argument (something that is not a key path).
473473
IgnoreKeyPathSubscriptIndexMismatch,
474+
475+
/// Ignore the following situations:
476+
///
477+
/// 1. Where we have a function that expects a function typed parameter
478+
/// without a sendable parameter but is passed a function type with a sending
479+
/// parameter.
480+
///
481+
/// 2. Where we have a function that expects a function typed parameter with a
482+
/// sending result, but is passed a function typeed parameter without a
483+
/// sending result.
484+
AllowSendingMismatch,
474485
};
475486

476487
class ConstraintFix {
@@ -2619,6 +2630,49 @@ class TreatEphemeralAsNonEphemeral final : public AllowArgumentMismatch {
26192630
}
26202631
};
26212632

2633+
/// Error if a user passes let f: (sending T) -> () as a (T) -> ().
2634+
///
2635+
/// This prevents data races since f assumes its parameter if the parameter is
2636+
/// non-Sendable is safe to transfer onto other situations. The caller though
2637+
/// that this is being sent to does not enforce that invariants within its body.
2638+
class AllowSendingMismatch final : public ContextualMismatch {
2639+
public:
2640+
enum class Kind {
2641+
Parameter,
2642+
Result,
2643+
};
2644+
2645+
private:
2646+
Kind kind;
2647+
2648+
AllowSendingMismatch(ConstraintSystem &cs, Type argType, Type paramType,
2649+
ConstraintLocator *locator, Kind kind,
2650+
FixBehavior fixBehavior)
2651+
: ContextualMismatch(cs, FixKind::AllowSendingMismatch, argType,
2652+
paramType, locator, fixBehavior),
2653+
kind(kind) {}
2654+
2655+
public:
2656+
std::string getName() const override {
2657+
return "treat a function argument with sending parameter as a function "
2658+
"argument without sending parameters";
2659+
}
2660+
2661+
bool diagnose(const Solution &solution, bool asNote = false) const override;
2662+
2663+
bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
2664+
return diagnose(*commonFixes.front().first);
2665+
}
2666+
2667+
static AllowSendingMismatch *create(ConstraintSystem &cs,
2668+
ConstraintLocator *locator, Type srcType,
2669+
Type dstType, Kind kind);
2670+
2671+
static bool classof(const ConstraintFix *fix) {
2672+
return fix->getKind() == FixKind::AllowSendingMismatch;
2673+
}
2674+
};
2675+
26222676
class SpecifyBaseTypeForContextualMember final : public ConstraintFix {
26232677
DeclNameRef MemberName;
26242678

lib/Sema/CSDiagnostics.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7918,6 +7918,24 @@ bool NonEphemeralConversionFailure::diagnoseAsError() {
79187918
return true;
79197919
}
79207920

7921+
bool SendingOnFunctionParameterMismatchFail::diagnoseAsError() {
7922+
emitDiagnosticAt(getLoc(), diag::sending_function_wrong_sending,
7923+
getFromType(), getToType())
7924+
.warnUntilSwiftVersion(6);
7925+
emitDiagnosticAt(getLoc(),
7926+
diag::sending_function_param_with_sending_param_note);
7927+
return true;
7928+
}
7929+
7930+
bool SendingOnFunctionResultMismatchFailure::diagnoseAsError() {
7931+
emitDiagnosticAt(getLoc(), diag::sending_function_wrong_sending,
7932+
getFromType(), getToType())
7933+
.warnUntilSwiftVersion(6);
7934+
emitDiagnosticAt(getLoc(),
7935+
diag::sending_function_result_with_sending_param_note);
7936+
return true;
7937+
}
7938+
79217939
bool AssignmentTypeMismatchFailure::diagnoseMissingConformance() const {
79227940
auto srcType = getFromType();
79237941
auto dstType = getToType()->lookThroughAllOptionalTypes();

lib/Sema/CSDiagnostics.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2291,6 +2291,28 @@ class NonEphemeralConversionFailure final : public ArgumentMismatchFailure {
22912291
void emitSuggestionNotes() const;
22922292
};
22932293

2294+
class SendingOnFunctionParameterMismatchFail final : public ContextualFailure {
2295+
public:
2296+
SendingOnFunctionParameterMismatchFail(const Solution &solution, Type srcType,
2297+
Type dstType,
2298+
ConstraintLocator *locator,
2299+
FixBehavior fixBehavior)
2300+
: ContextualFailure(solution, srcType, dstType, locator, fixBehavior) {}
2301+
2302+
bool diagnoseAsError() override;
2303+
};
2304+
2305+
class SendingOnFunctionResultMismatchFailure final : public ContextualFailure {
2306+
public:
2307+
SendingOnFunctionResultMismatchFailure(const Solution &solution, Type srcType,
2308+
Type dstType,
2309+
ConstraintLocator *locator,
2310+
FixBehavior fixBehavior)
2311+
: ContextualFailure(solution, srcType, dstType, locator, fixBehavior) {}
2312+
2313+
bool diagnoseAsError() override;
2314+
};
2315+
22942316
class AssignmentTypeMismatchFailure final : public ContextualFailure {
22952317
public:
22962318
AssignmentTypeMismatchFailure(const Solution &solution,

lib/Sema/CSFix.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1845,6 +1845,34 @@ std::string TreatEphemeralAsNonEphemeral::getName() const {
18451845
return name;
18461846
}
18471847

1848+
bool AllowSendingMismatch::diagnose(const Solution &solution,
1849+
bool asNote) const {
1850+
switch (kind) {
1851+
case Kind::Parameter: {
1852+
SendingOnFunctionParameterMismatchFail failure(
1853+
solution, getFromType(), getToType(), getLocator(), fixBehavior);
1854+
return failure.diagnose(asNote);
1855+
}
1856+
case Kind::Result: {
1857+
SendingOnFunctionResultMismatchFailure failure(
1858+
solution, getFromType(), getToType(), getLocator(), fixBehavior);
1859+
return failure.diagnose(asNote);
1860+
}
1861+
}
1862+
llvm_unreachable("Covered switch isn't covered?!");
1863+
}
1864+
1865+
AllowSendingMismatch *AllowSendingMismatch::create(ConstraintSystem &cs,
1866+
ConstraintLocator *locator,
1867+
Type srcType, Type dstType,
1868+
Kind kind) {
1869+
auto fixBehavior = cs.getASTContext().LangOpts.isSwiftVersionAtLeast(6)
1870+
? FixBehavior::Error
1871+
: FixBehavior::DowngradeToWarning;
1872+
return new (cs.getAllocator())
1873+
AllowSendingMismatch(cs, srcType, dstType, locator, kind, fixBehavior);
1874+
}
1875+
18481876
bool SpecifyBaseTypeForContextualMember::diagnose(const Solution &solution,
18491877
bool asNote) const {
18501878
MissingContextualBaseInMemberRefFailure failure(solution, MemberName,

lib/Sema/CSSimplify.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3239,7 +3239,11 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
32393239
// () -> sending T can be a subtype of () -> T... but not vis-a-versa.
32403240
if (func1->hasSendingResult() != func2->hasSendingResult() &&
32413241
(!func1->hasSendingResult() || kind < ConstraintKind::Subtype)) {
3242-
return getTypeMatchFailure(locator);
3242+
auto *fix = AllowSendingMismatch::create(
3243+
*this, getConstraintLocator(locator), func1, func2,
3244+
AllowSendingMismatch::Kind::Result);
3245+
if (recordFix(fix))
3246+
return getTypeMatchFailure(locator);
32433247
}
32443248

32453249
if (!matchFunctionIsolations(func1, func2, kind, flags, locator))
@@ -3676,7 +3680,11 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
36763680
// with a function that expects a non-sending parameter.
36773681
if (func1Param.getParameterFlags().isSending() &&
36783682
!func2Param.getParameterFlags().isSending()) {
3679-
return getTypeMatchFailure(argumentLocator);
3683+
auto *fix = AllowSendingMismatch::create(
3684+
*this, getConstraintLocator(argumentLocator), func1, func2,
3685+
AllowSendingMismatch::Kind::Parameter);
3686+
if (recordFix(fix))
3687+
return getTypeMatchFailure(argumentLocator);
36803688
}
36813689

36823690
// FIXME: We should check value ownership too, but it's not completely
@@ -15117,6 +15125,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
1511715125
}
1511815126
}
1511915127

15128+
case FixKind::AllowSendingMismatch:
1512015129
case FixKind::InsertCall:
1512115130
case FixKind::RemoveReturn:
1512215131
case FixKind::RemoveAddressOf:

test/Concurrency/transfernonsendable_functionsubtyping.swift

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-typecheck-verify-swift -swift-version 6 %s
1+
// RUN: %target-typecheck-verify-swift -swift-version 6
22

33
// READ THIS! This file only contains tests that validate that the relevant
44
// function subtyping rules for sending work. Please do not put other tests in
@@ -38,15 +38,17 @@ func takeFnWithoutSendingParam(_ fn: (NonSendableKlass) -> ()) {}
3838

3939
func testFunctionMatching() {
4040
let _: (NonSendableKlass) -> () = functionWithSendingParameter
41-
// expected-error @-1 {{cannot convert value of type '@Sendable (sending NonSendableKlass) -> ()' to specified type '(NonSendableKlass) -> ()'}}
41+
// expected-error @-1 {{converting a value of type '@Sendable (sending NonSendableKlass) -> ()' to type '(NonSendableKlass) -> ()' risks causing data races}}
42+
// expected-note @-2 {{converting a function typed value with a sending parameter to one without risks allowing actor-isolated values to escape their isolation domain as an argument to an invocation of value}}
4243
let _: (sending NonSendableKlass) -> () = functionWithSendingParameter
4344

4445
let _: (NonSendableKlass) -> () = functionWithoutSendingParameter
4546
let _: (sending NonSendableKlass) -> () = functionWithoutSendingParameter
4647

4748
takeFnWithSendingParam(functionWithSendingParameter)
4849
takeFnWithoutSendingParam(functionWithSendingParameter)
49-
// expected-error @-1 {{@Sendable (sending NonSendableKlass) -> ()' to expected argument type '(NonSendableKlass) -> ()}}
50+
// expected-error @-1 {{converting a value of type '@Sendable (sending NonSendableKlass) -> ()' to type '(NonSendableKlass) -> ()' risks causing data races}}
51+
// expected-note @-2 {{converting a function typed value with a sending parameter to one without risks allowing actor-isolated values to escape their isolation domain as an argument to an invocation of value}}
5052
takeFnWithSendingParam(functionWithoutSendingParameter)
5153
takeFnWithoutSendingParam(functionWithoutSendingParameter)
5254
}
@@ -56,14 +58,17 @@ func testReturnValueMatching() {
5658
let _: () -> sending NonSendableKlass = functionWithSendingResult
5759
let _: () -> NonSendableKlass = functionWithoutSendingResult
5860
let _: () -> sending NonSendableKlass = functionWithoutSendingResult
59-
// expected-error @-1 {{cannot convert value of type '@Sendable () -> NonSendableKlass' to specified type '() -> sending NonSendableKlass'}}
61+
// expected-error @-1 {{converting a value of type '@Sendable () -> NonSendableKlass' to type '() -> sending NonSendableKlass' risks causing data races}}
62+
// expected-note @-2 {{converting a function typed value without a sending result as one with risks allowing actor-isolated values to escape their isolation domain through a result of an invocation of value}}
6063

6164
takeFnWithSendingResult(functionWithSendingResult)
6265
takeFnWithSendingResult(functionWithoutSendingResult)
63-
// expected-error @-1 {{cannot convert value of type '@Sendable () -> NonSendableKlass' to expected argument type '() -> sending NonSendableKlass'}}
66+
// expected-error @-1 {{converting a value of type '@Sendable () -> NonSendableKlass' to type '() -> sending NonSendableKlass' risks causing data races}}
67+
// expected-note @-2 {{converting a function typed value without a sending result as one with risks allowing actor-isolated values to escape their isolation domain through a result of an invocation of value}}
6468
let x: () -> NonSendableKlass = { fatalError() }
6569
takeFnWithSendingResult(x)
66-
// expected-error @-1 {{cannot convert value of type '() -> NonSendableKlass' to expected argument type '() -> sending NonSendableKlass'}}
70+
// expected-error @-1 {{converting a value of type '() -> NonSendableKlass' to type '() -> sending NonSendableKlass' risks causing data races}}
71+
// expected-note @-2 {{converting a function typed value without a sending result as one with risks allowing actor-isolated values to escape their isolation domain through a result of an invocation of value}}
6772

6873
takeFnWithoutSendingResult(functionWithSendingResult)
6974
takeFnWithoutSendingResult(functionWithoutSendingResult)

0 commit comments

Comments
 (0)